search

hpcloud / tail

Go package for reading from continously updated files (tail -f)
MIT License
2.7k stars 502 forks source link

Don’t block on write if we’ve been killed #163

Open paulsc opened 4 years ago

paulsc commented 4 years ago

Hi - I've been debugging this issue and fixed it with the changes in this pull request. I'm a bit new to go - so forgive me If I misunderstand, but here's what happened.

Quick context first: I am using hpcloud/tail in my component, called "CsvTail" which tails a file, parses the CSV records in that file, and only stops when it encounters the NUL byte at the beginning of a line. The CSV files are streamed and written by another service, which uses the NUL byte to indicate the end of the file.

The code would sometimes deadlock when calling tail.Stop(). Stop() internally calls tail.Wait(), which waits for the tomb to be declared as dead.

What happens is that another line comes in after my read of the NUL byte, but before my Stop() call, causing the tail library to block on this line.

Tail would never reach the end of tailFileSync() where it checks for tail.Dying(), returns, and sets the tomb as Done().

My fix is to add a select and "cancel" the write if the tomb enters dying state.

Not sure if this is right though let me know if I'm misunderstanding or misusing the library.

Best,

Paul

nxadm commented 4 years ago

Sadly, this repo seems dormant for more than 1,5 year. I forked it because I use it in my code. I integrated the PRs waiting merging and refreshed the code. A PR was recently merged that seems to address the same issue: https://github.com/nxadm/tail/pull/7

There were some other fixes that prevent not showing the last line in the buffer while waiting on a newline. If you have the time, please do review the changes.

paulsc commented 4 years ago

Ah ok. Thanks for update. I wish the owner of the hpcloud repo would add a note in the readme to point to yours!

paulsc commented 4 years ago

FYI I tried switching to your fork @nxadm, but encountered an issue where I would get incomplete lines sometimes so had to switch back. I wish I could give more detail but don't have time to debug further right now.

nxadm commented 4 years ago

FYI I tried switching to your fork @nxadm, but encountered an issue where I would get incomplete lines sometimes so had to switch back. I wish I could give more detail but don't have time to debug further right now.

After the merging of the PRs all tests passed. Once you have the time I would love know which kind of logs trigger this error (e.g. what is a incomplete line). It may be related to the fix of the bug where the last line of a log wasn't sent until new messages arrived, e.g. because this last line didn't have a new line). Feel free to contact me once you have the time.

nxadm commented 3 years ago

Reviewing the code, I saw the change you proposed is already the nxadm/tail repo, probably by another PR. If you find the time, please test if the problem still occurs, and if it does please reopen an issue at https://github.com/nxadm/tail/issues (if possible with the data file and a small test).