Properly handle in-flight deletes followed by adds in OrderedListState #28171
Conversation
|
Assigning reviewers. If you would like to opt out of this review, comment R: @lostluck added as fallback since no labels match configuration Available commands:
The PR bot will only process comments in the main thread (not review comments). |
| Iterables.filter( | ||
| Iterables.transform(includingAdds, TimestampedValueWithId::getValue), | ||
| tv -> !pendingDeletes.contains(tv.getTimestamp())); | ||
| Iterables.transform(includingAdds, TimestampedValueWithId::getValue); |
There was a problem hiding this comment.
What if a delete was issued after an add? It feels like the timing of overlapping deletes and adds also needs to be considered.
There was a problem hiding this comment.
This case was already correctly handled. When clearRange is called, any pending adds in that range are removed from pendingAdds:
There was a problem hiding this comment.
The last delete will win because of the filter clause at 950 and the last insert will win because of the transform that will merge the add at 975?
|
|
||
| orderedListState.add(TimestampedValue.of("second", Instant.ofEpochMilli(1))); | ||
| orderedListState.add(TimestampedValue.of("third", Instant.ofEpochMilli(2))); | ||
| orderedListState.add(TimestampedValue.of("fourth", Instant.ofEpochMilli(2))); |
There was a problem hiding this comment.
Is it a typo for the "fourth" element - same ts as "third" and gets added later with a different ts?
There was a problem hiding this comment.
no - I delete it and then add it back with a different ts (though one still within the deletion range)
| TimestampedValue.of("fourth", Instant.ofEpochMilli(4)), | ||
| TimestampedValue.of("fifth", Instant.ofEpochMilli(5)), | ||
| TimestampedValue.of("sixth", Instant.ofEpochMilli(5)), | ||
| TimestampedValue.of("seventh", Instant.ofEpochMilli(5)), |
There was a problem hiding this comment.
Side note - would be good to describe the behavior of the list when multiple elements are added with the same timestamp (the fact that it behaves as a multimap and ordering guarantees for the entries under the same key, if any).
reuvenlax
force-pushed
the
ordered_list_state_consistency
branch
from
57a8ccf to
f2f5bf6
Compare
|
Run Java PreCommit |
|
Reminder, please take a look at this pr: @lostluck |
No description provided.