You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2020/06/02 05:58:11 UTC

[GitHub] [hadoop-ozone] prashantpogde opened a new pull request #1004: HDDS-3639. Maintain FileHandle Information in OMMetadataManager.

prashantpogde opened a new pull request #1004:
URL: https://github.com/apache/hadoop-ozone/pull/1004


   ## What changes were proposed in this pull request?
   
   Maintain FileHandle Information in OMMetadataManager
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-3639
   
   ## How was this patch tested?
   
   Clean build and UTs.
   


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



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

[GitHub] [hadoop-ozone] hanishakoneru commented on a change in pull request #1004: HDDS-3639. Maintain FileHandle Information in OMMetadataManager.

Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #1004:
URL: https://github.com/apache/hadoop-ozone/pull/1004#discussion_r437726086



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyRenameResponse.java
##########
@@ -93,6 +93,15 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
       omMetadataManager.getKeyTable().putWithBatch(batchOperation,

Review comment:
       Let me elaborate on the scenario I am talking about:
     Trxn n1 : Create Key - key1
     Trxn n2 : Commit Key - key1 (adds n1 -> key1 entry to KeyID table)
     Trxn n3 : Rename key1 -> key2 (update n1 -> key2 in KeyID table)
   
   Now lets say we replay transactions from n1. 
     Replay Trxn n1 : Create Key - key1 (key1 does not exist so this transaction is replayed)
     Replay Trxn n2 : Commit Key - key1 (this again updates n1 -> key1 in KeyID table)
     Replay Trxn n3 : Since key2 exists, we figure out this is replay and execute 
                                 deleteFromKeyOnly() logic in addToBatch.
   
   So finally after replay, we have n1 in KeyID table pointing to key1 instead of key2.
   
   > I also noticed that the sequence of delete and newkey addition was reversed before this change. Not sure how this worked.
   
   The order does not matter here as they are part of batch operation.




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



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

[GitHub] [hadoop-ozone] bharatviswa504 commented on pull request #1004: HDDS-3639. Maintain FileHandle Information in OMMetadataManager.

Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #1004:
URL: https://github.com/apache/hadoop-ozone/pull/1004#issuecomment-640846363


   > Thanks @prashantpogde for working on this. I have posted my comments inline.
   > 
   > @bharatviswa504, I just wanted to confirm that for S3 Multipart, all the partKeys will only be part of the main key in Key Table and we won't have individual part keys in the KeyTable. Also, after Multipart upload is completed, the MultipartKey would be deleted from getMultipartInfoTable, right?
   
   
   all the partKeys will only be part of the main key in Key Table and we won't have individual part keys in the KeyTable. 
   
   I think here you mean openKeyTable. If yes, it is right. It will be in openKeyTable until the part is committed.
    And also individual parts will be in openKeyTable, until they are committed. Once committed, they will be deleted from OpenKey table and moved to MultipartInfo Table.
   
   Also, after Multipart upload is completed, the MultipartKey would be deleted from getMultipartInfoTable, right?
   Yes, you are right.
   
   


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



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

[GitHub] [hadoop-ozone] bharatviswa504 edited a comment on pull request #1004: HDDS-3639. Maintain FileHandle Information in OMMetadataManager.

Posted by GitBox <gi...@apache.org>.
bharatviswa504 edited a comment on pull request #1004:
URL: https://github.com/apache/hadoop-ozone/pull/1004#issuecomment-640846363


   > Thanks @prashantpogde for working on this. I have posted my comments inline.
   > 
   > @bharatviswa504, I just wanted to confirm that for S3 Multipart, all the partKeys will only be part of the main key in Key Table and we won't have individual part keys in the KeyTable. Also, after Multipart upload is completed, the MultipartKey would be deleted from getMultipartInfoTable, right?
   
   
   > all the partKeys will only be part of the main key in Key Table and we won't have individual part keys in the KeyTable. 
   
   I think here you mean openKeyTable. If yes, it is right. It will be in openKeyTable until the part is committed.
    And also individual parts will be in openKeyTable, until they are committed. Once committed, they will be deleted from OpenKey table and moved to MultipartInfo Table.
   
   > Also, after Multipart upload is completed, the MultipartKey would be deleted from getMultipartInfoTable, right?
   
   Yes, you are right.
   
   


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



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

[GitHub] [hadoop-ozone] prashantpogde commented on a change in pull request #1004: HDDS-3639. Maintain FileHandle Information in OMMetadataManager.

Posted by GitBox <gi...@apache.org>.
prashantpogde commented on a change in pull request #1004:
URL: https://github.com/apache/hadoop-ozone/pull/1004#discussion_r434276194



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyRenameResponse.java
##########
@@ -88,11 +88,22 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
           omMetadataManager.getOzoneKey(volumeName, bucketName, fromKeyName));
     } else if (createToKeyAndDeleteFromKey()) {
       // If both from and toKeyName are equal do nothing
-      omMetadataManager.getKeyTable().deleteWithBatch(batchOperation,
-          omMetadataManager.getOzoneKey(volumeName, bucketName, fromKeyName));
-      omMetadataManager.getKeyTable().putWithBatch(batchOperation,
-          omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName),
-          newKeyInfo);
+      if (!toKeyName.equals(fromKeyName)) {

Review comment:
       Yes, the check is there in createToKeyAndDeleteFromKey() and it is redundant. I will remove the check from here and upload again.




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



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

[GitHub] [hadoop-ozone] hanishakoneru commented on a change in pull request #1004: HDDS-3639. Maintain FileHandle Information in OMMetadataManager.

Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #1004:
URL: https://github.com/apache/hadoop-ozone/pull/1004#discussion_r435465333



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java
##########
@@ -103,6 +103,16 @@
 
   String getOzoneKey(String volume, String bucket, String key);
 
+  /**
+   * Given a volume, bucket, return the corresponding DB key.
+   *
+   * @param fileHandleInfo - unique keyId that is used by NFS to create a

Review comment:
       The Javadoc needs to be updated here. We are not passing the volume and bucket

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyRenameResponse.java
##########
@@ -93,6 +93,15 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
       omMetadataManager.getKeyTable().putWithBatch(batchOperation,
           omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName),
           newKeyInfo);
+      // At this point we can also update the KeyIdTable.
+      if (newKeyInfo.getFileHandleInfo() != 0) {
+        omMetadataManager.getKeyIdTable().putWithBatch(batchOperation,
+            omMetadataManager.getOzoneKeyIdTableKey(
+                newKeyInfo.getFileHandleInfo()), toKeyName);
+        omMetadataManager.getKeyIdTable().deleteWithBatch(batchOperation,
+            omMetadataManager.getOzoneKeyIdTableKey(
+                newKeyInfo.getFileHandleInfo()));

Review comment:
       We are putting and deleting the same key from KeyID Table. 
   The delete operation is not required here as keyId -> fromKey will be overridden with keyId -> toKey.

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
##########
@@ -118,12 +118,19 @@
    * |----------------------------------------------------------------------|
    * |  multipartInfoTable| /volumeName/bucketName/keyName/uploadId ->...   |
    * |----------------------------------------------------------------------|
+   * | keyIdTable         | /volumeName/bucketName/keyId -> KeyName         |
+   * |----------------------------------------------------------------------|
+   *
+   * TBD : Renames need to be made keyIdTable aware. Also KeyId based lookups
+   * should be able to handle any possible race with renames/deletes.
+   *
    */
 
   public static final String USER_TABLE = "userTable";
   public static final String VOLUME_TABLE = "volumeTable";
   public static final String BUCKET_TABLE = "bucketTable";
   public static final String KEY_TABLE = "keyTable";
+  public static final String KEY_ID_TABLE = "keyIdTable";

Review comment:
       Just a thought - should this table be named something else like FileHandle Table to avoid confusing this with Key Table? (It's just a thought. Please feel free to ignore if you think "KeyId" is the more appropriate name).

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/file/OMDirectoryCreateResponse.java
##########
@@ -81,6 +81,15 @@ protected void addToDBBatch(OMMetadataManager omMetadataManager,
       omMetadataManager.getKeyTable().putWithBatch(batchOperation, dirKey,
           dirKeyInfo);
 
+      // We can also persist the fileHandle to KeyIdTable.
+      if (dirKeyInfo.getFileHandleInfo() != 0) {
+        omMetadataManager.getKeyIdTable().putWithBatch(batchOperation,
+            omMetadataManager.getOzoneKeyIdTableKey(
+                dirKeyInfo.getFileHandleInfo()),
+            omMetadataManager.getOzoneKey(dirKeyInfo.getVolumeName(),
+                dirKeyInfo.getBucketName(),
+                dirKeyInfo.getKeyName()));

Review comment:
       OzoneKey is already processed in line 79 (dirKey). That can be reused here.

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyRenameResponse.java
##########
@@ -93,6 +93,15 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
       omMetadataManager.getKeyTable().putWithBatch(batchOperation,

Review comment:
       In the if case (deleteFromKeyOnly()) - toKey already exists but fromKey is created as part of replay. When the fromKey is being created, it would update KeyID to fromKey value. So KeyId Table should be updated with correct value (toKey) for this condition also.
   
   @bharatviswa504  is working on replay optimization after which we would not require to take care of replay scenarios. But let's fix it for now anyway till the old replay code is still in effect.

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3InitiateMultipartUploadRequest.java
##########
@@ -156,6 +156,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
           .setReplicationFactor(keyArgs.getFactor())
           .setObjectID(objectID)
           .setUpdateID(transactionLogIndex)
+          .setFileHandleInfo(objectID)

Review comment:
       FileHandleInfo in OmMultipartKeyInfo object is not used anywhere. 

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyCreateResponse.java
##########
@@ -87,6 +87,15 @@ protected void addToDBBatch(OMMetadataManager omMetadataManager,
         omKeyInfo.getBucketName(), omKeyInfo.getKeyName(), openKeySessionID);
     omMetadataManager.getOpenKeyTable().putWithBatch(batchOperation,
         openKey, omKeyInfo);
+
+    // We can also persist the fileHandle to KeyIdTable.
+    if (omKeyInfo.getFileHandleInfo() != 0) {
+      omMetadataManager.getKeyIdTable().putWithBatch(batchOperation,
+          omMetadataManager.getOzoneKeyIdTableKey(
+              omKeyInfo.getFileHandleInfo()),
+          omMetadataManager.getOzoneKey(omKeyInfo.getVolumeName(),
+              omKeyInfo.getBucketName(), omKeyInfo.getKeyName()));
+    }

Review comment:
       This keyId be added when the key is actually getting committed i.e. in OMKeyCommitResponse#validateAndUpdateCache()? Otherwise, KeyID could point to a key that does not exist.
   

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/multipart/S3InitiateMultipartUploadResponse.java
##########
@@ -69,6 +69,16 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
         multipartKey, omKeyInfo);
     omMetadataManager.getMultipartInfoTable().putWithBatch(batchOperation,
         multipartKey, omMultipartKeyInfo);
+
+
+    if (omKeyInfo.getFileHandleInfo() != 0) {
+      omMetadataManager.getKeyIdTable().putWithBatch(batchOperation,
+          omMetadataManager.getOzoneKeyIdTableKey(
+              omKeyInfo.getFileHandleInfo()),
+          omMetadataManager.getOzoneKey(omKeyInfo.getVolumeName(),
+              omKeyInfo.getBucketName(),
+              omKeyInfo.getKeyName()));
+    }

Review comment:
       This keyId be added when the key is actually getting committed i.e. in S3MultipartUploadComplete#validateAndUpdateCache()? Otherwise, KeyID could point to a key that does not exist.




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



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

[GitHub] [hadoop-ozone] prashantpogde commented on a change in pull request #1004: HDDS-3639. Maintain FileHandle Information in OMMetadataManager.

Posted by GitBox <gi...@apache.org>.
prashantpogde commented on a change in pull request #1004:
URL: https://github.com/apache/hadoop-ozone/pull/1004#discussion_r436884838



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java
##########
@@ -103,6 +103,16 @@
 
   String getOzoneKey(String volume, String bucket, String key);
 
+  /**
+   * Given a volume, bucket, return the corresponding DB key.
+   *
+   * @param fileHandleInfo - unique keyId that is used by NFS to create a

Review comment:
       yup, done.

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
##########
@@ -118,12 +118,19 @@
    * |----------------------------------------------------------------------|
    * |  multipartInfoTable| /volumeName/bucketName/keyName/uploadId ->...   |
    * |----------------------------------------------------------------------|
+   * | keyIdTable         | /volumeName/bucketName/keyId -> KeyName         |
+   * |----------------------------------------------------------------------|
+   *
+   * TBD : Renames need to be made keyIdTable aware. Also KeyId based lookups
+   * should be able to handle any possible race with renames/deletes.
+   *
    */
 
   public static final String USER_TABLE = "userTable";
   public static final String VOLUME_TABLE = "volumeTable";
   public static final String BUCKET_TABLE = "bucketTable";
   public static final String KEY_TABLE = "keyTable";
+  public static final String KEY_ID_TABLE = "keyIdTable";

Review comment:
       If you insist, I will change :)

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/file/OMDirectoryCreateResponse.java
##########
@@ -81,6 +81,15 @@ protected void addToDBBatch(OMMetadataManager omMetadataManager,
       omMetadataManager.getKeyTable().putWithBatch(batchOperation, dirKey,
           dirKeyInfo);
 
+      // We can also persist the fileHandle to KeyIdTable.
+      if (dirKeyInfo.getFileHandleInfo() != 0) {
+        omMetadataManager.getKeyIdTable().putWithBatch(batchOperation,
+            omMetadataManager.getOzoneKeyIdTableKey(
+                dirKeyInfo.getFileHandleInfo()),
+            omMetadataManager.getOzoneKey(dirKeyInfo.getVolumeName(),
+                dirKeyInfo.getBucketName(),
+                dirKeyInfo.getKeyName()));

Review comment:
       yup, done.

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyCreateResponse.java
##########
@@ -87,6 +87,15 @@ protected void addToDBBatch(OMMetadataManager omMetadataManager,
         omKeyInfo.getBucketName(), omKeyInfo.getKeyName(), openKeySessionID);
     omMetadataManager.getOpenKeyTable().putWithBatch(batchOperation,
         openKey, omKeyInfo);
+
+    // We can also persist the fileHandle to KeyIdTable.
+    if (omKeyInfo.getFileHandleInfo() != 0) {
+      omMetadataManager.getKeyIdTable().putWithBatch(batchOperation,
+          omMetadataManager.getOzoneKeyIdTableKey(
+              omKeyInfo.getFileHandleInfo()),
+          omMetadataManager.getOzoneKey(omKeyInfo.getVolumeName(),
+              omKeyInfo.getBucketName(), omKeyInfo.getKeyName()));
+    }

Review comment:
       yes, moved to OmKeyCommitResponse.

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java
##########
@@ -103,6 +103,16 @@
 
   String getOzoneKey(String volume, String bucket, String key);
 
+  /**
+   * Given a volume, bucket, return the corresponding DB key.
+   *
+   * @param fileHandleInfo - unique keyId that is used by NFS to create a

Review comment:
       @hanishakoneru thank you for reviewing the changes.
   Yes, updated this.

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyRenameResponse.java
##########
@@ -93,6 +93,15 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
       omMetadataManager.getKeyTable().putWithBatch(batchOperation,
           omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName),
           newKeyInfo);
+      // At this point we can also update the KeyIdTable.
+      if (newKeyInfo.getFileHandleInfo() != 0) {
+        omMetadataManager.getKeyIdTable().putWithBatch(batchOperation,
+            omMetadataManager.getOzoneKeyIdTableKey(
+                newKeyInfo.getFileHandleInfo()), toKeyName);
+        omMetadataManager.getKeyIdTable().deleteWithBatch(batchOperation,
+            omMetadataManager.getOzoneKeyIdTableKey(
+                newKeyInfo.getFileHandleInfo()));

Review comment:
       yup, I have changed this.

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyRenameResponse.java
##########
@@ -93,6 +93,15 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
       omMetadataManager.getKeyTable().putWithBatch(batchOperation,

Review comment:
       Not sure, if we need to do this. 
   - In case of replay its only deleting fromKey, in this case we do not have the toKeyName available, its NULL,
   - In the first tie, it seems the sequence would be : 
       - update mapping id <-> toKeyname
       - add toKey
       - delete FromKey
   
   So in case of replay if the newkey is already there then we should also have the id-<->keyname mapping updated.
   
   I also noticed that the sequence of delete and newkey addition was reversed before this change. Not sure how this worked.

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3InitiateMultipartUploadRequest.java
##########
@@ -156,6 +156,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
           .setReplicationFactor(keyArgs.getFactor())
           .setObjectID(objectID)
           .setUpdateID(transactionLogIndex)
+          .setFileHandleInfo(objectID)

Review comment:
       Let us keep this for now. If it becomes redundant, I will remove this with subsequent PRs.

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/multipart/S3InitiateMultipartUploadResponse.java
##########
@@ -69,6 +69,16 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
         multipartKey, omKeyInfo);
     omMetadataManager.getMultipartInfoTable().putWithBatch(batchOperation,
         multipartKey, omMultipartKeyInfo);
+
+
+    if (omKeyInfo.getFileHandleInfo() != 0) {
+      omMetadataManager.getKeyIdTable().putWithBatch(batchOperation,
+          omMetadataManager.getOzoneKeyIdTableKey(
+              omKeyInfo.getFileHandleInfo()),
+          omMetadataManager.getOzoneKey(omKeyInfo.getVolumeName(),
+              omKeyInfo.getBucketName(),
+              omKeyInfo.getKeyName()));
+    }

Review comment:
       Yup, Updated the changes.




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



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

[GitHub] [hadoop-ozone] prashantpogde commented on a change in pull request #1004: HDDS-3639. Maintain FileHandle Information in OMMetadataManager.

Posted by GitBox <gi...@apache.org>.
prashantpogde commented on a change in pull request #1004:
URL: https://github.com/apache/hadoop-ozone/pull/1004#discussion_r434276036



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyRenameResponse.java
##########
@@ -88,11 +88,22 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
           omMetadataManager.getOzoneKey(volumeName, bucketName, fromKeyName));
     } else if (createToKeyAndDeleteFromKey()) {
       // If both from and toKeyName are equal do nothing
-      omMetadataManager.getKeyTable().deleteWithBatch(batchOperation,
-          omMetadataManager.getOzoneKey(volumeName, bucketName, fromKeyName));
-      omMetadataManager.getKeyTable().putWithBatch(batchOperation,
-          omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName),
-          newKeyInfo);
+      if (!toKeyName.equals(fromKeyName)) {
+        omMetadataManager.getKeyTable().deleteWithBatch(batchOperation,
+            omMetadataManager.getOzoneKey(volumeName, bucketName, fromKeyName));
+        omMetadataManager.getKeyTable().putWithBatch(batchOperation,
+            omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName),
+            newKeyInfo);
+        // At this point we can also update the KeyIdTable.
+        if (newKeyInfo.getFileHandleInfo() != 0) {

Review comment:
       This means we never created the file handle for this key. The check is there to take care of this case. Eventually we should have file handle for all keys.




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



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

[GitHub] [hadoop-ozone] hanishakoneru commented on pull request #1004: HDDS-3639. Maintain FileHandle Information in OMMetadataManager.

Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on pull request #1004:
URL: https://github.com/apache/hadoop-ozone/pull/1004#issuecomment-641677419


   Thanks for working on this @prashantpogde. 
   +1 for merging into branch HDDS-3001, pending CI.


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



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

[GitHub] [hadoop-ozone] hanishakoneru commented on a change in pull request #1004: HDDS-3639. Maintain FileHandle Information in OMMetadataManager.

Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #1004:
URL: https://github.com/apache/hadoop-ozone/pull/1004#discussion_r437820153



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyRenameResponse.java
##########
@@ -93,6 +93,15 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
       omMetadataManager.getKeyTable().putWithBatch(batchOperation,

Review comment:
       Thanks @prashantpogde for the offline discussion. 
   It will be lot of extra work updating the keyID in deleteFromKeyOnly case also as we don't send the toKey name in this case. It is safe to go ahead with this approach as Bharat's replay optimization - HDDS-3354 will take care of this.




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



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

[GitHub] [hadoop-ozone] avijayanhwx commented on a change in pull request #1004: HDDS-3639. Maintain FileHandle Information in OMMetadataManager.

Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #1004:
URL: https://github.com/apache/hadoop-ozone/pull/1004#discussion_r434125451



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyRenameResponse.java
##########
@@ -88,11 +88,22 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
           omMetadataManager.getOzoneKey(volumeName, bucketName, fromKeyName));
     } else if (createToKeyAndDeleteFromKey()) {
       // If both from and toKeyName are equal do nothing
-      omMetadataManager.getKeyTable().deleteWithBatch(batchOperation,
-          omMetadataManager.getOzoneKey(volumeName, bucketName, fromKeyName));
-      omMetadataManager.getKeyTable().putWithBatch(batchOperation,
-          omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName),
-          newKeyInfo);
+      if (!toKeyName.equals(fromKeyName)) {

Review comment:
       Isn't this check already done inside createToKeyAndDeleteFromKey()?

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyRenameResponse.java
##########
@@ -88,11 +88,22 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
           omMetadataManager.getOzoneKey(volumeName, bucketName, fromKeyName));
     } else if (createToKeyAndDeleteFromKey()) {
       // If both from and toKeyName are equal do nothing
-      omMetadataManager.getKeyTable().deleteWithBatch(batchOperation,
-          omMetadataManager.getOzoneKey(volumeName, bucketName, fromKeyName));
-      omMetadataManager.getKeyTable().putWithBatch(batchOperation,
-          omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName),
-          newKeyInfo);
+      if (!toKeyName.equals(fromKeyName)) {
+        omMetadataManager.getKeyTable().deleteWithBatch(batchOperation,
+            omMetadataManager.getOzoneKey(volumeName, bucketName, fromKeyName));
+        omMetadataManager.getKeyTable().putWithBatch(batchOperation,
+            omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName),
+            newKeyInfo);
+        // At this point we can also update the KeyIdTable.
+        if (newKeyInfo.getFileHandleInfo() != 0) {

Review comment:
       What does it mean for the fileHandleInfo to be zero? If it is zero, we do not delete the old Id from the keyIdTable?




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



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

[GitHub] [hadoop-ozone] avijayanhwx commented on a change in pull request #1004: HDDS-3639. Maintain FileHandle Information in OMMetadataManager.

Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #1004:
URL: https://github.com/apache/hadoop-ozone/pull/1004#discussion_r435460407



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyRenameResponse.java
##########
@@ -88,11 +88,22 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
           omMetadataManager.getOzoneKey(volumeName, bucketName, fromKeyName));
     } else if (createToKeyAndDeleteFromKey()) {
       // If both from and toKeyName are equal do nothing
-      omMetadataManager.getKeyTable().deleteWithBatch(batchOperation,
-          omMetadataManager.getOzoneKey(volumeName, bucketName, fromKeyName));
-      omMetadataManager.getKeyTable().putWithBatch(batchOperation,
-          omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName),
-          newKeyInfo);
+      if (!toKeyName.equals(fromKeyName)) {
+        omMetadataManager.getKeyTable().deleteWithBatch(batchOperation,
+            omMetadataManager.getOzoneKey(volumeName, bucketName, fromKeyName));
+        omMetadataManager.getKeyTable().putWithBatch(batchOperation,
+            omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName),
+            newKeyInfo);
+        // At this point we can also update the KeyIdTable.
+        if (newKeyInfo.getFileHandleInfo() != 0) {

Review comment:
       Got it, thanks!




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



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