-
Notifications
You must be signed in to change notification settings - Fork 34
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
DBZ-7293 Fallback to old schema in case of race condition between ROW and FIELD #175
Conversation
Hi @s-gelazevicius, thanks for your contribution. Please prefix the commit message(s) with the DBZ-xxx JIRA issue key. |
Welcome as a new contributor to Debezium, @s-gelazevicius. Reviewers, please add missing author name(s) and alias name(s) to the COPYRIGHT.txt and Aliases.txt respectively. |
a8fc5c8
to
185f44e
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.
Thanks for the PR. It looks good as a start.
Please make sure the code is properly formatted (run the mvn install
locally).
I recommend to change the naming - instead of backup it should be delayed.
Also I belive the schema instance should be passed so it is visible that there migh be discrepancy in the schema processing.
Also please make sure the followinf scenarios are tested
- field added
- field removed
- field type changed
* the old schema version in case row length mismatches between Row and Table in schema cache. | ||
* <a href="https://vitess.io/docs/18.0/reference/vreplication/internal/tracker/#caveat">...</a> | ||
*/ | ||
private List<Column> resolveColumns(Row row, Table table, Boolean fromBackup) { |
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.
private List<Column> resolveColumns(Row row, Table table, Boolean fromBackup) { | |
private List<Column> resolveColumns(Row row, Table table, boolean fromBackup) { |
public VStreamOutputMessageDecoder(VitessDatabaseSchema schema) { | ||
this.schema = schema; | ||
// Schema can be null. See: VitessConnector.validateConnection | ||
if (schema != null) this.schemaBackup = schema.clone(); |
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.
There should be no cloning, both schema
and delayedSchema
should be passed from the constructor so it is apparent outside that two instances are needed.
@@ -47,8 +45,13 @@ public class VStreamOutputMessageDecoder implements MessageDecoder { | |||
|
|||
private final VitessDatabaseSchema schema; | |||
|
|||
private final VitessDatabaseSchema schemaBackup; |
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.
private final VitessDatabaseSchema schemaBackup; | |
private final VitessDatabaseSchema delayedSchema; |
This doesn't fix race condition fully, it's still not handled in |
Sometimes due to race condition in Vitess, ROW and TYPE messages can be mixed up. Adding a fallback to the old schema version in case row length mismatches between Row and Table in schema cache.
https://vitess.io/docs/18.0/reference/vreplication/internal/tracker/#caveat
Only best-effort versioning can be provided due to races between DDLs and DMLs