-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Correct clean bug that causes exception when partitionPaths are empty #202
Conversation
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 also create a github issue and assign it to yourself? Thanks.
Update: Noticed the issue exists already and added the comment to link this PR to the issue
try { | ||
List<String> partitionsToClean = | ||
FSUtils.getAllPartitionPaths(getFs(), getMetaClient().getBasePath(), config.shouldAssumeDatePartitioning()); | ||
logger.info("Partitions to clean up : " + partitionsToClean + ", with policy " + config | ||
.getCleanerPolicy()); | ||
partitionCleanStats = cleanPartitionPaths(partitionsToClean, jsc); | ||
if (partitionsToClean.isEmpty()) { | ||
logger.info("Nothing to clean here mom. It is already clean"); | ||
return partitionCleanStats; |
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.
return Lists.newArrayList()
@@ -492,17 +492,17 @@ protected HoodieUpdateHandle getUpdateHandle(String commitTime, String fileLoc, | |||
*/ | |||
@Override | |||
public List<HoodieCleanStat> clean(JavaSparkContext jsc) { | |||
List<HoodieCleanStat> partitionCleanStats; | |||
List<HoodieCleanStat> partitionCleanStats = Collections.emptyList(); |
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.
Remove this and return empty list when there is nothing to clean
if (partitionsToClean.isEmpty()) { | ||
logger.info("Nothing to clean here mom. It is already clean"); | ||
return partitionCleanStats; | ||
} | ||
partitionCleanStats = cleanPartitionPaths(partitionsToClean, jsc); |
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.
return this directly
8d4ab73
to
8e991b0
Compare
Updated the PR Prasanna. |
This fixes #200
Corrected logic and added a test to detect the bug.