Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

issues/55 fixed. #64

Open
wants to merge 1 commit into
base: 2.5.0.0
Choose a base branch
from

Conversation

LoneEngineer
Copy link

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)

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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant