-
Notifications
You must be signed in to change notification settings - Fork 36
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
fix: Don't trigger replay for duplicates #837
Conversation
patriknw
commented
Mar 30, 2023
- duplicates, less seqNr, have been processed and shouldn't trigger replay
- need more details from R2dbcOffsetStore.isAccepted so changed return value to adt
* duplicates, less seqNr, have been processed and shouldn't trigger replay * need more details from R2dbcOffsetStore.isAccepted so changed return value to adt
1eebb60
to
9df4661
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, as far I can tell.
Have some comments on naming though.
object IsAcceptedResult { | ||
case object Accepted extends IsAcceptedResult | ||
case object Duplicate extends IsAcceptedResult | ||
case object RejectedSeqNr extends IsAcceptedResult | ||
case object RejectedBacktrackingSeqNr extends IsAcceptedResult |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find it confusing that the trait is called IsAcceptedResult
, but it can be a rejection as well.
Maybe better to call is Validation
instead?
object Validation {
case object Accepted extends Validation
case object Duplicate extends Validation
case object RejectedSeqNr extends Validation
case object RejectedBacktrackingSeqNr extends Validation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
alright, I'll follow up with some renaming in separate PR
@@ -656,14 +672,17 @@ private[projection] class R2dbcOffsetStore( | |||
* Completed with `true` if accepted, otherwise `false` if rejected or failed with `EnvelopeRejected` if | |||
* backtracking envelope is rejected. | |||
*/ | |||
def isAccepted[Envelope](envelope: Envelope): Future[Boolean] = { | |||
def isAccepted[Envelope](envelope: Envelope): Future[IsAcceptedResult] = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following previous comment...
def validate[Envelope](envelope: Envelope): Future[Validation]
Also, scaladoc needs to be adapted. Not a boolean anymore.
test failures are in unrelated modules |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After the fact LGTM