feat(file source): notify-based discovery + idle handle closing (#3567) - #26332
feat(file source): notify-based discovery + idle handle closing (#3567)#26332Smelentyev wants to merge 1 commit into
Conversation
…ordotdev#3567) The file source keeps a file handle open, and re-globs/re-fingerprints, every matched file forever -- even files excluded by ignore_older -- which causes severe CPU/memory/handle overhead once the matched file count grows large. Adds two independent, opt-in mechanisms that together address both halves of that problem: - idle_timeout_secs (default: 60s): closes a file's handle once it has reached EOF and received no new data for that long. A cheap fs::metadata poll (not a full re-fingerprint) detects when an idle file gets new data again and reopens it. This is what stops the source from holding thousands of open handles for inactive files. - file_discovery_mode: notify (opt-in, default remains polling): uses OS-level file system event notifications (inotify on Linux, FSEvents on macOS, ReadDirectoryChangesW on Windows) instead of periodic glob re-scanning to discover new files and wake up idle-closed watchers. A much less frequent periodic reconciliation pass (reconcile_interval_secs) still runs as a correctness backstop. This is what stops the source from paying the cost of re-globbing and re-fingerprinting the entire matched set on a fixed interval, which idle_timeout_secs alone does not address. A three-way benchmark (polling-only idle-close vs. notify+idle-close vs. release) on 20,000 files, across Docker Desktop/WSL2, native Windows, and a real Linux VPS, confirmed notify+idle-close is the only combination that solves both the file descriptor count and the CPU/reaction-latency sides of vectordotdev#3567 at once. Tests: cargo test -p file-source -p file-source-common -- 51 passed, 0 failed. Covers the idle/active state machine (deactivate/reactivate, truncation detection while idle -- including truncation observed on the same poll as a forced post-failure retry, and truncation followed by refill past the old read position -- same-inode vs. rotated-file identity, rotation while idle, gzip files) and notify glob-pattern-to-watched-directory mapping. cargo clippy -p file-source -p file-source-common --all-targets -- -D warnings: clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
github-actions
Bot
added
the
domain: sources
|
Thank you for your contribution! Before we can merge this PR, please sign our Contributor License Agreement. To sign, copy and post the phrase below as a new comment on this PR.
I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b1d3b15b1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } else { | ||
| (Box::new(BufReader::new(gzip_multiple_decoder(reader))), 0) |
There was a problem hiding this comment.
When a gzip file starts with read_from: end, FileWatcher::new represents the intentionally skipped stream with a null reader and position 0. After the default idle timeout deactivates it, even an mtime-only change causes this branch to install a real gzip decoder because the position is still 0, emitting the entire backlog that read_from: end was supposed to skip. The idle state needs to retain whether the gzip stream was intentionally suppressed rather than inferring that solely from file_position.
Useful? React with 👍 / 👎.
| // Only record success: if `watch` failed, leaving this path out of | ||
| // `watched_dirs` means the next `resync_watches` call (from the backstop | ||
| // reconciliation pass) will see it as still "wanted but not yet watched" and | ||
| // retry, rather than wrongly concluding the watch is already in place and | ||
| // never trying again. |
There was a problem hiding this comment.
In notify mode, if the literal prefix of an include such as /var/log/newapp/*.log does not exist at startup, installing its watch fails and the only retry occurs during the backstop reconciliation. Because no existing ancestor is watched, creating newapp cannot wake the source; with the default interval its files are delayed by up to five minutes, and short-lived files can disappear before discovery. Watch the nearest existing ancestor or temporarily poll at the normal discovery cadence until the desired root exists.
Useful? React with 👍 / 👎.
| if self.watched_dirs.get(path) == Some(mode) { | ||
| continue; |
There was a problem hiding this comment.
On Linux/inotify, removing a watched directory invalidates the watch on that inode, but a PathsRemoved reconciliation leaves the directory recorded in watched_dirs. Consequently this equality check skips watch() after the directory is recreated, and since its parent is not watched the source remains permanently dependent on the five-minute full reconciliations, potentially losing short-lived files. A removal affecting a watched root must evict or invalidate that registration before resynchronizing.
Useful? React with 👍 / 👎.
| let drain_result = | ||
| tokio::time::timeout(NOTIFY_EVENT_DEBOUNCE, async { | ||
| while discovery.recv().await.is_some() {} | ||
| }) |
There was a problem hiding this comment.
If an ordinary path event is followed by BackendError during this 50 ms drain window, the loop discards the error without running the match above, so forget_watches() is never called and lost backend registrations remain marked active. The same drain also suppresses overflow telemetry. Debounce path notifications while still processing each drained control message through the normal error-handling path.
Useful? React with 👍 / 👎.
| if !watcher.is_idle() || !watcher.file_findable() { | ||
| continue; |
There was a problem hiding this comment.
When an idle file is rotated to a path outside a narrow include glob such as *.log, discovery marks its watcher unfindable and this guard prevents it from ever being polled or reaching the active read path that normally marks an unfindable EOF watcher dead. It is then retained until rotate_wait, whose default is effectively infinite, so every rotation permanently adds another watcher and checkpoint to fp_map; long-running sources can accumulate unbounded state. Unfindable idle watchers need a finite reaping path independent of the default rotation wait.
Useful? React with 👍 / 👎.
| if let Some(grace_period) = self.remove_after | ||
| && watcher | ||
| .idle_since() | ||
| .is_some_and(|idle| idle >= grace_period) |
There was a problem hiding this comment.
For remove_after_secs values longer than idle_timeout_secs, this check starts the full removal grace period again when the watcher enters Idle, because idle_since is initialized during deactivation rather than when EOF or the last data was observed. With the defaults, for example, remove_after_secs: 120 removes no earlier than roughly 180 seconds, contrary to the documented 120 seconds after EOF, and notify mode can add up to another reconciliation interval. Preserve the original last-activity time when transitioning to idle.
Useful? React with 👍 / 👎.
| let discovery_interval = if using_notify { | ||
| self.reconcile_interval | ||
| } else { | ||
| self.glob_minimum_cooldown | ||
| }; |
There was a problem hiding this comment.
When file_discovery_mode is Notify but constructing the OS watcher fails, using_notify becomes false and this branch polls using glob_minimum_cooldown, even though that option is documented as ignored in notify mode and the fallback is documented to use reconcile_interval. A configuration that leaves reconcile_interval_secs at five minutes but sets the supposedly ignored glob cooldown to an hour will therefore discover files only hourly whenever inotify resources are exhausted. Select the interval from the configured discovery mode and fallback state rather than watcher presence alone.
Useful? React with 👍 / 👎.
| if watcher.path == path { | ||
| trace!( | ||
| message = "Continue watching file.", | ||
| path = ?path, | ||
| ); |
There was a problem hiding this comment.
When an existing active watcher has been quiet for more than 10 seconds but has not yet reached the 60-second idle timeout, a notify event runs discovery but this same-path branch only refreshes findability. FileWatcher::should_read still suppresses reads until 10 seconds have elapsed since its last attempt, so an append arriving just after that attempt can wait almost 10 seconds despite notify mode promising prompt wakeups; the same applies indefinitely when idle closing is disabled. The notify path needs to reset the matching watcher's EOF/read retry state or otherwise mark it immediately readable.
Useful? React with 👍 / 👎.
| impl InternalEvent for FilesIdle { | ||
| fn emit(self) { | ||
| gauge!(GaugeName::IdleFiles).set(self.count as f64); |
There was a problem hiding this comment.
After a source has any idle watchers, component_idle_files retains its last nonzero value after shutdown because the teardown in src/sources/file.rs:795 resets only FileOpen. Metrics recorders continue exporting the gauge series, so source removal or reload leaves monitoring reporting files that are no longer tracked until the process restarts. Emit FilesIdle { count: 0 } alongside the existing open-files reset.
Useful? React with 👍 / 👎.
1 participant
Summary
The
filesource keeps a file handle open, and re-globs/re-fingerprints, every matched fileforever -- even files excluded by
ignore_older-- which causes severe CPU/memory/handleoverhead once the matched file count grows large (issue #3567). This PR adds two independent,
opt-in mechanisms that together address both halves of that problem:
idle_timeout_secs(default: 60s): closes a file's handle once it has reached EOF andreceived no new data for that long. A cheap
fs::metadatapoll (not a full re-fingerprint)detects when an idle file gets new data again and reopens it. This is what stops the source
from holding thousands of open handles for inactive files.
file_discovery_mode: notify(opt-in, default remainspolling): uses OS-level filesystem event notifications (inotify on Linux, FSEvents on macOS,
ReadDirectoryChangesWonWindows) instead of periodic glob re-scanning to discover new files and wake up idle-closed
watchers. A much less frequent periodic reconciliation pass (
reconcile_interval_secs) stillruns as a correctness backstop. This is what stops the source from paying the cost of
re-globbing and re-fingerprinting the entire matched set on a fixed interval, which
idle_timeout_secsalone does not address (closing a handle after inactivity still means thenext write has to wait for the next full rescan to be noticed).
References
Closes: #3567
Vector configuration
How did you test this PR?
Unit tests
Result: 51 passed, 0 failed (30 in
file-source, 21 infile-source-common).Coverage includes the idle/active watcher state machine:
case, and the narrower case where the truncation is observed on the exact same poll as a
forced retry after a failed reactivation
notify-mode glob-pattern-to-watched-directory mapping (single*,**, nested/overlappingpatterns, literal paths, recursive vs. non-recursive merging)
Several of these tests are regression tests for bugs found across multiple rounds of review on
this branch; each was verified to actually fail without its corresponding fix before being
committed (temporarily reverting the fix, confirming the test fails, then restoring it).
Static analysis
Result: clean, no warnings.
No formatting changes needed.
Manual benchmark
Compared three variants on a 20,000-file corpus (19,990 old files past
ignore_older, 10 filesreceiving active writes/rotation) across three environments (Docker Desktop/WSL2, native Windows,
and a real Linux VPS with
--cpus=1):notify)notify+ idle-close (this PR)idle_timeout_secsalone (middle row) already fixes the file descriptor count, but stillre-globs/re-fingerprints the entire 20k-file set on a fixed interval, which shows up directly as
a CPU sawtooth pattern between polls. Adding
file_discovery_mode: notify(bottom row) removesthat rescan cycle entirely -- CPU stays flat under 0.5% the whole time, only moving in response
to actual OS-level file system events.
Also surfaced along the way:
release's current behavior exhausts a default Linuxulimit -nof1024 outright at this file count (
Too many open files (os error 24), dropped events, stoppedcheckpoint writes) -- not a synthetic edge case, but Docker's and many distros' out-of-the-box
per-process limit. This PR's idle-close mechanism avoids that by design, peaking around 20-24
open FDs regardless of matched file count.
Does this PR include user facing changes?
changelog.d/3567_file_source_notify_discovery.enhancement.md.Contributor Guidelines
cargo fmt,cargo clippy -D warnings, fullfile-source/file-source-commontest suite -- see results above).