swamirishi commented on code in PR #9132:
URL: https://github.com/apache/ozone/pull/9132#discussion_r2594527554
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -4240,89 +4240,118 @@ private void stopTrashEmptier() {
}
/**
- * Replace the current OM DB with the new DB checkpoint.
+ * Replaces the OM DB with checkpoint data from leader.
+ * Creates a single parent backup directory containing all current state.
*
- * @param lastAppliedIndex the last applied index in the current OM DB.
- * @param checkpointPath path to the new DB checkpoint
- * @return location of backup of the original DB
+ * @param lastAppliedIndex last applied transaction index
+ * @param oldDB current DB directory
+ * @param checkpointLocation checkpoint data directory from leader
+ * @return backup directory containing original state
+ * @throws IOException if operations fail
*/
File replaceOMDBWithCheckpoint(long lastAppliedIndex, File oldDB,
- Path checkpointPath) throws IOException {
+ Path checkpointLocation) throws IOException {
- // Take a backup of the current DB
+ // Create single parent backup directory
String dbBackupName = OzoneConsts.OM_DB_BACKUP_PREFIX +
lastAppliedIndex + "_" + System.currentTimeMillis();
File dbDir = oldDB.getParentFile();
-
- // Backup the active fs and snapshot dirs.
File dbBackupDir = new File(dbDir, dbBackupName);
+
if (!dbBackupDir.mkdirs()) {
- throw new IOException("Failed to make db backup dir: " +
- dbBackupDir);
+ throw new IOException("Failed to create backup directory: " +
dbBackupDir);
}
- File dbBackup = new File(dbBackupDir, oldDB.getName());
- File dbSnapshotsDir = new File(dbDir, OM_SNAPSHOT_DIR);
- File dbSnapshotsBackup = new File(dbBackupDir, OM_SNAPSHOT_DIR);
- Files.move(oldDB.toPath(), dbBackup.toPath());
- if (dbSnapshotsDir.exists()) {
- Files.move(dbSnapshotsDir.toPath(),
- dbSnapshotsBackup.toPath());
+
+ // Move entire current state to backup (everything in dbDir that we care
about)
+ File[] currentContents = dbDir.listFiles();
+ if (currentContents != null) {
+ for (File item : currentContents) {
+ // Skip backup directories and marker files
+ if (item.getName().startsWith(OzoneConsts.OM_DB_BACKUP_PREFIX) ||
+ item.getName().equals(DB_TRANSIENT_MARKER)) {
+ continue;
+ }
+
+ // Move to backup - Files.move handles both files and directories
recursively
+ Path targetPath = dbBackupDir.toPath().resolve(item.getName());
+ Files.move(item.toPath(), targetPath,
StandardCopyOption.REPLACE_EXISTING);
+ }
}
- moveCheckpointFiles(oldDB, checkpointPath, dbDir, dbBackup, dbSnapshotsDir,
- dbSnapshotsBackup);
+ // Move checkpoint files
+ moveCheckpointFiles(oldDB, checkpointLocation, dbDir, dbBackupDir);
+
return dbBackupDir;
}
- private void moveCheckpointFiles(File oldDB, Path checkpointPath, File dbDir,
- File dbBackup, File dbSnapshotsDir,
- File dbSnapshotsBackup) throws IOException {
- // Move the new DB checkpoint into the om metadata dir
+ /**
+ * Moves all contents from checkpointLocation to dbDir, replacing existing
files/dirs.
+ * Uses a single parent backup for rollback on failure.
+ *
+ * @param oldDB the old DB directory (will be replaced)
+ * @param checkpointLocation source directory containing checkpoint data
+ * @param dbDir target directory (parent of oldDB)
+ * @param dbBackupDir backup directory containing the original state
+ * @throws IOException if file operations fail
+ */
+ private void moveCheckpointFiles(File oldDB, Path checkpointLocation, File
dbDir,
+ File dbBackupDir) throws IOException {
Path markerFile = new File(dbDir, DB_TRANSIENT_MARKER).toPath();
+
try {
- // Create a Transient Marker file. This file will be deleted if the
- // checkpoint DB is successfully moved to the old DB location or if the
- // old DB backup is reset to its location. If not, then the OM DB is in
- // an inconsistent state and this marker file will fail OM from
- // starting up.
+ // Create transient marker file
Files.createFile(markerFile);
- // Link each of the candidate DB files to real DB directory. This
- // preserves the links that already exist between files in the
- // candidate db.
- OmSnapshotUtils.linkFiles(checkpointPath.toFile(),
- oldDB);
- moveOmSnapshotData(oldDB.toPath(), dbSnapshotsDir.toPath());
+ // Move everything from checkpointLocation to dbDir, replacing existing
+ if (!Files.exists(checkpointLocation) ||
!Files.isDirectory(checkpointLocation)) {
+ throw new IOException("Checkpoint data directory does not exist: " +
checkpointLocation);
+ }
+ try (Stream<Path> checkpointContents = Files.list(checkpointLocation)) {
+ for (Path sourcePath :
checkpointContents.collect(Collectors.toList())) {
+ Path targetPath = dbDir.toPath().resolve(sourcePath.getFileName());
+ // Delete target if it exists (file or directory)
+ if (Files.exists(targetPath)) {
Review Comment:
Rollback won't happen if IllegalStateException is thrown. Throw IOException
instead
--
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]
