feat(lambda-rs-logging): Update log files to be kept open on flush#107
Merged
feat(lambda-rs-logging): Update log files to be kept open on flush#107
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the FileHandler in lambda-rs-logging to keep a persistent file handle via Mutex<BufWriter<File>>, reducing syscall overhead and ensuring logs continue writing to the same file descriptor even if the log path is renamed (supporting log rotation scenarios).
Changes:
- Replace the
file: Stringpath field inFileHandlerwith aMutex<io::BufWriter<File>>that is opened once at construction. - Update
FileHandler::logto buffer messages with poisoning-tolerant mutex handling, batch every 10 messages, then write through the persistentBufWriterand flush. - Add a Unix-only test
file_handler_does_not_reopen_between_flushesto verify that the handler does not reopen or recreate the original path when the log file is renamed, and continues writing to the original file descriptor.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
crates/lambda-rs-logging/src/handler.rs |
Refactors FileHandler to maintain a persistent Mutex<BufWriter<File>>, updates logging implementation to use the persistent writer with poison-recovery and std::mem::take for buffered messages, and preserves behavior of flushing after ten messages. |
crates/lambda-rs-logging/src/lib.rs |
Extends the test suite with a Unix-only test that exercises log file renaming while FileHandler is active to ensure it does not reopen the original path and that both pre- and post-rename messages are written to the same file. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Summary
Improves
FileHandlerinlambda-rs-loggingto keep the log file open via aMutex<BufWriter<File>>instead of reopening it on every buffer flush. This reduces file I/O overhead and ensures that log writes continue to the same file descriptor even if the original path is renamed or moved (useful for log rotation scenarios).Related Issues
Changes
FileHandlerto open the log file once during construction and store aMutex<io::BufWriter<File>>for writesfile: Stringfield that stored the path for repeated opensemitmethod to use proper mutex poisoning recovery withinto_inner()std::mem::taketo efficiently clear the buffer while holding the lockwriter.flush()after writing to ensure data is persistedfile_handler_does_not_reopen_between_flushesthat verifies the handler continues writing to the original file descriptor after the file path is renamedType of Change
Affected Crates
lambda-rslambda-rs-platformlambda-rs-argslambda-rs-loggingChecklist
cargo +nightly fmt --all)cargo clippy --workspace --all-targets -- -D warnings)cargo test --workspace)Testing
Commands run:
Manual verification steps (if applicable):
file_handler_does_not_reopen_between_flushesto verify that the file handler maintains the same file descriptor across flushesScreenshots/Recordings
N/A - No visual changes.
Platform Testing
Additional Notes
#[cfg(unix)]because it relies on Unix file semantics where renaming a file while it's open allows continued writes to the original file descriptorinto_inner()) ensures the handler remains functional even if a previous thread panicked while holding the lock