smengcl commented on code in PR #10774:
URL: https://github.com/apache/ozone/pull/10774#discussion_r3730228038
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/defrag/SnapshotDefragService.java:
##########
@@ -573,20 +574,74 @@ OmMetadataManagerImpl createCheckpoint(SnapshotInfo
snapshotInfo,
Set<String> incrementalColumnFamilies) throws IOException {
try (UncheckedAutoCloseableSupplier<OmSnapshot> snapshot =
omSnapshotManager.getActiveSnapshot(
snapshotInfo.getVolumeName(), snapshotInfo.getBucketName(),
snapshotInfo.getName())) {
- DBCheckpoint checkpoint =
snapshot.get().getMetadataManager().getStore().getCheckpoint(tmpDefragDir,
true);
- try (OmMetadataManagerImpl metadataManagerBeforeTruncate =
- createDefragCheckpointMetadataManager(checkpoint, false)) {
- DBStore dbStore = metadataManagerBeforeTruncate.getStore();
- for (String table : metadataManagerBeforeTruncate.listTableNames()) {
- if (!incrementalColumnFamilies.contains(table)) {
- dbStore.dropTable(table);
+ DBStore snapshotStore = snapshot.get().getMetadataManager().getStore();
+ DBCheckpoint checkpoint = snapshotStore.getCheckpoint(tmpDefragDir,
true);
+ if (checkpoint == null) {
+ deletePartialCheckpointDirs(snapshotStore.getDbLocation().getName());
+ throw new IOException("Failed to create checkpoint under " +
tmpDefragDir + " for snapshot: "
+ + snapshotInfo.getTableKey() + " (ID: " +
snapshotInfo.getSnapshotId() + ")");
+ }
+ Path checkpointLocation = checkpoint.getCheckpointLocation();
+ boolean checkpointSuccessful = false;
+ try {
+ try (OmMetadataManagerImpl metadataManagerBeforeTruncate =
+ createDefragCheckpointMetadataManager(checkpoint, false)) {
+ DBStore dbStore = metadataManagerBeforeTruncate.getStore();
+ for (String table : metadataManagerBeforeTruncate.listTableNames()) {
+ if (!incrementalColumnFamilies.contains(table)) {
+ dbStore.dropTable(table);
+ }
+ }
+ } catch (Exception e) {
+ throw new IOException("Failed to prepare defrag checkpoint for
snapshot: " + snapshotInfo.getSnapshotId(), e);
+ }
+ // This will recreate the column families in the checkpoint.
+ OmMetadataManagerImpl result =
createDefragCheckpointMetadataManager(checkpoint, false);
+ checkpointSuccessful = true;
+ return result;
+ } finally {
+ if (!checkpointSuccessful && Files.exists(checkpointLocation)) {
+ try {
+ deleteDirectory(checkpointLocation);
+ } catch (IOException cleanupException) {
+ snapshotMetrics.incNumSnapshotDefragFails();
+ LOG.error("Failed to clean up checkpoint directory {} for
snapshot: {} (ID: {}). " +
Review Comment:
Actually, I realized that when exception is caught, this would be double
counting failures because triggerSnapshotDefragOnce() is already counting it:
https://github.com/cchung100m/ozone/blob/HDDS-13173/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/defrag/SnapshotDefragService.java#L851-L853
```suggestion
} catch (Exception e) {
throw new IOException("Failed to prepare defrag checkpoint for
snapshot: " + snapshotInfo.getSnapshotId(), e);
}
// This will recreate the column families in the checkpoint.
OmMetadataManagerImpl result =
createDefragCheckpointMetadataManager(checkpoint, false);
checkpointSuccessful = true;
return result;
} finally {
if (!checkpointSuccessful && Files.exists(checkpointLocation)) {
try {
deleteDirectory(checkpointLocation);
} catch (IOException cleanupException) {
LOG.error("Failed to clean up checkpoint directory {} for
snapshot: {} (ID: {}). " +
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
