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

How to handle first event with unknown sequence number #74

Closed
patriknw opened this issue Oct 26, 2021 · 0 comments · Fixed by #120
Closed

How to handle first event with unknown sequence number #74

patriknw opened this issue Oct 26, 2021 · 0 comments · Fixed by #120
Assignees

Comments

@patriknw
Copy link
Member

We must be careful with accepting events with seqNr > 1 where previous sequence number is not known by the offset store. It could have missed previous sequence number because how timestamp based offset works (see design notes).

Current implementation will therefore reject such event unless it is 3 seconds old (configurable by accept-new-sequence-number-after-age). The idea is that it will be picked up by the backtracking query and then it would be safe since the transaction_timestamp issues would have been resolved.

I don't like that solution. It introduces additional delay for that case (an entity that wasn't active for a long time). How long must the delay be to be safe?

What can we do instead? I'm not sure, but one thought would be to query the journal for the db_timestamp of the previous event. If that is older than the offset store window then it's safe to accept immediately.

@patriknw patriknw self-assigned this Nov 4, 2021
patriknw added a commit that referenced this issue Nov 5, 2021
* When the offset store receives an unknown sequence number
  in `isAccepted` it will try to lookup the timestamp of the
  previous event. If that is older than then time window of
  the offset store it will be accepted, otherwise rejected.
* The reason for rejecting is that it previous event could
  be missing when reading from the tail based on the latest
  timestamp offset.
* Backtracking will pick up the missing event and fill the
  gap.
* This is better than the previous accept-new-sequence-number-after-age
  (3 seconds) because it will not introduce that delay for the
  "normal case" when a new event is emitted and previous event
  was older than the offset store time window.
* Also, previous solution had the the uncertainty about if
  3 seconds would be enough and backtracking.behind-current-time
  can now be increased without adding to the delay mentioned above.
patriknw added a commit that referenced this issue Nov 5, 2021
* When the offset store receives an unknown sequence number
  in `isAccepted` it will try to lookup the timestamp of the
  previous event. If that is older than then time window of
  the offset store it will be accepted, otherwise rejected.
* The reason for rejecting is that it previous event could
  be missing when reading from the tail based on the latest
  timestamp offset.
* Backtracking will pick up the missing event and fill the
  gap.
* This is better than the previous accept-new-sequence-number-after-age
  (3 seconds) because it will not introduce that delay for the
  "normal case" when a new event is emitted and previous event
  was older than the offset store time window.
* Also, previous solution had the the uncertainty about if
  3 seconds would be enough and backtracking.behind-current-time
  can now be increased without adding to the delay mentioned above.
patriknw added a commit that referenced this issue Nov 5, 2021
* When the offset store receives an unknown sequence number
  in `isAccepted` it will try to lookup the timestamp of the
  previous event. If that is older than then time window of
  the offset store it will be accepted, otherwise rejected.
* The reason for rejecting is that it previous event could
  be missing when reading from the tail based on the latest
  timestamp offset.
* Backtracking will pick up the missing event and fill the
  gap.
* This is better than the previous accept-new-sequence-number-after-age
  (3 seconds) because it will not introduce that delay for the
  "normal case" when a new event is emitted and previous event
  was older than the offset store time window.
* Also, previous solution had the the uncertainty about if
  3 seconds would be enough and backtracking.behind-current-time
  can now be increased without adding to the delay mentioned above.
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 a pull request may close this issue.

1 participant