Skip to content

Fix the file channel and the test suite on Windows - #478

Merged
ppkarwasz merged 17 commits into
trunkfrom
feat/fix-file-channel-windows
Jul 31, 2026
Merged

Fix the file channel and the test suite on Windows#478
ppkarwasz merged 17 commits into
trunkfrom
feat/fix-file-channel-windows

Conversation

@ppkarwasz

@ppkarwasz ppkarwasz commented Jul 30, 2026

Copy link
Copy Markdown
Member

On Windows, the file channel cannot restart over an existing checkpoint directory: mapped and open files stay OS-locked, deletions fail with The process cannot access the file because it is being used by another process, and 20+ flume-file-channel tests fail (first observed on the Windows job of #470).

File channel (the main change)

  • Resource leaks fixed on every lifecycle path.

    • The BadCheckpointException recovery in Log.replay discarded the old queue and backing store without closing them,
    • a failed FileChannel.start() never released its log,
    • and several constructor error paths leaked file handles or mappings (including the V2 to V3 checkpoint upgrade).

    All of these now close their resources, and EventQueueBackingStoreFile.close() is idempotent with fail-fast guards against use after close.

  • The checkpoint MappedByteBuffer is reclaimed by the GC. close() drops every reference to the buffer, so the garbage collector can reclaim it and hence safely unmap the file.
    The few operations that can race the collector pay a small, Windows-only delay: the corrupt-checkpoint recovery retries deletions with GC cycles in between, and tests wait for deletability the same way (TestUtils.awaitDelete). On a first-attempt success, which is the norm everywhere and always the case on Linux, there is no extra cost.

  • The queueset MapDB is no longer memory-mapped. MapDB 0.9.x unmaps its buffers by reflecting on sun.nio.ch.DirectBuffer, which fails silently on Java 17, so db.close() left the files locked. The index only lives during replay, so plain file access is fine.

Windows fixes outside the file channel

Enabling windows-latest in CI flushed out one failing module per run:

  • flume-ng-node and flume-ng-environment-variable-config-filter: the EnvironmentVariables JUnit rule needs --add-opens java.base/java.lang=ALL-UNNAMED on Windows, where it also patches ProcessEnvironment.theCaseInsensitiveEnvironment.
  • flume-tools: TestFileChannelIntegrityTool retries its teardown deletions, so a still-mapped checkpoint no longer leaks files into the next test.
  • flume-ng-core: TestLogFile closes its writer before deleting the file, and TestLog fills the disk well below the minimum-space limit, so background churn on a shared runner cannot push the usable space back above it.
  • Tests that are inherently POSIX are skipped on Windows: TestExternalProcessConfigFilter (shell scripts), TestFileChannelErrorMetrics (deletes in-use files), and the Taildir tests (unix:ino file attribute). The syslog TLS test fix is copied from Build with the logging-parent reusable workflows #470.

Both ubuntu-latest and windows-latest CI jobs are green. Whichever of this PR and #470 merges second drops the build.yml change (replaced there by build.yaml) and deduplicates the copied commits.

🤖 Generated with Claude Code

TaildirSource tracks files by inode, so its tests cannot pass when the
default file system lacks a `unix` attribute view (e.g. on Windows).
Document that requirement in the user guide.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
The test wrote to the TLS socket and immediately closed it. The client
never reads the TLS 1.3 session tickets sent by the server, so close()
aborts the connection with a TCP RST, which on Windows discards the
syslog record before the server reads it. Keep the socket open, waiting
with Awaitility, until the server commits the event to the channel.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
Assisted-By: Claude Fable 5 <noreply@anthropic.com>
On Java 17 nothing unmaps the checkpoint MappedByteBuffer, and on
Windows a mapped file cannot be deleted, so checkpoint recovery and
restarts fail. Unmap explicitly via sun.misc.Unsafe.invokeCleaner,
make close() idempotent, guard the accessors against use after unmap
(which would crash the JVM) and release the mapping when a constructor
throws BadCheckpointException or when upgrading a V2 checkpoint.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
MapDB 0.9.x cannot unmap its buffers on Java 17, so disable
memory-mapped files for the transient queue set. Close the queue and
backing store discarded by the bad-checkpoint recovery in Log.replay,
close the log of a channel whose start() failed, and release the
inflight files when the queue constructor throws. Windows keeps open
and mapped files locked, so any of these leaks made the channel unable
to restart over the same directories.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
Several tests leaked FlumeEventQueue instances or reused one temporary
directory across test methods, which fails on Windows where the leaked
MapDB files cannot be deleted. Also stop channels whose start() failed,
now that FileChannel releases the log in that case.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
TestFileChannelRestart never closed the stream it corrupted the
checkpoint metadata with, so the recovery could not delete the file on
Windows. TestFileChannelEncryption asserted the POSIX rendering of a
FileNotFoundException message. TestFileChannelErrorMetrics injects I/O
errors by deleting files the running channel uses, which Windows does
not allow at all, so those tests are skipped there.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR targets Windows-specific file-handle and memory-mapping retention issues in the Flume file channel, improving restart reliability over existing checkpoint directories and stabilizing Windows CI by tightening resource lifecycle management and test cleanup.

Changes:

  • Disable MapDB mmap usage for the transient queueSet index and add explicit MappedByteBuffer unmapping to release Windows file locks promptly.
  • Fix several file-channel lifecycle leaks (replay recovery, failed start/stop paths, constructor failure paths) and harden tests to close resources and avoid Windows-incompatible test patterns.
  • Update docs/tests and CI to reflect Windows limitations (taildir inode tracking) and validate on windows-latest.

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/site/antora/modules/ROOT/pages/FlumeUserGuide.adoc Clarifies taildir source requires unix file attributes; explicitly notes it won’t work on Windows.
flume-parent/pom.xml Adds Awaitility version + dependency management for new test synchronization.
flume-ng-sources/flume-taildir-source/src/test/java/org/apache/flume/source/taildir/TestTaildirSource.java Skips taildir tests when the default FS lacks the unix attribute view.
flume-ng-sources/flume-taildir-source/src/test/java/org/apache/flume/source/taildir/TestTaildirEventReader.java Same as above for the event reader tests.
flume-ng-sources/flume-syslog-source/src/test/java/org/apache/flume/source/syslog/TestSyslogTcpSource.java Uses Awaitility + safer socket lifecycle to reduce Windows TLS/RST-related flakiness.
flume-ng-sources/flume-syslog-source/pom.xml Adds Awaitility test dependency for syslog source tests.
flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestFlumeEventQueue.java Ensures per-test isolation/cleanup for queue backing store directories and closes leaked resources.
flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestFileChannelRestart.java Closes streams needed for Windows checkpoint-file deletion during recovery.
flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestFileChannelErrorMetrics.java Skips deletion-based I/O error injection tests on Windows where open-file deletion is unsupported.
flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestFileChannelBase.java Always stops the channel to release partially-opened resources after failed starts.
flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestEventQueueBackingStoreFactory.java Ensures queues are closed to release files before subsequent steps/tests.
flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestCheckpointRebuilder.java Closes queues after replay to release checkpoint files before channel replay.
flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestCheckpoint.java Adds cleanup and explicit replay completion/close to avoid queueSet DB locking.
flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/encryption/TestFileChannelEncryption.java Makes exception-message assertions platform-tolerant.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/MappedFiles.java Introduces explicit unmapping via Unsafe.invokeCleaner to release Windows file locks.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java Closes discarded queues/backing stores during bad-checkpoint recovery; guards queue usage in close.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FlumeEventQueue.java Disables MapDB mmap mode and adds constructor failure cleanup to avoid handle leaks.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java Ensures stop()/close() releases resources even if start() failed partway.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/EventQueueBackingStoreFileV3.java Ensures constructor failures release superclass checkpoint mapping to avoid Windows locks.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/EventQueueBackingStoreFileV2.java Ensures runtime constructor failures close/unmap resources.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/EventQueueBackingStoreFile.java Refactors mapping creation with safe cleanup on failure; adds close idempotence and closed guards.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/EventQueueBackingStoreFactory.java Ensures V2 backing store is closed before V3 remaps the same checkpoint during upgrade.
.github/workflows/build.yml Adds windows-latest to the workflow matrix to validate on Windows.
Suppressed comments (1)

flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestFlumeEventQueue.java:69

  • reset() creates a new temp directory but never cleans up the previous one before overwriting baseDir, so the directory created in the supplier constructor leaks. Also, inflightTakes/inflightPuts are pointing at the opposite filenames (inflightputs vs inflighttakes), which is confusing and inconsistent with the rest of the file-channel tests.
        void reset() {
            baseDir = Files.createTempDir();
            checkpoint = new File(baseDir, "checkpoint");
            inflightTakes = new File(baseDir, "inflightputs");
            inflightPuts = new File(baseDir, "inflighttakes");

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

ppkarwasz and others added 3 commits July 31, 2026 15:54
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Attach close failures to the original exception instead of logging
them separately, so they show up with their cause in the recovery
warnings. Fold the V3 constructor helper back into the constructor,
which lets metaDataFile become final.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
The FlumeEventQueue constructor deletes the queueset directory, which
fails on Windows while the previous queue instance still holds the
MapDB open. Production code always calls replayComplete() before a
queue is abandoned, so do the same in the tests.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
@ppkarwasz
ppkarwasz marked this pull request as ready for review July 31, 2026 14:28
Forcibly unmapping the checkpoint MappedByteBuffer via
sun.misc.Unsafe.invokeCleaner is unsafe: any access to an unmapped
buffer crashes the JVM. Drop the buffer references when the backing
store closes and let the garbage collector release the mapping instead.

The corrupt-checkpoint recovery path deletes checkpoint files moments
after the discarded store is closed, so retry the deletion with GC
cycles in between (a no-op when the first attempt succeeds). Tests that
delete a recently mapped checkpoint wait the same way via
TestUtils.awaitDelete. The V2 to V3 upgrade now reads the queue state,
closes the V2 store and backs up the file before rewriting it in place,
which needs no unmapping: writing to and re-mapping a mapped file is
allowed even on Windows, only deletion is blocked.

Also ratchet the spotbugs and pmd ceilings to the current counts.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
Assisted-By: Claude Fable 5 <noreply@anthropic.com>
TestLogFile deleted the data file while the writer from setup() still
had it open, which Windows forbids. TestLog filled the disk to exactly
the minimum required space, so any temp files freed by other processes
on a shared CI runner pushed the usable space back above the limit
before the put checked it.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
On Windows the rule also patches
ProcessEnvironment.theCaseInsensitiveEnvironment, which lives in
java.lang, while on Linux opening java.util was enough.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
On Windows the checkpoint file of a stopped channel cannot be deleted
until the garbage collector releases its mapping. The failed teardown
leaked files into the shared directories and broke the following tests.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
Same fix as in flume-ng-node: on Windows the rule also patches
ProcessEnvironment.theCaseInsensitiveEnvironment, which lives in
java.lang.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
The external commands under test are shell scripts, which Windows
cannot execute.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
@ppkarwasz
ppkarwasz requested a review from Copilot July 31, 2026 19:23
@ppkarwasz ppkarwasz changed the title Fix the file channel on Windows Fix the file channel and the test suite on Windows Jul 31, 2026
@ppkarwasz
ppkarwasz requested a review from rgoers July 31, 2026 19:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 31 out of 31 changed files in this pull request and generated no new comments.

Suppressed comments (2)

flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestCheckpoint.java:70

  • Only queueOut2 is closed. queueIn and queueOut keep their inflight RandomAccessFiles open, which can prevent cleanup from deleting inflightTakes/inflightPuts on Windows and also leaks file descriptors on all platforms. Close the earlier queues as well once you are done with them.
        queueOut2.close();

flume-ng-sources/flume-syslog-source/src/test/java/org/apache/flume/source/syslog/TestSyslogTcpSource.java:283

  • The Awaitility wait has no explicit upper bound. If the event is never accepted (e.g., regression/flaky CI), this can stall the test suite for a long time. Add an explicit atMost timeout to keep failures bounded and easier to diagnose.
            await().until(() -> source.getSourceCounter().getEventAcceptedCount() >= 1);

@ppkarwasz
ppkarwasz merged commit ae6c7db into trunk Jul 31, 2026
6 checks passed
@ppkarwasz
ppkarwasz deleted the feat/fix-file-channel-windows branch July 31, 2026 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants