[HUDI-3135] Make delete partitions lazy to be executed by the cleaner #4489

XuQianJin-Stars · 2022-01-01T14:23:58Z

What is the purpose of the pull request

As of now, delete partitions will ensure all file groups are deleted, but the partition as such is not deleted. So, get all partitions might be returning the deleted partitions as well. but no data will be served since all file groups are deleted. With this patch, we are fixing it. We are letting cleaner take care of deleting the partitions when all file groups pertaining to a partitions are deleted.

Brief change log

Fixed the CleanPlanActionExecutor to return meta info about list of partitions to be deleted. If there are no valid file groups for a partition, clean planner will include the partition to be deleted.
Fixed HoodieCleanPlan avro schema to include the list of partitions to be deleted
CleanActionExecutor is fixed to delete partitions if any (as per clean plan)
Same info is added to HoodieCleanMetadata
Metadata table when applying clean metadata, will check for partitions to be deleted and will update the "all_partitions" record for the deleted partitions.

Verify this pull request

This change added tests and can be verified as follows:

Added tests to TestHoodieBackedMetadata
Added tests to TestAlterTableDropPartition

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.

xushiyan

@XuQianJin-Stars thanks for the patch. Question: without fully understanding the bug, from the look of it, this is a fix on metadata table, but will the problem persist if metadata is disabled? cc @nsivabalan please have a look when you got a chance.

XuQianJin-Stars · 2022-01-02T05:14:14Z

@XuQianJin-Stars thanks for the patch. Question: without fully understanding the bug, from the look of it, this is a fix on metadata table, but will the problem persist if metadata is disabled? cc @nsivabalan please have a look when you got a chance.

add two partitions dt='2021-10-01', dt='2021-10-02'
drop one partition dt='2021-10-01'
show partitions command
The query result: dt='2021-10-01', dt='2021-10-02'
The expected result is: dt='2021-10-02'

nsivabalan

LGTM. Can you add a UT to TestHoodieBackedMetadata to test DeletePartitions.

hudi-common/src/main/java/org/apache/hudi/metadata/HoodieTableMetadataUtil.java

nsivabalan · 2022-01-07T14:12:50Z

@XuQianJin-Stars : may I know when you plan to address the feedback. We are looking to get this in for 0.10.1. would appreciate if you can find time to address them sooner. Let me know.

XuQianJin-Stars · 2022-01-07T14:16:02Z

@XuQianJin-Stars : may I know when you plan to address the feedback. We are looking to get this in for 0.10.1. would appreciate if you can find time to address them sooner. Let me know.

well, sorry for late reply. I will update as soon as possible

nsivabalan · 2022-01-07T20:41:19Z

thanks! no probs.

XuQianJin-Stars · 2022-01-08T02:33:16Z

@hudi-bot run azure

xushiyan

Overall strategy looks good. Had a question about roll back.

.../main/java/org/apache/hudi/table/action/commit/SparkDeletePartitionCommitActionExecutor.java

xushiyan · 2022-01-08T08:24:17Z

.../main/java/org/apache/hudi/table/action/commit/SparkDeletePartitionCommitActionExecutor.java

+      if (table.getMetaClient().getFs().exists(fullPartitionPath)) {
+        table.getMetaClient().getFs().delete(fullPartitionPath, true);


tricky thing is: can we even roll back purge delete partitions? shall we make an exception for making delete partition irreversible?

tricky thing is: can we even roll back purge delete partitions? shall we make an exception for making delete partition irreversible?

Is this purge operation really irreversible, or do we need to put this soft delete in a specific location for easy rollback? And I want to thoroughly deal with this one when I want to implement the hidden partition in the next step.

yeah, I was also thinking about the same. from the current code, looks like it will be irresversible. Probably we should let cleaner clean up deleted partitions when it comes around cleaning up replaced files and delete partitions if all file groups have been cleaned up.
Or we can delete partition paths during clean action execution, just before transitioning clean.inflight to clean.completed.

hudi/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/clean/CleanActionExecutor.java

Line 212 in 98ec215

table.getActiveTimeline().transitionCleanInflightToComplete(inflightInstant,

may be, I will sync up with Raymond on this and see how we can go about it.

hudi-common/src/main/java/org/apache/hudi/metadata/HoodieTableMetadataUtil.java

nsivabalan · 2022-01-10T21:01:38Z

I would really love to get this in for 0.10.1, but let's be cautious in such irreversible operations and we don't have good precedence to follow as well. Lets not hurry up. I will remove it from 0.10.1, but lets try to brainstorm and get a closure for this.

nsivabalan

hi folks.
here is my take.
We definitely can't delete the partitions within SparkDeletePartitionCommitActionExecutor. bcoz, if operation partially fails, we don't know if partition is deleted or not. or We need to call out explicitly that this operation may or may not succeed. and users have to keep calling until delete_partition succeeds.

If not, we can mark all file groups as deleted.
and let cleaner take care of deleting the partitions for which all file groups are replaced/deleted.
But we may have to evolve the HoodieCleanMetadata may be which should be fine.

nsivabalan · 2022-01-08T13:47:19Z

.../main/java/org/apache/hudi/table/action/commit/SparkDeletePartitionCommitActionExecutor.java

+      if (table.getMetaClient().getFs().exists(fullPartitionPath)) {
+        table.getMetaClient().getFs().delete(fullPartitionPath, true);


yeah, I was also thinking about the same. from the current code, looks like it will be irresversible. Probably we should let cleaner clean up deleted partitions when it comes around cleaning up replaced files and delete partitions if all file groups have been cleaned up.
Or we can delete partition paths during clean action execution, just before transitioning clean.inflight to clean.completed.

hudi/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/clean/CleanActionExecutor.java

Line 212 in 98ec215

table.getActiveTimeline().transitionCleanInflightToComplete(inflightInstant,

may be, I will sync up with Raymond on this and see how we can go about it.

zhangyue19921010 · 2022-01-23T03:55:30Z

Hi @nsivabalan I agree with your opinion that let cleaner delete files and partitions. Just a little concern that how can we deal with the scenario that we trigger a delete partition action with a async cleaner + enable metadata table. -> Async cleaner started and finished before replaced committed :

Async cleaner finished to delete old replaced files.
Current replaced committed.
Meta table synced ==> partitions are deleted in meta data table.

We maybe get different result between getAllPartitions form metadata table and number of physical partitions.
Also wen could have a strict limit that delete partition only works with sync cleaner but wondering to know if it is possible.
Or this kind of out-of-sync will cause no damage.

updated: we could let cleaner sync metadata table and delete partitions in it. It could solve the consistency issue.

nsivabalan · 2022-02-07T21:27:22Z

@zhangyue19921010 : you are bringing up a good point. if I am not wrong, you are talking about a scenario, where someone triggered delete_partition for partition X and later added new data to the same partition is it? if yes, I see there could be some inconsistencies especially w/ async cleaner.

t10: trigger delete partition of partitionX. async cleaner is enabled and so, actual delete partition is not yet synced to metadata table.
t15: add new data to partition X which gets synced to Metadata table right away.
t20: async cleaner completes and deletes partition X. which gets synced to metadata table and deletes partition X.

Or were you talking about some other scenario ?

nsivabalan · 2022-02-07T21:27:56Z

@XuQianJin-Stars : we are looking to get this in for 0.11. Lets see if we can address feedback by this week so that we can land it by next week after review.

XuQianJin-Stars · 2022-02-08T07:22:16Z

@XuQianJin-Stars : we are looking to get this in for 0.11. Lets see if we can address feedback by this week so that we can land it by next week after review.

Well, I'll try to get this done this week.

nsivabalan · 2022-03-30T01:43:43Z

@XuQianJin-Stars @xushiyan : We are in need of this patch for async indexer feature in metadata table. So, I took over and pushed an update. I have fixed the description to explain what I have done. Have cleaned up few things which was not required. Do review it when you get a chance.

XuQianJin-Stars · 2022-03-30T02:10:46Z

@XuQianJin-Stars @xushiyan : We are in need of this patch for async indexer feature in metadata table. So, I took over and pushed an update. I have fixed the description to explain what I have done. Have cleaned up few things which was not required. Do review it when you get a chance.

hi @nsivabalan Thank you so much for the update, sorry for replying so late, interrupted by other things.

hudi-bot · 2022-03-31T03:02:59Z

CI report:

8573748 Azure: FAILURE

Bot commands

@hudi-bot supports the following commands:

@hudi-bot run azure re-run the last Azure build

codope

LGTM. I've verfied this patch together with #5169 and the metadata partition does get physically deleted. I'll address a couple of minor comments here in #5169.

codope · 2022-03-31T06:00:00Z

...client/hudi-client-common/src/main/java/org/apache/hudi/table/action/clean/CleanPlanner.java

@@ -350,8 +355,12 @@ public CleanPlanner(HoodieEngineContext context, HoodieTable<T, I, K, O> hoodieT
          }
        }
      }
+      // if there are no valid file groups for the partition, mark it to be deleted
+      if (fileGroups.isEmpty()) {


…apache#4489) As of now, delete partitions will ensure all file groups are deleted, but the partition as such is not deleted. So, get all partitions might be returning the deleted partitions as well. but no data will be served since all file groups are deleted. With this patch, we are fixing it. We are letting cleaner take care of deleting the partitions when all file groups pertaining to a partitions are deleted. - Fixed the CleanPlanActionExecutor to return meta info about list of partitions to be deleted. If there are no valid file groups for a partition, clean planner will include the partition to be deleted. - Fixed HoodieCleanPlan avro schema to include the list of partitions to be deleted - CleanActionExecutor is fixed to delete partitions if any (as per clean plan) - Same info is added to HoodieCleanMetadata - Metadata table when applying clean metadata, will check for partitions to be deleted and will update the "all_partitions" record for the deleted partitions. Co-authored-by: sivabalan <n.siva.b@gmail.com>

Zouxxyy · 2022-10-21T09:15:56Z

@XuQianJin-Stars if disabled hudi metadata, the following bug still exists:

add two partitions dt='2021-10-01', dt='2021-10-02'
drop one partition dt='2021-10-01'
show partitions command
The query result: dt='2021-10-01', dt='2021-10-02'
The expected result is: dt='2021-10-02'

Zouxxyy · 2022-10-21T14:43:07Z

@XuQianJin-Stars

just set hoodie.metadata.enable = false and then run TestAlterTableDropPartition will fail

nsivabalan · 2022-10-21T21:54:46Z

@Zouxxyy : yes, physical deletion of partitions is lazy. only when cleaner comes around next time, it will clean it up.
but if you try to list data for the deleted partition (2021-10-01), you should not see any data once drop one partition dt='2021-10-01' succeeds.

XuQianJin-Stars force-pushed the HUDI-3135 branch 2 times, most recently from aa50fdf to 9bfe088 Compare January 2, 2022 04:11

xushiyan requested a review from nsivabalan January 2, 2022 04:30

xushiyan reviewed Jan 2, 2022

View reviewed changes

XuQianJin-Stars force-pushed the HUDI-3135 branch from 9bfe088 to 5feae09 Compare January 2, 2022 08:59

nsivabalan added the priority:critical production down; pipelines stalled; Need help asap. label Jan 4, 2022

nsivabalan reviewed Jan 4, 2022

View reviewed changes

nsivabalan changed the title ~~[HUDI-3135] Fix Show Partitions Command's Result after drop partition~~ [HUDI-3135] Fix Delete partitions with metadata table and fix show partitions in spark sql Jan 4, 2022

xushiyan reviewed Jan 5, 2022

View reviewed changes

hudi-common/src/main/java/org/apache/hudi/metadata/HoodieTableMetadataUtil.java Outdated Show resolved Hide resolved

nsivabalan self-assigned this Jan 6, 2022

XuQianJin-Stars force-pushed the HUDI-3135 branch from edcd592 to 7ca735c Compare January 8, 2022 02:37

xushiyan reviewed Jan 8, 2022

View reviewed changes

XuQianJin-Stars force-pushed the HUDI-3135 branch from fa5894f to 8896e81 Compare January 9, 2022 01:29

nsivabalan assigned xushiyan Jan 9, 2022

nsivabalan added priority:major degraded perf; unable to move forward; potential bugs and removed priority:critical production down; pipelines stalled; Need help asap. labels Jan 10, 2022

nsivabalan reviewed Jan 13, 2022

View reviewed changes

nsivabalan mentioned this pull request Jan 17, 2022

[HUDI-3116]Add a new HoodieDropPartitionsTool to let users drop table partitions through a standalone job. #4459

Merged

5 tasks

nsivabalan added the priority:critical production down; pipelines stalled; Need help asap. label Feb 8, 2022

XuQianJin-Stars force-pushed the HUDI-3135 branch from ad5ed95 to d7168e4 Compare March 26, 2022 16:21

XuQianJin-Stars changed the title ~~[HUDI-3135] Fix Delete partitions with metadata table and fix show partitions in spark sql~~ [WIP][HUDI-3135] Fix Delete partitions with metadata table and fix show partitions in spark sql Mar 26, 2022

XuQianJin-Stars added 2 commits March 28, 2022 11:25

[HUDI-3135] Fix Show Partitions Command's Result after drop partition

562683b

[HUDI-3135] Fix Show Partitions Command's Result after drop partition

2ede26d

XuQianJin-Stars force-pushed the HUDI-3135 branch from d7168e4 to 2ede26d Compare March 28, 2022 08:33

[HUDI-3135] Fix Show Partitions Command's Result after drop partition

c7a2b48

XuQianJin-Stars changed the title ~~[WIP][HUDI-3135] Fix Delete partitions with metadata table and fix show partitions in spark sql~~ [HUDI-3135] Fix Delete partitions with metadata table and fix show partitions in spark sql Mar 28, 2022

[HUDI-3135] Fix Show Partitions Command's Result after drop partition

e1e36f4

nsivabalan added the priority:blocker label Mar 29, 2022

nsivabalan changed the title ~~[HUDI-3135] Fix Delete partitions with metadata table and fix show partitions in spark sql~~ [HUDI-3135] Make delete partitions lazy to be executed by the cleaner Mar 30, 2022

XuQianJin-Stars mentioned this pull request Mar 30, 2022

[HUDI-3743] Support DELETE_PARTITION for metadata table #5169

Merged

5 tasks

XuQianJin-Stars force-pushed the HUDI-3135 branch from 4100259 to 227baaa Compare March 30, 2022 15:25

yihua assigned yihua and unassigned nsivabalan and xushiyan Mar 30, 2022

Fixing lazy deletion of partitions

8573748

nsivabalan force-pushed the HUDI-3135 branch from 227baaa to 8573748 Compare March 30, 2022 23:22

codope approved these changes Mar 31, 2022

View reviewed changes

codope self-assigned this Mar 31, 2022

nsivabalan merged commit 80011df into apache:master Mar 31, 2022

codope mentioned this pull request Aug 4, 2022

[SUPPORT] DELETE_PARTITION causes AWS Athena Query failure #6024

Closed

boneanxs mentioned this pull request Nov 8, 2023

[HUDI-7054] ShowPartitionsCommand should consider lazy delete_partitions #10019

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HUDI-3135] Make delete partitions lazy to be executed by the cleaner #4489

[HUDI-3135] Make delete partitions lazy to be executed by the cleaner #4489

XuQianJin-Stars commented Jan 1, 2022 •

edited by nsivabalan

Loading

xushiyan left a comment •

edited

Loading

XuQianJin-Stars commented Jan 2, 2022 •

edited

Loading

nsivabalan left a comment

nsivabalan commented Jan 7, 2022

XuQianJin-Stars commented Jan 7, 2022

nsivabalan commented Jan 7, 2022

XuQianJin-Stars commented Jan 8, 2022

xushiyan left a comment

xushiyan Jan 8, 2022

XuQianJin-Stars Jan 8, 2022 •

edited

Loading

nsivabalan Jan 8, 2022

nsivabalan commented Jan 10, 2022

nsivabalan left a comment

nsivabalan Jan 8, 2022

zhangyue19921010 commented Jan 23, 2022 •

edited

Loading

nsivabalan commented Feb 7, 2022

nsivabalan commented Feb 7, 2022

XuQianJin-Stars commented Feb 8, 2022

nsivabalan commented Mar 30, 2022

XuQianJin-Stars commented Mar 30, 2022

hudi-bot commented Mar 31, 2022

codope left a comment

codope Mar 31, 2022

Zouxxyy commented Oct 21, 2022

Zouxxyy commented Oct 21, 2022 •

edited

Loading

nsivabalan commented Oct 21, 2022

		if (table.getMetaClient().getFs().exists(fullPartitionPath)) {
		table.getMetaClient().getFs().delete(fullPartitionPath, true);

[HUDI-3135] Make delete partitions lazy to be executed by the cleaner #4489

[HUDI-3135] Make delete partitions lazy to be executed by the cleaner #4489

Conversation

XuQianJin-Stars commented Jan 1, 2022 • edited by nsivabalan Loading

What is the purpose of the pull request

Brief change log

Verify this pull request

Committer checklist

xushiyan left a comment • edited Loading

Choose a reason for hiding this comment

XuQianJin-Stars commented Jan 2, 2022 • edited Loading

nsivabalan left a comment

Choose a reason for hiding this comment

nsivabalan commented Jan 7, 2022

XuQianJin-Stars commented Jan 7, 2022

nsivabalan commented Jan 7, 2022

XuQianJin-Stars commented Jan 8, 2022

xushiyan left a comment

Choose a reason for hiding this comment

xushiyan Jan 8, 2022

Choose a reason for hiding this comment

XuQianJin-Stars Jan 8, 2022 • edited Loading

Choose a reason for hiding this comment

nsivabalan Jan 8, 2022

Choose a reason for hiding this comment

nsivabalan commented Jan 10, 2022

nsivabalan left a comment

Choose a reason for hiding this comment

nsivabalan Jan 8, 2022

Choose a reason for hiding this comment

zhangyue19921010 commented Jan 23, 2022 • edited Loading

nsivabalan commented Feb 7, 2022

nsivabalan commented Feb 7, 2022

XuQianJin-Stars commented Feb 8, 2022

nsivabalan commented Mar 30, 2022

XuQianJin-Stars commented Mar 30, 2022

hudi-bot commented Mar 31, 2022

CI report:

codope left a comment

Choose a reason for hiding this comment

codope Mar 31, 2022

Choose a reason for hiding this comment

Zouxxyy commented Oct 21, 2022

Zouxxyy commented Oct 21, 2022 • edited Loading

nsivabalan commented Oct 21, 2022

XuQianJin-Stars commented Jan 1, 2022 •

edited by nsivabalan

Loading

xushiyan left a comment •

edited

Loading

XuQianJin-Stars commented Jan 2, 2022 •

edited

Loading

XuQianJin-Stars Jan 8, 2022 •

edited

Loading

zhangyue19921010 commented Jan 23, 2022 •

edited

Loading

Zouxxyy commented Oct 21, 2022 •

edited

Loading