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

MySqlStreamingChangeEventSource: Convert non-full rows to full rows #5348

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

shanielh
Copy link

@shanielh shanielh commented Mar 4, 2024

This should fix DBZ-7591

Copy link

github-actions bot commented Mar 4, 2024

Welcome as a new contributor to Debezium, @shanielh. Reviewers, please add missing author name(s) and alias name(s) to the COPYRIGHT.txt and Aliases.txt respectively.

Copy link

github-actions bot commented Mar 4, 2024

Hi @shanielh, thanks for your contribution. Please prefix the commit message(s) with the DBZ-xxx JIRA issue key.

@shanielh shanielh force-pushed the DBZ-7591 branch 3 times, most recently from 735e5d6 to e9e4ac1 Compare March 4, 2024 07:25
@Naros
Copy link
Member

Naros commented Mar 4, 2024

Hi @shanielh the issue reports this as affecting MariaDB but I do not see anything in the code that guards this change specifically to MariaDB. In addition, could this be due to the binlog row format configuration?

@shanielh
Copy link
Author

shanielh commented Mar 4, 2024

Hi @shanielh the issue reports this as affecting MariaDB but I do not see anything in the code that guards this change specifically to MariaDB. In addition, could this be due to the binlog row format configuration?

Hi @Naros, thanks for the fast response. I think this might happen in MySQL too since it's related to the MySQL binlog wire format, it might be related to the configuration but I couldn't find anything related to it. I'll try look for more information tomorrow.

@Naros
Copy link
Member

Naros commented Mar 7, 2024

Hi @shanielh can you please add some tests for this change?

@shanielh
Copy link
Author

shanielh commented Mar 9, 2024

Hi @Naros, sure. I'll try to add tests using minimal row binlog setting. I hope to do that by Monday.

@Naros
Copy link
Member

Naros commented Mar 20, 2024

Hi @shanielh, just wanting to follow-up and see if you have any questions or issues adding the test.

@shanielh
Copy link
Author

Hi @shanielh, just wanting to follow-up and see if you have any questions or issues adding the test.

Yeah actually I do, should I change the format of the row to minimal, or should I somehow create another instance of a MySQL in which I'll change the configuration?

@jpechane
Copy link
Contributor

@shanielh I don't think it is necessary to create another long-running instance

  1. It seems to me tha binlog format is a dynamic setting. So I'd first recommend to try set the different format as part of the test class setup and run tests against it. It is necessary to validate that the non-full rows were read and processed. After the test it is necessary to make sure that the format is switched back to the full one.
  2. If option 1) will show to be non-feasible (like breaking the rest of test suite etc.) then the other option is to write the test to use test containers to manage its own private MySQL with requested config and run test against it.

@jpechane jpechane marked this pull request as draft March 26, 2024 06:43
@jpechane jpechane added the pending tests Tests running locally or internal CI, requires passing before merging. label Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending tests Tests running locally or internal CI, requires passing before merging.
Projects
None yet
3 participants