Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

StopAtEOF is racy #67

Open
tgummerer opened this issue Apr 3, 2024 · 0 comments · May be fixed by #70
Open

StopAtEOF is racy #67

tgummerer opened this issue Apr 3, 2024 · 0 comments · May be fixed by #70
Labels
bug Something isn't working

Comments

@tgummerer
Copy link

If you sent an issue/PR to hpcloud, you can have a look at
this meta issue tracking issues
and PRs of the dormant upstream. However, as the code bases diverge the
issue may have been solved already.

Describe the bug
The StopAtEOF function can sometimes stop tailing the file before we actually encounter an EOF. In particular, when using the polling function for checking for file changes, we might select this tail.Dying channel here first before the poller gets another chance to notify us of new changes, which I believe makes us then return here, even though we should probably be waiting for another poll cycle.

Expected behaviour
The library checks again for new changes in the file after StopAtEOF is reached.

To Reproduce
This is a bit awkward to reproduce, since it's based on racyness. However tailing a file that's being written to in short intervals, and calling StopAtEOF might not return all data before exiting.

Additional context
I'd be happy to contribute a PR for this, but probably won't have the time to do so before next week.

@tgummerer tgummerer added the bug Something isn't working label Apr 3, 2024
github-merge-queue bot pushed a commit to pulumi/pulumi that referenced this issue Apr 3, 2024
The library we're using unfortunately has a race condition that's
especially worse when using polling to check for new changes of the
file. Avoid this racyness by sleeping for a bit for now, but really this
should be fixed upstream ideally.

I opened a upstream issue for this in
nxadm/tail#67. I think we should merge this in
the meantime, as it makes this work, even though it's very hacky.

I tested this in #15853, where I
could no longer reproduce the failure we've seen before.

Fixes #15659
Fixes #15235
@tgummerer tgummerer linked a pull request Apr 8, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant