From c81df99e50f2df84d85f08ff3a839595dad974d7 Mon Sep 17 00:00:00 2001 From: Danny Chan Date: Sat, 25 Dec 2021 18:10:43 +0800 Subject: [PATCH] [HUDI-3102] Do not store rollback plan in inflight instant (#4445) --- .../rollback/BaseRollbackActionExecutor.java | 79 ++++++++----------- .../table/timeline/HoodieActiveTimeline.java | 5 +- 2 files changed, 33 insertions(+), 51 deletions(-) diff --git a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/rollback/BaseRollbackActionExecutor.java b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/rollback/BaseRollbackActionExecutor.java index 7d2c36696..9d5895de8 100644 --- a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/rollback/BaseRollbackActionExecutor.java +++ b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/rollback/BaseRollbackActionExecutor.java @@ -105,62 +105,45 @@ public abstract class BaseRollbackActionExecutor table, HoodieInstant rollbackInstant, HoodieRollbackPlan rollbackPlan) { ValidationUtils.checkArgument(rollbackInstant.getState().equals(HoodieInstant.State.REQUESTED) || rollbackInstant.getState().equals(HoodieInstant.State.INFLIGHT)); - try { - final HoodieInstant inflightInstant; - final HoodieTimer timer = new HoodieTimer(); - timer.startTimer(); - if (rollbackInstant.isRequested()) { - inflightInstant = table.getActiveTimeline().transitionRollbackRequestedToInflight(rollbackInstant, - TimelineMetadataUtils.serializeRollbackPlan(rollbackPlan)); - } else { - inflightInstant = rollbackInstant; - } + final HoodieTimer timer = new HoodieTimer(); + timer.startTimer(); + final HoodieInstant inflightInstant = rollbackInstant.isRequested() + ? table.getActiveTimeline().transitionRollbackRequestedToInflight(rollbackInstant) + : rollbackInstant; - HoodieTimer rollbackTimer = new HoodieTimer().startTimer(); - List stats = doRollbackAndGetStats(rollbackPlan); - HoodieRollbackMetadata rollbackMetadata = TimelineMetadataUtils.convertRollbackMetadata( - instantTime, - Option.of(rollbackTimer.endTimer()), - Collections.singletonList(instantToRollback), - stats); - if (!skipTimelinePublish) { - finishRollback(inflightInstant, rollbackMetadata); - } - - // Finally, remove the markers post rollback. - WriteMarkersFactory.get(config.getMarkersType(), table, instantToRollback.getTimestamp()) - .quietDeleteMarkerDir(context, config.getMarkersDeleteParallelism()); - - return rollbackMetadata; - } catch (IOException e) { - throw new HoodieIOException("Failed to rollback commit ", e); + HoodieTimer rollbackTimer = new HoodieTimer().startTimer(); + List stats = doRollbackAndGetStats(rollbackPlan); + HoodieRollbackMetadata rollbackMetadata = TimelineMetadataUtils.convertRollbackMetadata( + instantTime, + Option.of(rollbackTimer.endTimer()), + Collections.singletonList(instantToRollback), + stats); + if (!skipTimelinePublish) { + finishRollback(inflightInstant, rollbackMetadata); } + + // Finally, remove the markers post rollback. + WriteMarkersFactory.get(config.getMarkersType(), table, instantToRollback.getTimestamp()) + .quietDeleteMarkerDir(context, config.getMarkersDeleteParallelism()); + + return rollbackMetadata; } @Override public HoodieRollbackMetadata execute() { table.getMetaClient().reloadActiveTimeline(); - List rollBackInstants = table.getRollbackTimeline() - .filterInflightsAndRequested().getInstants().collect(Collectors.toList()); - if (rollBackInstants.isEmpty()) { - throw new HoodieRollbackException("No Requested Rollback Instants found to execute rollback "); + Option rollbackInstant = table.getRollbackTimeline() + .filterInflightsAndRequested() + .filter(instant -> instant.getTimestamp().equals(instantTime)) + .firstInstant(); + if (!rollbackInstant.isPresent()) { + throw new HoodieRollbackException("No pending rollback instants found to execute rollback"); } - HoodieInstant rollbackInstant = null; - for (HoodieInstant instant : rollBackInstants) { - if (instantTime.equals(instant.getTimestamp())) { - rollbackInstant = instant; - break; - } - } - if (rollbackInstant != null) { - try { - HoodieRollbackPlan rollbackPlan = RollbackUtils.getRollbackPlan(table.getMetaClient(), rollbackInstant); - return runRollback(table, rollBackInstants.get(0), rollbackPlan); - } catch (IOException e) { - throw new HoodieIOException("Failed to fetch rollback plan to rollback commit " + rollbackInstant.getTimestamp(), e); - } - } else { - throw new HoodieIOException("No inflight rollback instants found for commit time " + instantTime); + try { + HoodieRollbackPlan rollbackPlan = RollbackUtils.getRollbackPlan(table.getMetaClient(), rollbackInstant.get()); + return runRollback(table, rollbackInstant.get(), rollbackPlan); + } catch (IOException e) { + throw new HoodieIOException("Failed to fetch rollback plan for commit " + instantTime, e); } } diff --git a/hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieActiveTimeline.java b/hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieActiveTimeline.java index ee442b196..e4ed49d72 100644 --- a/hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieActiveTimeline.java +++ b/hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieActiveTimeline.java @@ -357,14 +357,13 @@ public class HoodieActiveTimeline extends HoodieDefaultTimeline { * Transition Rollback State from requested to inflight. * * @param requestedInstant requested instant - * @param data Optional data to be stored * @return commit instant */ - public HoodieInstant transitionRollbackRequestedToInflight(HoodieInstant requestedInstant, Option data) { + public HoodieInstant transitionRollbackRequestedToInflight(HoodieInstant requestedInstant) { ValidationUtils.checkArgument(requestedInstant.getAction().equals(HoodieTimeline.ROLLBACK_ACTION)); ValidationUtils.checkArgument(requestedInstant.isRequested()); HoodieInstant inflight = new HoodieInstant(State.INFLIGHT, ROLLBACK_ACTION, requestedInstant.getTimestamp()); - transitionState(requestedInstant, inflight, data); + transitionState(requestedInstant, inflight, Option.empty()); return inflight; }