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

tailer: don't load entire file into memory, stream it out bits at a time #48

Merged
merged 1 commit into from
Oct 28, 2018

Conversation

virtuald
Copy link
Contributor

Thanks!

@fstab
Copy link
Owner

fstab commented Oct 24, 2018

Maybe we could solve this issue together with #47.

Some background info for other people finding this on Google

When does grok_exporter read an entire log file into memory?

grok_exporter usually does not load the entire file into memory. It buffers only as many lines as are waiting to be processed. The number of buffered lines should be very low unless the system is under extreme load. If you are unsure, you can look at grok_exporter's built-in metrics to monitor the buffer size on your system. It should be reasonably low.

However, there is one exception when grok_exporter does in fact read the entire file: When the readall flag is set in the configuration. As the documentation says, this is intended for debugging when you want to test your metrics against an existing log file. It is not intended for production, because if you have a conter metric counting warnings in your log, you will count them again whenever you restart grok_exporter, so you will end up with wrong numbers.

Why are lines buffered?

If the system is under heavy load, processing grok patterns might temporarily take longer than reading new log lines. This means that the reader lags behind, and in some load tests this resulted in lost file system events, so grok_exporter lost track of moved / truncated / etc. log files.

The buffered line reader solves this problem by decoupling the line reader and the grok pattern processor. The line reader runs in one goroutine and keeps reading lines and handing them over to the buffer. The grok pattern processor runs in another goroutine and takes the lines off the buffer. That way, the line reader can temporarily read lines at a faster rate than they are processed.

What does this PR change?

This PR couples the line reader and the pattern processing in a way that the line reader has to wait for a line to be fully processed before the next line is read. This is similar to a very early implementation of grok_exporter, and I fear this might result in lost events under extreme system load.

What can we do?

I have two thoughts on this:

  • I understand that if you have a very large log file and test it with the readall configuration, you don't want to read the entire file into memory. Maybe we can combine this with Tailer: add option to start tailing at specified seek offset #47 and provide a read_last_n_lines configuration where the last n lines are processed if the logfile exists. This could be handy for testing large log files.
  • In production when readall is false, I would like to keep a buffer that allows grok_exporter to temporarily read lines faster than they are processed. We could implement a maximum size for the buffer, but I don't think this is needed in practice, because the buffer tends to be very small.

@virtuald
Copy link
Contributor Author

If you wanted to add a buffer, you could just make the lines channel able to hold N entries via make(chan string, N), and that would be roughly equivalent?

@fstab
Copy link
Owner

fstab commented Oct 25, 2018

It seems I was a little tired when I wrote my comment yesterday, I confused the buffered tailer (bufferedTailer.go) and the buffered line reader (linereader.go). It's actually the buffered tailer's job to decouple reading lines from processing matches. The line reader has nothing to do with it.

Looking at it again it really seems unnecessary to buffer more than one line in the line reader, because there is already a buffer in the buffered tailer. I will look at this again later today...

Sorry for the confusion.

@fstab
Copy link
Owner

fstab commented Oct 26, 2018

It seems that linereader_test.go needs to be adapted to the new behavior.

@fstab fstab merged commit 261b831 into fstab:master Oct 28, 2018
@fstab
Copy link
Owner

fstab commented Oct 28, 2018

Merged it, because we don't need one buffer in bufferedTailer.go and another buffer in linereader.go. Note however that the buffered tailer might still read the entire file into memory if system load is high and processing the lines takes long.

Thanks for the PR!

I think I will rename the buffered line reader, because it doesn't buffer lines anymore...

@virtuald virtuald deleted the streaming branch October 30, 2018 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants