-
Notifications
You must be signed in to change notification settings - Fork 2.5k
[HUDI-9691] Migrate MysqlDebeziumPayload to merge mode #13727
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
[HUDI-9691] Migrate MysqlDebeziumPayload to merge mode #13727
Conversation
80fa4f1 to
3a7726d
Compare
| if (!params.contains(HoodieWriteConfig.PRECOMBINE_FIELD_NAME.key()) && tableConfig.getPreCombineFieldsStr.isPresent) { | ||
| missingWriteConfigs ++= Map(HoodieWriteConfig.PRECOMBINE_FIELD_NAME.key -> tableConfig.getPreCombineFieldsStr.orElse(null)) | ||
| } |
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 was one issue which I was facing. Basically when precombine config is not configured, we add it using table config.
Write config is therefore pointing to older table config but the table config is updated to newer config after upgrade. In order to avoid such a scenario removed addition of write config here.
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.
With the deprecation of write config in #13718, this should be further streamlined
...i-client-common/src/main/java/org/apache/hudi/table/upgrade/NineToEightDowngradeHandler.java
Outdated
Show resolved
Hide resolved
...tasource/hudi-spark/src/test/scala/org/apache/hudi/functional/RecordLevelIndexTestBase.scala
Outdated
Show resolved
Hide resolved
yihua
left a comment
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 overall, made a few minor revisions. Let's fix test failures.
yihua
left a comment
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
| } | ||
|
|
||
| @Test | ||
| def testUpgradeDowngradeMySqlDebeziumPayload(): Unit = { |
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.
nit: we might want to decouple the upgrade and downgrade tests from RecordLevelIndexTestBase, which can be done separately. @lokeshj1703 could you file a JIRA to track this?
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.
Co-authored-by: Lokesh Jain <ljain@192.168.1.21> Co-authored-by: Lokesh Jain <ljain@192.168.0.234> Co-authored-by: Y Ethan Guo <ethan.guoyihua@gmail.com>
Change Logs
Migrating MySqlDebeziumPayload to merge mode.
Pending work:
Adding tests to TestPayloadDeprecationFlow for MySqlDebeziumPayload
Impact
MySqlDebeziumPayload in V9 will be using merge modes w/ multiple ordering fields.
Risk level (write none, low medium or high below)
medium
Documentation Update
Describe any necessary documentation update if there is any new feature, config, or user-facing change. If not, put "none".
ticket number here and follow the instruction to make
changes to the website.
Contributor's checklist