Xushaohong commented on code in PR #3778:
URL: https://github.com/apache/ozone/pull/3778#discussion_r1001326570


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java:
##########
@@ -239,6 +243,26 @@ public OMClientResponse 
validateAndUpdateCache(OzoneManager ozoneManager,
         omBucketInfo.incrUsedNamespace(1L);
       }
 
+      // let the uncommitted blocks pretend as key's old version blocks
+      // which will be deleted as RepeatedOmKeyInfo
+      if (!uncommitted.isEmpty()) {
+        LOG.info("Detect allocated but uncommitted blocks {} in key {}.",
+            uncommitted, keyName);
+        OmKeyInfo pseudoKeyInfo = omKeyInfo.copyObject();
+        // set dataSize -1 here to distinguish from normal keyInfo
+        pseudoKeyInfo.setDataSize(-1);

Review Comment:
   Setting -1 here is to have an explicit difference with normal keyInfo.
   1. The pseudoKeyInfo is copied from the keyInfo, so the data size comes from 
the client and is the real key length, which is irrelevant to these uncommitted 
blocks. If we don't reset this, it may get confusing with **OmKeyInfo** which 
has real key block info.
   2. It could be a debug trace.
   So I think it is good to have~



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java:
##########
@@ -239,6 +243,26 @@ public OMClientResponse 
validateAndUpdateCache(OzoneManager ozoneManager,
         omBucketInfo.incrUsedNamespace(1L);
       }
 
+      // let the uncommitted blocks pretend as key's old version blocks
+      // which will be deleted as RepeatedOmKeyInfo
+      if (!uncommitted.isEmpty()) {
+        LOG.info("Detect allocated but uncommitted blocks {} in key {}.",
+            uncommitted, keyName);
+        OmKeyInfo pseudoKeyInfo = omKeyInfo.copyObject();
+        // set dataSize -1 here to distinguish from normal keyInfo
+        pseudoKeyInfo.setDataSize(-1);

Review Comment:
   Setting -1 here is to have an explicit difference with normal keyInfo.
   1. The pseudoKeyInfo is copied from the keyInfo, so the data size comes from 
the client and is the real key length, which is irrelevant to these uncommitted 
blocks. If we don't reset this, it may get confusing with **OmKeyInfo** which 
has real key block info.
   2. It could be a debug trace.
   
   So I think it is good to have~



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