-
Notifications
You must be signed in to change notification settings - Fork 9
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
[#148] Modifications to add, poll and peek in Merger #150
[#148] Modifications to add, poll and peek in Merger #150
Conversation
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.
Will there be a new PR for asserts?
It is now added as a part of the same PR. |
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.
For testing asserts, it should be sufficient to add two messages out of order in the same tablet? That should fire the assert (that you may add) in setTabletSafeTime
.
Two messages that have the same commitTime but out of order like COMMIT followed by BEGIN should trigger the other assert in add
.
src/main/java/io/debezium/connector/yugabytedb/consistent/Merger.java
Outdated
Show resolved
Hide resolved
src/test/java/io/debezium/connector/yugabytedb/consistent/MessageTest.java
Show resolved
Hide resolved
…tor-yugabytedb into cdc_consistency
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.
Couple of minor comments. Ack on taking up the config parameter in a different PR
This diff adds the following modifications to the above mentioned functions:
add()
now updates the tablet safetime on adding every messagepeek()
now returns a message if the commit time of the front message is less than or equal to the stream safetime.AssertionError
and will cause the connector to stop now.