Skip to content

[GOBBLIN-1825]Hive retention job should fail if deleting underlying files fail #3687

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.

Sign up for GitHub

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

Merged
ZihanLi58 merged 3 commits into from
May 24, 2023
Merged

[GOBBLIN-1825]Hive retention job should fail if deleting underlying files fail #3687

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d43871c
fail hive retention if we fail to delete underlying hdfs files
Apr 24, 2023
625ee66
address PR comments
Apr 24, 2023
029fe85
address PR comments
May 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions .../java/org/apache/gobblin/data/management/retention/version/HiveDatasetVersionCleaner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,20 +88,31 @@ public void clean() throws IOException {
Partition partition = hiveDatasetVersion.getPartition();
try {
if (!cleanableHiveDataset.isSimulate()) {
// As part of the cleanup process, we want to delete both: hive partition and underlying hdfs files
// However, scenarios arise where hive partition is dropped, but hdfs files aren't, leading to dangling files
// Thus, we reverse the order of cleaning up hdfs files first and then drop hive partition
// In cases where HMS was unresponsive and hive partition couldn't be dropped
// re-running hive retention would drop the partition with no hdfs files found to be deleted
// or set the flag `isShouldDeleteData` to false
if (cleanableHiveDataset.isShouldDeleteData()) {
cleanableHiveDataset.getFsCleanableHelper().clean(hiveDatasetVersion, possiblyEmptyDirectories);
Comment thread
meethngala marked this conversation as resolved.
}
client.get().dropPartition(partition.getTable().getDbName(), partition.getTable().getTableName(), partition.getValues(), false);
Comment thread
meethngala marked this conversation as resolved.
log.info("Successfully dropped partition " + partition.getCompleteName());
} else {
log.info("Simulating drop partition " + partition.getCompleteName());
}
if (cleanableHiveDataset.isShouldDeleteData()) {
cleanableHiveDataset.getFsCleanableHelper().clean(hiveDatasetVersion, possiblyEmptyDirectories);
}
} catch (TException | IOException e) {
log.warn(String.format("Failed to completely delete partition %s.", partition.getCompleteName()), e);
throw new IOException(e);
}
}
cleanableHiveDataset.getFsCleanableHelper().cleanEmptyDirectories(possiblyEmptyDirectories, cleanableHiveDataset);
try {
cleanableHiveDataset.getFsCleanableHelper().cleanEmptyDirectories(possiblyEmptyDirectories, cleanableHiveDataset);
} catch (IOException ex) {
log.warn(String.format("Failed to delete at least one or more empty directories from total:{%s} with root path %s", possiblyEmptyDirectories.size(), cleanableHiveDataset.datasetRoot()), ex);
throw new IOException(ex);
}
}

@Override
Expand Down