fix(logfiles): don't lose log lines appended during search - #3573
Merged
Conversation
The timestamp of the last log search was the modification time of the offset file, which is written after the search finishes reading the log files. A line appended to a log file while the search was running got a modification time older than the offset file. When the log file was never modified again (e.g. node shut down), the next search filtered the file out and the line was never searched. Record the time when the search starts reading the log files in the offset file (third record) and use it as the timestamp of the last search. An offset file without a valid timestamp record degrades to the old modification time behavior. Non-finite and negative timestamps are rejected, as e.g. an "inf" timestamp would permanently exclude all files from searches. The recorded timestamp is clamped to the current time in case the clock stepped backward during the search. This also improves the ignore rules expiry: the expire time is now compared with the search start, so a rule cannot expire while messages created before its expire time are still unsearched. Add unit tests covering the timestamp round trip and fallbacks, and a regression test for the appended-during-search scenario.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a correctness gap in log searching where log lines appended while a search is running could be skipped in subsequent searches due to how the “last search time” was recorded. It changes the offset file to persist the search start timestamp (instead of relying on offset-file mtime) and updates ignore-rule expiry behavior accordingly, with added unit/regression tests.
Changes:
- Persist a third record (search start timestamp) in per-log offset files, with validation and safe fallbacks.
- Use the stored timestamp (fallback to offset-file mtime when absent/invalid) as the “last search time” for subsequent searches.
- Add tests for timestamp round-trip, fallbacks, and the appended-during-search regression scenario.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| framework_tests/test_logfiles.py | Extends tests to cover timestamp persistence/fallbacks and the appended-during-search regression. |
| cardano_node_tests/utils/logfiles.py | Updates offset file format + parsing/validation, records search start time, and adjusts ignore rule expiry semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
On filesystems with coarse timestamps, a line appended shortly after the search start can get a modification time that is not newer than the recorded search start time. The next search then filtered the file out and the line was never searched. Always include the live log file and the file the seek offset was recorded for, regardless of their modification time. Their already searched content is excluded by the seek offset, so nothing is reported twice.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The timestamp of the last log search was the modification time of the offset file, which is written after the search finishes reading the log files. A line appended to a log file while the search was running got a modification time older than the offset file. When the log file was never modified again (e.g. node shut down), the next search filtered the file out and the line was never searched.
Record the time when the search starts reading the log files in the offset file (third record) and use it as the timestamp of the last search. An offset file without a valid timestamp record degrades to the old modification time behavior. Non-finite and negative timestamps are rejected, as e.g. an "inf" timestamp would permanently exclude all files from searches. The recorded timestamp is clamped to the current time in case the clock stepped backward during the search.
This also improves the ignore rules expiry: the expire time is now compared with the search start, so a rule cannot expire while messages created before its expire time are still unsearched.
Add unit tests covering the timestamp round trip and fallbacks, and a regression test for the appended-during-search scenario.