fix: Error on rotated file if new file takes time to appear #22
Conversation
This file contains 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
The test for memory consumption creates a large log file. Previously, it was creating it in the local test directory which clutters up the project dir and could potentially get committed. This change restructures the test to use a temp directory as given by the OS. Semver: patch
|
I'll look at this today. |
darinspivey
force-pushed
the
darinspivey/issue18
branch
from
b6978bb to
dc4dd2c
Compare
jorgebay
reviewed
Jun 1, 2021
There was a problem hiding this comment.
It's looking good, I've added a minor remark below.
I think there's a small race condition on _readRemainderFromFileHandle() that might be more likely to appear now: we are nullifying the file handle field after async reading/closing of the file handle (other events/timers can occur in the meantime). Instead, we could store the reference to the file handle locally, set the property to null and then do the async ops.
jorgebay
reviewed
Jun 1, 2021
darinspivey
force-pushed
the
darinspivey/issue18
branch
6 times, most recently
from
d0d8cb4 to
652e0bd
Compare
mdeltito
previously approved these changes
Jun 1, 2021
When a file's inode changes, or the `stat` call throws causes tail-file to read the remainder of the contents via a FileHandle. If the file disappears, but takes a bit to re-appear, `_readRemainderFromFileHandle()` is erroneously called twice; once for the `ENOENT` from the disappearing file, and once when the new file causes the inode to change. This commit fixes a bug where the FH is nullified after the first call, and thus throws on the subsequent call. Semver: patch Fixes: #18
darinspivey
force-pushed
the
darinspivey/issue18
branch
from
652e0bd to
3576bd4
Compare
mdeltito
approved these changes
Jun 1, 2021
jorgebay
approved these changes
Jun 2, 2021
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.
fix: Error on rotated file if new file takes time to appear
When a file's inode changes, or the
statcall throwscauses tail-file to read the remainder of the contents
via a FileHandle. If the file disappears, but takes a bit
to re-appear,
_readRemainderFromFileHandle()iserroneously called twice; once for the
ENOENTfrom thedisappearing file, and once when the new file causes the
inode to change. This commit fixes a bug where the FH is
nullified after the first call, and thus throws on the
subsequent call.
Semver: patch
Fixes: #18
refactor: memory-usage tests should use a temp directory
The test for memory consumption creates a large log file.
Previously, it was creating it in the local test directory
which clutters up the project dir and could potentially
get committed. This change restructures the test to use
a temp directory as given by the OS.
Semver: patch
deps: tap@15.0.9
deps: eslint-config-logdna@5.1.0
deps: eslint@7.27.0