nsivabalan commented on code in PR #8430:
URL: https://github.com/apache/hudi/pull/8430#discussion_r1185522435
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java:
##########
@@ -663,6 +663,16 @@ public class HoodieWriteConfig extends HoodieConfig {
+ "will not print any configuration which contains the configured
filter. For example with "
+ "a configured filter `ssl`, value for config
`ssl.trustore.location` would be masked.");
+ public static final ConfigProperty<Boolean> ROLLBACK_BACKUP_ENABLED =
ConfigProperty
+ .key("hoodie.rollback.backup.enabled")
+ .defaultValue(false)
+ .withDocumentation("Backup instants removed during rollback and restore
(useful for debugging)");
+
+ public static final ConfigProperty<String> ROLLBACK_BACKUP_DIRECTORY =
ConfigProperty
+ .key("hoodie.rollback.backup.dir")
Review Comment:
hoodie.rollback.timeline.backup.dir
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java:
##########
@@ -663,6 +663,16 @@ public class HoodieWriteConfig extends HoodieConfig {
+ "will not print any configuration which contains the configured
filter. For example with "
+ "a configured filter `ssl`, value for config
`ssl.trustore.location` would be masked.");
+ public static final ConfigProperty<Boolean> ROLLBACK_BACKUP_ENABLED =
ConfigProperty
+ .key("hoodie.rollback.backup.enabled")
Review Comment:
can we add "timeline" may be to the name. not very apparent. seems like
entire data might be backedup.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java:
##########
@@ -663,6 +663,16 @@ public class HoodieWriteConfig extends HoodieConfig {
+ "will not print any configuration which contains the configured
filter. For example with "
+ "a configured filter `ssl`, value for config
`ssl.trustore.location` would be masked.");
+ public static final ConfigProperty<Boolean> ROLLBACK_BACKUP_ENABLED =
ConfigProperty
+ .key("hoodie.rollback.backup.enabled")
+ .defaultValue(false)
+ .withDocumentation("Backup instants removed during rollback and restore
(useful for debugging)");
+
+ public static final ConfigProperty<String> ROLLBACK_BACKUP_DIRECTORY =
ConfigProperty
+ .key("hoodie.rollback.backup.dir")
+ .defaultValue(".rollback_backup")
+ .withDocumentation("Path where instants being rolled back are copied. If
not absolute path then a directoruy relative to .hoodie folder is created.");
Review Comment:
typo "directoruy"
##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieActiveTimeline.java:
##########
@@ -828,4 +829,17 @@ protected Option<byte[]> readDataFromPath(Path detailPath)
{
public HoodieActiveTimeline reload() {
return new HoodieActiveTimeline(metaClient);
}
+
+ public void copyInstant(HoodieInstant instant, Path dstDir) {
+ Path srcPath = new Path(metaClient.getMetaPath(), instant.getFileName());
+ Path dstPath = new Path(dstDir, instant.getFileName());
+ try {
+ FileSystem srcFs = srcPath.getFileSystem(metaClient.getHadoopConf());
+ FileSystem dstFs = dstPath.getFileSystem(metaClient.getHadoopConf());
+ dstFs.mkdirs(dstDir);
+ FileUtil.copy(srcFs, srcPath, dstFs, dstPath, false, srcFs.getConf());
Review Comment:
lets make 5th arg as true. on crash scenarios, if we retry to back up again,
we should not fail.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/rollback/BaseRollbackActionExecutor.java:
##########
@@ -297,4 +301,35 @@ protected void dropBootstrapIndexIfNeeded(HoodieInstant
instantToRollback) {
BootstrapIndex.getBootstrapIndex(table.getMetaClient()).dropIndex();
}
}
+
+ private void backupRollbackInstantsIfNeeded() {
+ if (!config.shouldBackupRollbacks()) {
+ // Backup not required
+ return;
+ }
+
+ Path backupDir = new Path(config.getRollbackBackupDirectory());
+ if (!backupDir.isAbsolute()) {
+ // Path specified is relative to the meta directory
+ backupDir = new Path(table.getMetaClient().getMetaPath(),
config.getRollbackBackupDirectory());
+ }
+
+ // Determine the instants to backup. Completed and inflight instants are
backed up.
+ HoodieActiveTimeline activeTimeline = table.getActiveTimeline();
+ List<HoodieInstant> instantsToBackup = new ArrayList<HoodieInstant>(3);
+ instantsToBackup.add(instantToRollback);
+ if (instantToRollback.isCompleted()) {
+
instantsToBackup.add(HoodieTimeline.getInflightInstant(instantToRollback,
table.getMetaClient()));
Review Comment:
lets ensure we backup all 3 if present. (requested, inflight and completed
instant)
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/rollback/BaseRollbackActionExecutor.java:
##########
@@ -297,4 +301,35 @@ protected void dropBootstrapIndexIfNeeded(HoodieInstant
instantToRollback) {
BootstrapIndex.getBootstrapIndex(table.getMetaClient()).dropIndex();
}
}
+
+ private void backupRollbackInstantsIfNeeded() {
+ if (!config.shouldBackupRollbacks()) {
+ // Backup not required
+ return;
+ }
+
+ Path backupDir = new Path(config.getRollbackBackupDirectory());
+ if (!backupDir.isAbsolute()) {
+ // Path specified is relative to the meta directory
+ backupDir = new Path(table.getMetaClient().getMetaPath(),
config.getRollbackBackupDirectory());
+ }
+
+ // Determine the instants to backup. Completed and inflight instants are
backed up.
+ HoodieActiveTimeline activeTimeline = table.getActiveTimeline();
+ List<HoodieInstant> instantsToBackup = new ArrayList<HoodieInstant>(3);
+ instantsToBackup.add(instantToRollback);
Review Comment:
this is inline w/ what we do for archival. we will retain entire history.
also helps w/ investigation w/ metadata going out of sync may be.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java:
##########
@@ -663,6 +663,16 @@ public class HoodieWriteConfig extends HoodieConfig {
+ "will not print any configuration which contains the configured
filter. For example with "
+ "a configured filter `ssl`, value for config
`ssl.trustore.location` would be masked.");
+ public static final ConfigProperty<Boolean> ROLLBACK_BACKUP_ENABLED =
ConfigProperty
+ .key("hoodie.rollback.backup.enabled")
Review Comment:
hoodie.rollback.timeline.backup.enabled
--
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]
