Modify clean_removed handling #3827
Conversation
ruflin
added
Filebeat
ruflin
force-pushed
the
improved-clean-removed
branch
2 times, most recently
from
1c223e5
to
d774da1
Compare
ruflin
added
review
and removed
in progress
|
@elastic/beats Would be good to hear your thoughts on this change. |
| @@ -94,24 +94,35 @@ func (l *Log) Run() { | |||
| if l.config.CleanRemoved { | |||
| for _, state := range l.Prospector.states.GetStates() { | |||
| // os.Stat will return an error in case the file does not exist | |||
| _, err := os.Stat(state.Source) | |||
| stat, err := os.Stat(state.Source) | |||
| if err != nil { | |||
There was a problem hiding this comment.
Should we be more strict here and check vs os.PathError?
There was a problem hiding this comment.
Good idea. What should we do in all other cases? Log an error and not remove the state?
There was a problem hiding this comment.
Yea that's probably the best we can do. os.PathError is all that can return right now but you never now..
| // Only clean up files where state is Finished | ||
| if state.Finished { | ||
| state.TTL = 0 | ||
| err := l.Prospector.updateState(input.NewEvent(state)) |
There was a problem hiding this comment.
We could return here and check the error in the caller, and also remove the else
There was a problem hiding this comment.
I changed it to return directly if state is not finished but I kept the error handling local.
Previously if a file could be found under the same name as a state, the state was not removed. But this could have been also an other file. With this change also the file itself is compared and if it is not the same file, the state will be removed. This has the affect that in case a file is renamed after monitoring the file finished, the state will be removed. In most cases this should not have any side affect. The positive effect of this change is that there will be less left over states in the registry. Closes elastic#3789
ruflin
force-pushed
the
improved-clean-removed
branch
from
d774da1
to
f8f540b
Compare
| // Only clean up files where state is Finished | ||
| if state.Finished { | ||
| state.TTL = 0 | ||
| err := l.Prospector.updateState(input.NewEvent(state)) |
There was a problem hiding this comment.
I changed it to return directly if state is not finished but I kept the error handling local.
| @@ -94,24 +94,35 @@ func (l *Log) Run() { | |||
| if l.config.CleanRemoved { | |||
| for _, state := range l.Prospector.states.GetStates() { | |||
| // os.Stat will return an error in case the file does not exist | |||
| _, err := os.Stat(state.Source) | |||
| stat, err := os.Stat(state.Source) | |||
| if err != nil { | |||
Previously if a file could be found under the same name as a state, the state was not removed. But this could have been also an other file. With this change also the file itself is compared and if it is not the same file, the state will be removed. This has the affect that in case a file is renamed after monitoring the file finished, the state will be removed. In most cases this should not have any side affect.
The positive effect of this change is that there will be less left over states in the registry.
Closes elastic#3789