nsivabalan commented on code in PR #7605:
URL: https://github.com/apache/hudi/pull/7605#discussion_r1063907200
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieWriteClient.java:
##########
@@ -836,17 +838,19 @@ public boolean rollback(final String commitInstantTime,
Option<HoodiePendingRoll
* NOTE : This action requires all writers (ingest and compact) to a table
to be stopped before proceeding. Revert
* the (inflight/committed) record changes for all commits after the
provided instant time.
*
- * @param instantTime Instant time to which restoration is requested
+ * @param savepointToRestoreTimestamp savepoint intstant time to which
restoration is requested
*/
- public HoodieRestoreMetadata restoreToInstant(final String instantTime,
boolean initialMetadataTableIfNecessary) throws HoodieRestoreException {
- LOG.info("Begin restore to instant " + instantTime);
- final String restoreInstantTime =
HoodieActiveTimeline.createNewInstantTime();
+ public HoodieRestoreMetadata restoreToInstant(final String
savepointToRestoreTimestamp, boolean initialMetadataTableIfNecessary) throws
HoodieRestoreException {
+ LOG.info("Begin restore to instant " + savepointToRestoreTimestamp);
Timer.Context timerContext = metrics.getRollbackCtx();
try {
- HoodieTable<T, I, K, O> table = initTable(WriteOperationType.UNKNOWN,
Option.of(restoreInstantTime), initialMetadataTableIfNecessary);
- Option<HoodieRestorePlan> restorePlanOption =
table.scheduleRestore(context, restoreInstantTime, instantTime);
+ Triple<String, Option<HoodieRestorePlan>, HoodieTable<T, I, K, O>>
restorePlanReturn = getRestorePlan(savepointToRestoreTimestamp,
initialMetadataTableIfNecessary);
Review Comment:
you can leave the initTable here itself.
pass table as an arg to getRestorePlan()
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieWriteClient.java:
##########
@@ -857,11 +861,29 @@ public HoodieRestoreMetadata restoreToInstant(final
String instantTime, boolean
}
return restoreMetadata;
} else {
- throw new HoodieRestoreException("Failed to restore " +
config.getBasePath() + " to commit " + instantTime);
+ throw new HoodieRestoreException("Failed to restore " +
config.getBasePath() + " to commit " + savepointToRestoreTimestamp);
}
} catch (Exception e) {
- throw new HoodieRestoreException("Failed to restore to " + instantTime,
e);
+ throw new HoodieRestoreException("Failed to restore to " +
savepointToRestoreTimestamp, e);
+ }
+ }
+
+ private Triple<String, Option<HoodieRestorePlan>, HoodieTable<T, I, K, O>>
getRestorePlan(final String savepointToRestoreTimestamp, boolean
initialMetadataTableIfNecessary) throws IOException {
+ //Use the previous restore if it failed
+ HoodieTable<T, I, K, O> table = initTable(WriteOperationType.UNKNOWN,
Option.empty(), initialMetadataTableIfNecessary);
Review Comment:
Option.empty() is find for this call. internally its used for upgrade flow,
and MDT initialization to understand current instant in progress. incase of
restore, may be won't be any. since we wouldn't have scheduled the restore yet.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/restore/RestoreUtils.java:
##########
@@ -43,4 +46,24 @@ public static HoodieRestorePlan
getRestorePlan(HoodieTableMetaClient metaClient,
return TimelineMetadataUtils.deserializeAvroMetadata(
metaClient.getActiveTimeline().readRestoreInfoAsBytes(requested).get(),
HoodieRestorePlan.class);
}
+
+ /**
+ * Get the savepoint timestamp that this restore instant is restoring
+ * @param table the HoodieTable
+ * @param restoreInstant Instant referring to restore action
+ * @return timestamp of the savepoint we are restoring
+ * @throws IOException
+ *
+ * */
+ public static String getSavepointToRestoreTimestamp(HoodieTable table,
HoodieInstant restoreInstant) throws IOException {
Review Comment:
can we write UTs for this method.
##########
hudi-common/src/main/avro/HoodieRestorePlan.avsc:
##########
@@ -32,6 +32,12 @@
{
"name":"version",
"type":["int", "null"],
- "default": 1
- }]
+ "default": 2
+ },
+ {
+ "name": "savepointToRestoreTimestamp",
+ "type": ["string", "null"],
+ "default": "0"
Review Comment:
default should be null. why "0" ?
--
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]
