Copilot commented on code in PR #11530:
URL: https://github.com/apache/ozone/pull/11530#discussion_r4268451794


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java:
##########
@@ -176,6 +176,12 @@ public OMClientResponse 
validateAndUpdateCache(OzoneManager ozoneManager, Execut
         throw new OMException("Key not found " + fromKey, KEY_NOT_FOUND);
       }
 
+      if (renameKeyRequest.hasUpdateID() && fromKeyValue.getUpdateID() != 
renameKeyRequest.getUpdateID()) {
+        throw new OMException("UpdateID does not match. Key: " + fromKeyName +
+            ", Expected UpdateID: " + fromKeyValue.getUpdateID() +
+            ", Given UpdateID: " + renameKeyRequest.getUpdateID(), 
OMException.ResultCodes.UPDATE_ID_NOT_MATCH);

Review Comment:
   The exception message uses `fromKeyName` (potentially only the leaf name) 
which can be ambiguous in logs/errors. Consider using the fully-qualified key 
identifier already available in this method (eg `fromKey` including 
volume/bucket) and formatting the message consistently (eg `expected=<...>, 
actual=<...>`) to make debugging easier for operators and clients.



##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyRenameRequest.java:
##########
@@ -91,6 +91,29 @@ public void testValidateAndUpdateCache() throws Exception {
     assertModificationTime(keyArgs.getModificationTime());
   }
 
+  @Test
+  public void testValidateAndUpdateCacheWithUpdateID() throws Exception {
+    String dbFromKey = addKeyToTable(fromKeyInfo);
+    long updateID = fromKeyInfo.getUpdateID();

Review Comment:
   This test derives the expected `updateID` from the in-memory `fromKeyInfo`, 
but the inserted/stored `OmKeyInfo` in the DB may not always be the same 
instance or may be mutated by helper methods in the future. To make the test 
more robust, consider reading the `updateID` back from the key table using 
`dbFromKey` (ie assert the row exists, then take its `getUpdateID()`), and use 
that value for both mismatch/match requests.



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