xushiyan commented on code in PR #7196:
URL: https://github.com/apache/hudi/pull/7196#discussion_r1032865892
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/HoodieTimelineArchiver.java:
##########
@@ -556,27 +555,29 @@ private boolean
deleteArchivedInstants(List<HoodieInstant> archivedInstants, Hoo
// other monitors on the timeline(such as the compaction or clustering
services) would
// mistakenly recognize the pending file as a pending operation,
// then all kinds of weird bugs occur.
- boolean success = deleteArchivedInstantFiles(context, true,
pendingInstantFiles);
- success &= deleteArchivedInstantFiles(context, success,
completedInstantFiles);
+ deleteArchivedInstants(context, metaClient, pendingInstants);
+ deleteArchivedInstants(context, metaClient, completedInstants);
// Remove older meta-data from auxiliary path too
Option<HoodieInstant> latestCommitted =
Option.fromJavaOptional(archivedInstants.stream().filter(i -> i.isCompleted()
&& (i.getAction().equals(HoodieTimeline.COMMIT_ACTION)
||
(i.getAction().equals(HoodieTimeline.DELTA_COMMIT_ACTION)))).max(Comparator.comparing(HoodieInstant::getTimestamp)));
LOG.info("Latest Committed Instant=" + latestCommitted);
if (latestCommitted.isPresent()) {
- success &=
deleteAllInstantsOlderOrEqualsInAuxMetaFolder(latestCommitted.get());
+ return
deleteAllInstantsOlderOrEqualsInAuxMetaFolder(latestCommitted.get());
}
- return success;
+ return true;
}
- private boolean deleteArchivedInstantFiles(HoodieEngineContext context,
boolean success, List<String> files) {
- Map<String, Boolean> resultDeleteInstantFiles =
deleteFilesParallelize(metaClient, files, context, false);
-
- for (Map.Entry<String, Boolean> result :
resultDeleteInstantFiles.entrySet()) {
- LOG.info("Archived and deleted instant file " + result.getKey() + " : "
+ result.getValue());
- success &= result.getValue();
+ private void deleteArchivedInstants(HoodieEngineContext context,
+ HoodieTableMetaClient metaClient,
+ List<HoodieInstant> instants) {
+ if (!instants.isEmpty()) {
+ context.foreach(
+ instants,
+ instant ->
metaClient.getActiveTimeline().deleteInstantFileIfExists(instant),
+ Math.min(instants.size(), config.getArchiveDeleteParallelism())
Review Comment:
no need to use a separate method: this method overload the other method,
only signature changes that confuses people about which calls which. This is
just 1 line invocation. you just call `context.foreach()` once for the 2 lists
together by chaining them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
