-
Notifications
You must be signed in to change notification settings - Fork 2.5k
fix: fix downgrade to not delete unintended partitions in MDT #14162
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: fix downgrade to not delete unintended partitions in MDT #14162
Conversation
|
PR desc please |
| + " or when using a custom Hoodie Concat Handle Implementation controlled by the config " + CONCAT_HANDLE_CLASS_NAME.key() | ||
| + ", enabling this config results in fallback to the default implementations if instantiation of the custom implementation fails"); | ||
|
|
||
| public static final ConfigProperty<Boolean> AUTO_DETECT_AND_DELETE_MDT_PARTITIONS = ConfigProperty |
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.
this is not end user config. we should remove this. we just need it as an additional argument to method calls. those originating from downgrade, dropIndex apis should set the argument value to false.
rest of the callers should leave it as true
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.
code path causing the issue is getting called from multiple places in the Upgrade/Downgrade code path which is making the approach of using additional argument to method calls very cumbersome.
For e.g rollbackFailedWritesAndCompact during downgrade also calls tableMetadata.getMetadataWriter which is the method that's causing the issue
Line 402 in e24a4b7
| UpgradeDowngradeUtils.rollbackFailedWritesAndCompact( |
| val cowTableName = "hudi-v9-table-mdt-validation-cow" | ||
| val morTableName = "hudi-v9-table-mdt-validation-mor" | ||
|
|
||
| val cowTableBasePath = basePath + "/" + cowTableName |
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.
can you help me remind why do we need a test fixture for this?
a functional test should suffice right?
we can create a v9 table and then downgrade to v8. and just validate the MDT partitions
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.
Makes sense, fixed the test
|
also, why the patch is in draft state? |
|
also, looks like you need to check CI failures as well. |
Made it ready for review, was working on UT |
8f3c2c2 to
11b94c6
Compare
...-datasource/hudi-spark/src/test/java/org/apache/hudi/table/upgrade/TestUpgradeDowngrade.java
Show resolved
Hide resolved
...-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/UpgradeDowngrade.java
Outdated
Show resolved
Hide resolved
104cfdb to
64b6e81
Compare
|
reviewed, pushed out a commit to address my own feedback. |
hudi-client/hudi-java-client/src/main/java/org/apache/hudi/table/HoodieJavaTable.java
Show resolved
Hide resolved
|
pushed another commit to remove the new write config and use arguments. |
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/HoodieTable.java
Outdated
Show resolved
Hide resolved
--------- Co-authored-by: Vamsi <vamsi@onehouse.ai> Co-authored-by: sivabalan <n.siva.b@gmail.com>

Describe the issue this Pull Request addresses
Fixes an issue where MDT partitions were being unintentionally deleted during downgrade operations
Summary and Changelog
Downgrade operation doesn't drop any partitions.
Impact
MDT partitions are not deleted during downgrade/upgrade operation
Risk Level
low
Documentation Update
NA
Contributor's checklist