ChenSammi commented on code in PR #11021:
URL: https://github.com/apache/ozone/pull/11021#discussion_r3813478215


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyLifecycleService.java:
##########
@@ -1536,19 +1544,20 @@ public OzoneManagerProtocolProtos.OMResponse run() 
throws Exception {
       }
     }
 
-    private void moveToTrash(OmBucketInfo bucket, LimitedExpiredObjectList 
keysList, boolean isDir) {
+    private int moveToTrash(OmBucketInfo bucket, LimitedExpiredObjectList 
keysList, boolean isDir) {
       if (keysList.isEmpty()) {
-        return;
+        return 0;
       }
+      int failedMoves = 0;
       String volumeName = bucket.getVolumeName();
       String bucketName = bucket.getBucketName();
       String trashRoot = TRASH_PREFIX + OM_KEY_PREFIX + bucket.getOwner();
       Path trashCurrent = new Path(trashRoot, CURRENT);
       try {
         checkAndCreateTrashDirIfNeeded(bucket, trashCurrent);
       } catch (IOException e) {
-        keysList.clear();
-        return;
+        LOG.error("Failed to prepare trash root {} for bucket {}/{}", 
trashCurrent, volumeName, bucketName, e);

Review Comment:
   @aryangupta1998 , thanks for working on the error handling improvement. 
   
   If Trash root cannot be created, let's just throw out the exception all the 
way to the task, and abort the whole task. 
   
   Apart this, I would suggest keep the current behavior.  Because there are 
more rename failure cases than the three in isMissingSourceStatus(), and they 
are not a problem too, such as UPDATE_ID_NOT_MATCH case, while it will cause 
the whole LifecycleActionTask abort, which is inappropriate.   If we add case 
by case filter in KeyLifecycleService, then any future changes in 
OMKeyRenameRequestWithFSO may break the logic in KeyLifecycleService, and abort 
the LifecycleActionTask unexpectedly. 



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