Disable backlog on container start/restart. #252
Conversation
|
@roman-vynar: it was previously mentioned in #187 and/or #201 that backlogs are needed on start because the container might have output logs before logspout has had a chance to connect. That's why #201 implements this as an env. var flag. |
|
@ebr then 2 things:
So the flag does not solve the above 2 issues. I would say it is better to have an option to send existing logs somehow else if needed on start. P.S. offtopic: hah thanks, have you been there? 😆 |
|
@roman-vynar i agree this PR fixes the issue, though in my opinion it's better to provide a runtime flag, in case one relies on being certain that no log lines are lost from the beginning of the container start (and is prepared to deal with expensive sending & duplicates). But this is up to maintainers to decide, of course. offtop: i grew up there :) |
| switch event.Status { | ||
| case "start", "restart": | ||
| go p.pumpLogs(event, true, inactivityTimeout) | ||
| go p.pumpLogs(event, false, inactivityTimeout) |
There was a problem hiding this comment.
Perhaps we should only set this to false on restart and have a true case on start. Thoughts?
There was a problem hiding this comment.
Just saw your previous conversation. I think I'd like to keep backlog set to true on start for now. That at least then doesn't break the current expectation.
There was a problem hiding this comment.
We can create an option then, if someone needs to re-send them.
Leaving it on start will not help as you can stop/start and it is all sent again.
There was a problem hiding this comment.
An option is fine with me.
There was a problem hiding this comment.
@roman-vynar is that something you're able to add here? if not, no worries but wanted to check before I run with this.
|
@michaelshobbs #201 works great in my fork (just because somebody already did the work to make it configurable). My vote would be to merge that one. |
|
I concur. I'll go with #201 unless @roman-vynar has any objection |
|
@michaelshobbs thanks 👍 |
4 participants
Addresses #187, #234
Sending the backlog does not really make sense on container restart or stop/start. Usually, you have those logs already and do not need duplicate ones.