-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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-3743] Support DELETE_PARTITION for metadata table #5169
Conversation
f13c723
to
25b886d
Compare
...park-client/src/main/java/org/apache/hudi/metadata/SparkHoodieBackedTableMetadataWriter.java
Show resolved
Hide resolved
...park-client/src/main/java/org/apache/hudi/metadata/SparkHoodieBackedTableMetadataWriter.java
Outdated
Show resolved
Hide resolved
LGTM, In addition, I want to wait for #4489 to merge in and then merge this? |
Got it. I'll wait for that to land first. |
...park-client/src/main/java/org/apache/hudi/metadata/SparkHoodieBackedTableMetadataWriter.java
Outdated
Show resolved
Hide resolved
...ent/hudi-client-common/src/main/java/org/apache/hudi/metadata/HoodieTableMetadataWriter.java
Outdated
Show resolved
Hide resolved
...i-spark-client/src/test/java/org/apache/hudi/client/functional/TestHoodieBackedMetadata.java
Outdated
Show resolved
Hide resolved
...i-spark-client/src/test/java/org/apache/hudi/client/functional/TestHoodieBackedMetadata.java
Show resolved
Hide resolved
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
0912c87
to
dfe7b9c
Compare
...source/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/TestAlterTableDropPartition.scala
Outdated
Show resolved
Hide resolved
Minor checkstyle fix
338c69a
to
6c89a72
Compare
try { | ||
// Because the partition of BaseTableMetadata has been deleted, | ||
// all partition information can only be obtained from FileSystemBackedTableMetadata. | ||
FileSystemBackedTableMetadata fsBackedTableMetadata = new FileSystemBackedTableMetadata(context, |
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.
#8384) - Looks like when we fallback to full partition cleaning in clean planner, we do FS based listing even though metadata is enabled. It was added in #5169 mainly due to how delete_partition was designed back then. Later delete_partition logic evolved and now we should be good to make this metadata based if applicable.
apache#8384) - Looks like when we fallback to full partition cleaning in clean planner, we do FS based listing even though metadata is enabled. It was added in apache#5169 mainly due to how delete_partition was designed back then. Later delete_partition logic evolved and now we should be good to make this metadata based if applicable.
apache#8384) - Looks like when we fallback to full partition cleaning in clean planner, we do FS based listing even though metadata is enabled. It was added in apache#5169 mainly due to how delete_partition was designed back then. Later delete_partition logic evolved and now we should be good to make this metadata based if applicable.
What is the purpose of the pull request
In order to drop any metadata partition (index), we can reuse the DELETE_PARTITION operation in metadata table. Subsequent to this, we can support drop index (with table config update) for async metadata indexer.
Brief change log
HoodieTableMetadataWriter
Verify this pull request
Added a unit test in TestHoodieBackedMetadata, which creates multiple metadata partitions and then drops one. Asserted that there are no file slice from the dropped partition.
Committer checklist
Has a corresponding JIRA in PR title & commit
Commit message is descriptive of the change
CI is green
Necessary doc changes done or have another open PR
For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.