bharatviswa504 commented on a change in pull request #986:
URL: https://github.com/apache/hadoop-ozone/pull/986#discussion_r438434496



##########
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerStateMachine.java
##########
@@ -338,20 +352,19 @@ public void unpause(long newLastAppliedSnaphsotIndex,
   }
 
   /**
-   * Take OM Ratis snapshot. Write the snapshot index to file. Snapshot index
-   * is the log index corresponding to the last applied transaction on the OM
-   * State Machine.
+   * Take OM Ratis snapshot is a dummy operation as when double buffer
+   * flushes the lastAppliedIndex is flushed to DB and that is used as
+   * snapshot index.
    *
    * @return the last applied index on the state machine which has been
    * stored in the snapshot file.
    */
   @Override
   public long takeSnapshot() throws IOException {
-    LOG.info("Saving Ratis snapshot on the OM.");
-    if (ozoneManager != null) {
-      return ozoneManager.saveRatisSnapshot().getIndex();
-    }
-    return 0;
+    LOG.info("Current Snapshot Index {}", getLastAppliedTermIndex());
+    long lastAppliedIndex = getLastAppliedTermIndex().getIndex();
+    ozoneManager.getMetadataManager().getStore().flush();

Review comment:
       Done

##########
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerStateMachine.java
##########
@@ -515,13 +528,12 @@ private synchronized void 
computeAndUpdateLastAppliedIndex(
     }
   }
 
-  public void updateLastAppliedIndexWithSnaphsotIndex() {
+  public void updateLastAppliedIndexWithSnaphsotIndex() throws IOException {

Review comment:
       Done

##########
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -3168,8 +3172,8 @@ File replaceOMDBWithCheckpoint(long lastAppliedIndex, 
Path checkpointPath)
    * All the classes which use/ store MetadataManager should also be updated
    * with the new MetadataManager instance.
    */
-  void reloadOMState(long newSnapshotIndex,
-      long newSnapShotTermIndex) throws IOException {
+  void reloadOMState(long newSnapshotIndex, long newSnapShotTermIndex)
+      throws IOException {

Review comment:
       Done

##########
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -3033,32 +3024,47 @@ public TermIndex installSnapshot(String leaderId) {
     DBCheckpoint omDBcheckpoint = getDBCheckpointFromLeader(leaderId);
     Path newDBlocation = omDBcheckpoint.getCheckpointLocation();
 
-    // Check if current ratis log index is smaller than the downloaded
-    // snapshot index. If yes, proceed by stopping the ratis server so that
-    // the OM state can be re-initialized. If no, then do not proceed with
-    // installSnapshot.
+    LOG.info("Downloaded checkpoint from Leader {}, in to the location {}",
+        leaderId, newDBlocation);
+
     long lastAppliedIndex = omRatisServer.getLastAppliedTermIndex().getIndex();
-    long checkpointSnapshotIndex = omDBcheckpoint.getRatisSnapshotIndex();
-    long checkpointSnapshotTermIndex =
-        omDBcheckpoint.getRatisSnapshotTerm();
-    if (checkpointSnapshotIndex <= lastAppliedIndex) {
-      LOG.error("Failed to install checkpoint from OM leader: {}. The last " +
-              "applied index: {} is greater than or equal to the checkpoint's"
-              + " " +
-              "snapshot index: {}. Deleting the downloaded checkpoint {}",
-          leaderId,
-          lastAppliedIndex, checkpointSnapshotIndex,
+
+    // Check if current ratis log index is smaller than the downloaded
+    // checkpoint transaction index. If yes, proceed by stopping the ratis
+    // server so that the OM state can be re-initialized. If no, then do not
+    // proceed with installSnapshot.
+
+    OMTransactionInfo omTransactionInfo = null;
+
+    Path dbDir = newDBlocation.getParent();
+    if (dbDir == null) {
+      LOG.error("Incorrect DB location path {} received from checkpoint.",
           newDBlocation);
-      try {
-        FileUtils.deleteFully(newDBlocation);
-      } catch (IOException e) {
-        LOG.error("Failed to fully delete the downloaded DB checkpoint {} " +
-                "from OM leader {}.", newDBlocation,
-            leaderId, e);
-      }
       return null;
     }
 
+    try {
+      omTransactionInfo =
+          OzoneManagerRatisUtils.getTransactionInfoFromDownloadedSnapshot(
+              configuration, dbDir);
+    } catch (Exception ex) {
+      LOG.error("Failed during opening downloaded snapshot from " +
+          "{} to obtain transaction index", newDBlocation, ex);
+      return null;
+    }
+
+    boolean canProceed =
+        OzoneManagerRatisUtils.verifyTransactionInfo(omTransactionInfo,
+        lastAppliedIndex, leaderId, newDBlocation);
+

Review comment:
       Done.

##########
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
##########
@@ -259,16 +261,25 @@ public void start(OzoneConfiguration configuration) 
throws IOException {
         rocksDBConfiguration.setSyncOption(true);
       }
 
-      DBStoreBuilder dbStoreBuilder = DBStoreBuilder.newBuilder(configuration,
-          rocksDBConfiguration).setName(OM_DB_NAME)
-          .setPath(Paths.get(metaDir.getPath()));
+      this.store = loadDB(configuration, metaDir);
 
-      this.store = addOMTablesAndCodecs(dbStoreBuilder).build();
+      // This value will be used internally, not to be exposed to end users.

Review comment:
       Yes removed




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

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org

Reply via email to