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

Improve --log-size-max handling #211

Open
rhatdan opened this issue Oct 6, 2020 · 5 comments
Open

Improve --log-size-max handling #211

rhatdan opened this issue Oct 6, 2020 · 5 comments

Comments

@rhatdan
Copy link
Member

rhatdan commented Oct 6, 2020

Currently conmon just truncates the log file when max is reached, this means we loose all of the data.

I think we should allow for a backup file. So when log-size-max is reached we rename the existing file to a .1 version. Now when the user reads the full log file they would read the .1 file first and then the main log file. This would mean we would always have log-size-max data available, after we fill it up. If .1 already existed, we would loose that data.

Since user asked for log-size-max, we should probably do the rename at log-size-max/2 to grant their wishes.

@haircommander @giuseppe @vrothberg @mheon WDYT?

@rhatdan
Copy link
Member Author

rhatdan commented Oct 6, 2020

Docker also handles an addition field to indicate the number of backup files, I believe.

@vrothberg
Copy link
Member

Docker also handles an addition field to indicate the number of backup files, I believe.

That answers the immediate question I had in mind: How do we set a limit to protect from running out of disk space?

SGTM

@rhatdan rhatdan changed the title Imporve --log-size-max handling Improve --log-size-max handling Oct 7, 2020
@haircommander
Copy link
Collaborator

the approach sounds good, as long as the behaviour is gated by a flag. In the k8s world, kubelet handles the renaming of the files, so we should preserve existing behaviour unless otherwise specified

@haircommander
Copy link
Collaborator

another thing to think about is how to handle the metadata. does docker truncate based on file size or based on size of log line? regardless, we'll have differing behaviour until conmon actually supports json-file

@exekias
Copy link

exekias commented Mar 26, 2021

++ on this issue, we (Filebeat dev here) are seeing more issues coming from k8s users using alternative runtimes, like CRI-o. Having truncate as the mechanism for rotation makes it impossible to guarantee delivery of all log messages. A rename + create approach like the one describe in this issue allows agents to keep reading both the old and the new file after a rotation event.

does docker truncate based on file size or based on size of log line?

Docker does this based on size, which probably makes sense, as it allows system administrators to allocate disk for logs in a predictable way, regardless of log line lengths.

Also a clarification about Docker rotation strategy: Docker doesn't truncate files, this is important. It will rename the existing log file and create a new one for the following logs.

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

No branches or pull requests

4 participants