jojochuang commented on code in PR #9132:
URL: https://github.com/apache/ozone/pull/9132#discussion_r2566641934


##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java:
##########
@@ -378,9 +379,12 @@ public void testHardLinkCreation() throws IOException {
     Files.move(hardLinkList, Paths.get(candidateDir.toString(),
         OM_HARDLINK_FILE));
 
+    Path snapshot2Path = Paths.get(candidateDir.getParent(), 
OM_CHECKPOINT_DATA_DIR,
+        OM_SNAPSHOT_CHECKPOINT_DIR, followerSnapDir2.getName());
+
     // Pointers to follower links to be created.

Review Comment:
   moving from <dir>/om.db.candidate/db.snapshots/checkpointState/snap2 to 
   <dir>/checkpoint_data/db.snapshots/checkpointState/snap2



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -4046,7 +4049,17 @@ public synchronized TermIndex 
installSnapshotFromLeader(String leaderId) {
     try {
       // Install hard links.
       OmSnapshotUtils.createHardLinks(omDBCheckpoint.getCheckpointLocation(), 
false);
-      termIndex = installCheckpoint(leaderId, omDBCheckpoint);
+      Path checkpointLocation = omDBCheckpoint.getCheckpointLocation();
+      if (checkpointLocation == null) {
+        throw new IOException("Cannot install checkpoint from leader " + 
leaderId + ": checkpointLocation is null");
+      }
+      Path parent = checkpointLocation.getParent();
+      if (parent == null) {
+        throw new IOException("Cannot install checkpoint from leader " + 
leaderId +
+            ": checkpointLocation has no parent: " + checkpointLocation);
+      }
+      Path checkpointDataDir = Paths.get(parent.toString(), 
OM_CHECKPOINT_DATA_DIR);

Review Comment:
   suggest to make a omDBCheckpoint.getCheckpointDataDir() to compute this 
path, otherwise it's easy to forget why this is needed. And it can be reused in 
other places in this PR.



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/OzoneManagerServiceProviderImpl.java:
##########
@@ -197,11 +196,11 @@ public OzoneManagerServiceProviderImpl(
     HttpConfig.Policy policy = HttpConfig.getHttpPolicy(configuration);
 
     omDBSnapshotUrl = "http://"; + ozoneManagerHttpAddress +
-        OZONE_DB_CHECKPOINT_HTTP_ENDPOINT_V2;

Review Comment:
   Recon connects to the OM's old endpoint, since Recon doesn't support OM 
snapshots anyway.



##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOzoneManagerSnapshotProvider.java:
##########
@@ -117,12 +121,21 @@ public void testDownloadCheckpoint() throws Exception {
 
   private long getDownloadedSnapshotIndex(DBCheckpoint dbCheckpoint)
       throws Exception {
+    Path checkpointLocation = dbCheckpoint.getCheckpointLocation();
+    assertNotNull(checkpointLocation);
 
-    OmSnapshotUtils.createHardLinks(dbCheckpoint.getCheckpointLocation(), 
true);
+    OmSnapshotUtils.createHardLinks(checkpointLocation, true);
+    Path parent = checkpointLocation.getParent();
+    assertNotNull(parent);
+
+    Path omDbLocation = Paths.get(parent.toString(),

Review Comment:
   OMSnapshotUtils.createHardLinks() moved omDb to this location ( 
checkpointLocation/../checkpoint_data/om.db)
   
   similarly, snapshots are moved to 
checkpointLocation/../checkpoint_data/checkpointState



-- 
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]

Reply via email to