[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #452: HDDS-2894. Handle replay of KeyDelete and KeyRename Requests

[GitBox <gi...@apache.org>]

bharatviswa504 commented on a change in pull request #452: HDDS-2894. Handle replay of KeyDelete and KeyRename Requests
URL: https://github.com/apache/hadoop-ozone/pull/452#discussion_r372111521
 
 

 ##########
 File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java
 ##########
 @@ -133,79 +143,146 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
       // Validate bucket and volume exists or not.
       validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
 
-      // fromKeyName should exist
-      String fromKey = omMetadataManager.getOzoneKey(
-          volumeName, bucketName, fromKeyName);
-      fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
-      if (fromKeyValue == null) {
-        // TODO: Add support for renaming open key
-        throw new OMException("Key not found " + fromKey, KEY_NOT_FOUND);
-      }
-
-      // toKeyName should not exist
-      String toKey =
-          omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName);
+      // Check if toKey exists
+      fromKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
+          fromKeyName);
+      toKey = omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName);
       OmKeyInfo toKeyValue = omMetadataManager.getKeyTable().get(toKey);
-      if (toKeyValue != null) {
-        throw new OMException("Key already exists " + toKeyName,
-            OMException.ResultCodes.KEY_ALREADY_EXISTS);
-      }
-
-      fromKeyValue.setKeyName(toKeyName);
 
-      //Set modification time
-      fromKeyValue.setModificationTime(renameKeyArgs.getModificationTime());
-
-      // Set the UpdateID to current transactionLogIndex
-      fromKeyValue.setUpdateID(transactionLogIndex);
-
-      // Add to cache.
-      // fromKey should be deleted, toKey should be added with newly updated
-      // omKeyInfo.
-      Table<String, OmKeyInfo> keyTable = omMetadataManager.getKeyTable();
-
-      keyTable.addCacheEntry(new CacheKey<>(fromKey),
-          new CacheValue<>(Optional.absent(), transactionLogIndex));
-
-      keyTable.addCacheEntry(new CacheKey<>(toKey),
-          new CacheValue<>(Optional.of(fromKeyValue), transactionLogIndex));
+      if (toKeyValue != null) {
 
-      omClientResponse = new OMKeyRenameResponse(fromKeyValue, toKeyName,
-        fromKeyName, omResponse.setRenameKeyResponse(
-            RenameKeyResponse.newBuilder()).build());
-      success = true;
+        // Check if this transaction is a replay of ratis logs.
+        if (isReplay(ozoneManager, toKeyValue.getUpdateID(),
+            trxnLogIndex)) {
+
+          // Check if fromKey is still in the DB and created before this
+          // replay.
+          // For example, lets say we have the following sequence of
+          // transactions.
+          //     Trxn 1 : Create Key1
+          //     Trnx 2 : Rename Key1 to Key2 -> Deletes Key1 and Creates Key2
+          // Now if these transactions are replayed:
+          //     Replay Trxn 1 : Creates Key1 again as Key1 does not exist in DB
+          //     Replay Trxn 2 : Key2 is not created as it exists in DB and the
+          //                     request would be deemed a replay. But Key1
+          //                     is still in the DB and needs to be deleted.
+          fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
+          if (fromKeyValue != null) {
+            // Check if this replay transaction was after the fromKey was
+            // created. If so, we have to delete the fromKey.
+            if (ozoneManager.isRatisEnabled() &&
+                trxnLogIndex > fromKeyValue.getUpdateID()) {
+              // Add to cache. Only fromKey should be deleted. ToKey already
+              // exists in DB as this transaction is a replay.
+              result = Result.DELETE_FROM_KEY_ONLY;
+              Table<String, OmKeyInfo> keyTable = omMetadataManager
+                  .getKeyTable();
+              keyTable.addCacheEntry(new CacheKey<>(fromKey),
+                  new CacheValue<>(Optional.absent(), trxnLogIndex));
+
+              omClientResponse = new OMKeyRenameResponse(omResponse
+                  .setRenameKeyResponse(RenameKeyResponse.newBuilder()).build(),
+                  fromKeyName, fromKeyValue);
+            }
+          }
+
+          if (result == null) {
 
 Review comment:
   Minor: We can write this also else part of fromKeyValue != null. It will be easy to follow the flow.
   
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

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