Skip to content

[HUDI-6675] Fix Clean action will delete the whole table#9413

Merged
danny0405 merged 3 commits intoapache:masterfrom
leosanqing:master
Aug 14, 2023
Merged

[HUDI-6675] Fix Clean action will delete the whole table#9413
danny0405 merged 3 commits intoapache:masterfrom
leosanqing:master

Conversation

@leosanqing
Copy link
Contributor

@leosanqing leosanqing commented Aug 10, 2023

Change Logs

Clean action.

Avoid to clean the whole table data on hdfs .

Impact

Clean action.

When it is a non-partition table, Clean action will input the "" as partition path , this will lead to Cleaner to clean the whole table data on the hdfs . path format is table_basepath + "/" + ""

so, when path is "", it means this clean action should not delete dir, just skip it;

Risk level (write none, low medium or high below)

low

Documentation Update

Describe any necessary documentation update if there is any new feature, config, or user-facing change

  • The config description must be updated if new configs are added or the default value of the configs are changed
  • Any new feature or user-facing change requires updating the Hudi website. Please create a Jira ticket, attach the
    ticket number here and follow the instruction to make
    changes to the website.

Contributor's checklist

  • Read through contributor's guide
  • Change Logs and Impact were stated clearly
  • Adequate tests were added if applicable
  • CI passed

@leosanqing leosanqing changed the title [HUDI-6675] Clean action will delete the whole table [HUDI-6675] Fix Clean action will delete the whole table Aug 10, 2023
try {
deleteFileAndGetResult(table.getMetaClient().getFs(), table.getMetaClient().getBasePath() + "/" + entry);
if (!StringUtils.isNullOrEmpty(entry)) {
deleteFileAndGetResult(table.getMetaClient().getFs(), table.getMetaClient().getBasePath() + "/" + entry);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kind of think the cleanerPlan.getPartitionsToBeDeleted() should be fixed, can we write a test case for it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. we do have a property in TableConfig to check if the table is partitioned or non-partitioned. We can consult w/ it and empty out partitionsToBeDeleted if its unpartitioned.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, the cleanerPlan.getPartitionsToBeDeleted() should be empty for non-partitioned table and I added a test to verify whether the whole table is deleted or not. But, maybe, in an unlikely scenario, when table is partitioned but with empty partition path, this could happen.

Copy link
Contributor

@boneanxs boneanxs Aug 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in an unlikely scenario, when table is partitioned but with empty partition path, this could happen.

It actually happened inside my company🥲, and all data are deleted, but have no clue how it happened...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @leosanqing

May I ask if you have encountered this issue? A writer generated data files incorrectly in table root path, but the table is a partitioned table.

E.g.

hdfs://..../hudi_table_folder/partition1/...parquet
hdfs://..../hudi_table_folder/partition2/...parquet
hdfs://..../hudi_table_folder/...parquet <- this file should not be here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @leosanqing

May I ask if you have encountered this issue? A writer generated data files incorrectly in table root path, but the table is a partitioned table.

E.g.

hdfs://..../hudi_table_folder/partition1/...parquet
hdfs://..../hudi_table_folder/partition2/...parquet
hdfs://..../hudi_table_folder/...parquet <- this file should not be here.

No, I haven't.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. Thanks for your reply.

@nsivabalan nsivabalan added release-0.14.0 priority:blocker Production down; release blocker labels Aug 11, 2023
@hudi-bot
Copy link
Collaborator

CI report:

Bot commands @hudi-bot supports the following commands:
  • @hudi-bot run azure re-run the last Azure build

@danny0405 danny0405 merged commit a5e5c7b into apache:master Aug 14, 2023
prashantwason pushed a commit that referenced this pull request Aug 18, 2023
The clean action mistakenly delete the whole table when the table is non-partitioned.

---------

Co-authored-by: Sagar Sumit <sagarsumit09@gmail.com>
@wqlsdb
Copy link

wqlsdb commented Oct 18, 2023

In my company, I also encountered a situation where the entire table directory was deleted

@danny0405
Copy link
Contributor

@wqlsdb , would you mind to cherry-pick this fix into your local repo?

@TengHuo
Copy link
Contributor

TengHuo commented Oct 18, 2023

In my company, I also encountered a situation where the entire table directory was deleted

Hi @wqlsdb would you like to discuss it offline or email?

We encountered this issue multiple times internally, and we are trying to find the root cause. Think it could be helpful if we can sync some common information.

@wqlsdb
Copy link

wqlsdb commented Oct 18, 2023

@danny0405 yes.Our company plans to upgrade to the latest version

@wqlsdb
Copy link

wqlsdb commented Dec 7, 2023

@TengHuo ok my email :wqlxh7891@163.com

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority:blocker Production down; release blocker release-0.14.0

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

8 participants