Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The problem happened then buffer's bound stops in between of events with timeuuid like:
...
e3e99ed0-a21d-11e8-b31a-e9435a127f49 // A: last event which put into a buffer
e3e99ed1-a21d-11e8-b31a-e9435a127f49 // B: next one
...
InMemoryReadJournal::eventsByTag::nextFromOffset uses unix timestamp to calculate 'next' event:
case TimeBasedUUID(time) => TimeBasedUUID(UUIDs.startOf(UUIDs.unixTimestamp(time) + 1))
and it skips event B because both of them have same unix timestamp: 1534510989629, and 'next' uuid will be:
e3e9c5e0-a21d-11e8-b31a-e9435a127f49
The difference in nanoseconds:
137538037896290000 for A
137538037896290001 for B
Where it comes from ?
InMemoryAsyncWriteJournal uses following functions to generate timeuuid for an event:
def nowUuid: UUID = UUIDs.timeBased()
def getTimeBasedUUID: TimeBasedUUID = TimeBasedUUID(nowUuid)
def timeBased(): UUID = {
new UUID(makeMSB(UUIDUtil.getCurrentTimestamp()), ClockSeqAndNode)
}
If we will take a look on UUIDUtil.getCurrentTimestamp more closely, we can see following:
public static final AtomicLong lastTimestamp = new AtomicLong(0L);
...
long now = fromUnixTimestamp(System.currentTimeMillis());
long last = lastTimestamp.get();
if (now > last) { ...
} else { ...
long candidate = last + 1;
So if two (or more) events are persisted in same millisecond, nanoseconds will be added to timeuuid. But they are not
taken into account when events are read from a journal.
PS: I also added the test for that scenario, unfortunately test is very depended on timing (performance)
and may NOT fail even with broken implementation.
I was able to choose parameters which gives me like ~100% failure rate.
I mean, the test never passed successfully with original implementation on my box
but I cannot guarantee that for other boxes.
(cherry picked from commit da77910)