Skip to content

Conversation

@vmarcella
Copy link
Member

Summary

Improves FileHandler in lambda-rs-logging to keep the log file open via a Mutex<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

  • Refactored FileHandler to open the log file once during construction and store a Mutex<io::BufWriter<File>> for writes
  • Removed the file: String field that stored the path for repeated opens
  • Updated the emit method to use proper mutex poisoning recovery with into_inner()
  • Used std::mem::take to efficiently clear the buffer while holding the lock
  • Added explicit writer.flush() after writing to ensure data is persisted
  • Added a new test file_handler_does_not_reopen_between_flushes that verifies the handler continues writing to the original file descriptor after the file path is renamed

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • Feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (updates to docs, specs, tutorials, or comments)
  • Refactor (code change that neither fixes a bug nor adds a feature)
  • Performance (change that improves performance)
  • Test (adding or updating tests)
  • Build/CI (changes to build process or CI configuration)

Affected Crates

  • lambda-rs
  • lambda-rs-platform
  • lambda-rs-args
  • lambda-rs-logging
  • Other:

Checklist

  • Code follows the repository style guidelines (cargo +nightly fmt --all)
  • Code passes clippy (cargo clippy --workspace --all-targets -- -D warnings)
  • Tests pass (cargo test --workspace)
  • New code includes appropriate documentation
  • Public API changes are documented
  • Breaking changes are noted in this PR description

Testing

Commands run:

cargo build --workspace
cargo test --workspace
cargo test -p lambda-rs-logging -- --nocapture

Manual verification steps (if applicable):

  1. Run the new test file_handler_does_not_reopen_between_flushes to verify that the file handler maintains the same file descriptor across flushes
  2. Verify logging still works correctly in examples that use file logging

Screenshots/Recordings

N/A - No visual changes.

Platform Testing

  • macOS
  • Windows
  • Linux

Additional Notes

  • The new test is gated with #[cfg(unix)] because it relies on Unix file semantics where renaming a file while it's open allows continued writes to the original file descriptor
  • This change is particularly useful for log rotation scenarios where external tools may rename log files while the application is running
  • The mutex poisoning recovery pattern (into_inner()) ensures the handler remains functional even if a previous thread panicked while holding the lock

@vmarcella vmarcella added the enhancement New feature or request label Jan 23, 2026
@vmarcella vmarcella requested a review from Copilot January 23, 2026 23:18
Copy link

Copilot AI left a comment

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 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: String path field in FileHandler with a Mutex<io::BufWriter<File>> that is opened once at construction.
  • Update FileHandler::log to buffer messages with poisoning-tolerant mutex handling, batch every 10 messages, then write through the persistent BufWriter and flush.
  • Add a Unix-only test file_handler_does_not_reopen_between_flushes to 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.

@vmarcella vmarcella merged commit 5b5e501 into main Jan 23, 2026
11 checks passed
@vmarcella vmarcella deleted the vmarcella/log-file branch January 23, 2026 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Keep log file open in FileHandler to avoid repeated syscalls

2 participants