Ignore duplicate state entries #2792
Conversation
tsg
added
review
Filebeat
tsg
force-pushed
the
ignore_duplicate_state_entries
branch
2 times, most recently
from
145ef2d to
1bbbfe8
Compare
| @@ -176,6 +168,32 @@ func (r *Registrar) loadAndConvertOldState(f *os.File) bool { | |||
| return true | |||
| } | |||
|
|
|||
| func convertOldStates(oldStates map[string]file.State) []file.State { | |||
There was a problem hiding this comment.
The logic you have looks correct. What do you think about using a map with struct key to do the deduplication?
states := map[file.StateOS]file.State{}
for _, state := range oldStates {
// If there is a duplicate, use the one with the larger offset.
existingState, found := states[state.FileStateOS]
if found && existingState.Offset > state.Offset {
continue
}
// Makes timestamp the time of migration, as this is the best guess.
state.Timestamp = time.Now()
states[state.FileStateOS] = state
}
There was a problem hiding this comment.
Hmm, that would simpler and would work, but bypassing the IsSame() method feels risky for future StateOS implementations (note that the structures is platform specific, so we could see variations).
I'm not really worried about the O(n^2) aspect here, since this is done only once and with an N up to tens of thousands at most.
There was a problem hiding this comment.
Good point, that might be brittle and break if there are any changes to StateOS.
When upgrading the registry file from 1.x versions, make sure the new state doesn't contain any duplicate inodes, as these will cause an invalid state. Fixes elastic#2784.
tsg
force-pushed
the
ignore_duplicate_state_entries
branch
from
1bbbfe8 to
f402b69
Compare
|
jenkins, retest it |
When upgrading the registry file from 1.x versions, make sure the new state doesn't contain any duplicate inodes, as these will cause an invalid state. Fixes elastic#2784.
|
LGTM. Thx for the fix. I also don't worry here about the complexity here because it only happens once. And using IsSame should be "future proof". |
…elastic#2797) When upgrading the registry file from 1.x versions, make sure the new state doesn't contain any duplicate inodes, as these will cause an invalid state. Fixes elastic#2784.
When upgrading the registry file from 1.x versions, make sure the new state doesn't contain any duplicate inodes, as these will cause an invalid state. Fixes #2784.