You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "priyeshkaratha (via GitHub)" <gi...@apache.org> on 2026/08/04 16:28:03 UTC

[PR] HDDS-16089. Improve MPU table evaluation efficiency in LifeCycleService [ozone]

priyeshkaratha opened a new pull request, #10949:
URL: https://github.com/apache/ozone/pull/10949

   ## What changes were proposed in this pull request?
   `KeyLifecycleService.processMultipartUploads` iterated every MPU in a bucket and immediately called `OMMultipartUploadUtils.getMultipartOpenKey` plus `openKeyTable.get(...)` before evaluating age or prefix. On buckets
   with many active uploads this wasted one key-format resolution and one RocksDB read per MPU, most of which would have been eliminated by the age/prefix check.
   
   Additionally, "orphan" MPUs entries present in `multipartInfoTable` butwhose `openKeyTable` entry was already removed (e.g. by the old `OpenKeyCleanupService` before HDDS-9017) were logged as a WARN and skipped entirely, causing them to accumulate in the table forever even when a lifecycle rule plainly covered them.
   
   ## What is the link to the Apache JIRA
   
   HDDS-16089
   
   ## How was this patch tested?
   
   Two new integration tests created by claude is added to `TestKeyLifecycleService.Normal`
   
   | Test | What it verifies |
   |---|---|
   | `testOrphanMpuAbortedByAgeAndPrefixRule` | Orphan MPU is aborted alongside a normal MPU when a prefix-only rule matches |
   | `testOrphanMpuNotAbortedByTagOnlyRule` | Orphan MPU is left untouched when the only applicable rule requires a tag filter |


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [PR] HDDS-16089. Improve MPU table evaluation efficiency in KeyLifeCycleService [ozone]

Posted by "priyeshkaratha (via GitHub)" <gi...@apache.org>.
priyeshkaratha commented on code in PR #10949:
URL: https://github.com/apache/ozone/pull/10949#discussion_r3755368163


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyLifecycleService.java:
##########
@@ -1109,55 +1109,103 @@ private void processMultipartUploads(OmBucketInfo bucketInfo, List<OmLCRule> rul
           upload.setCreationTime(Instant.ofEpochMilli(mpuKeyInfo.getCreationTime()));
           String keyName = upload.getKeyName();
 
-          String multipartOpenKey;
-          try {
-            multipartOpenKey = OMMultipartUploadUtils.getMultipartOpenKey(
-                volumeName, bucketName, keyName, upload.getUploadId(),
-                omMetadataManager, bucketInfo.getBucketLayout());
-          } catch (OMException e) {
-            LOG.warn("Failed to get multipart open key for {}/{}/{}, skipping",
-                volumeName, bucketName, keyName, e);
-            continue;
+          OmLCRule matchedRuleWithoutTags = null;
+          boolean needsTagMatch = false;
+          for (OmLCRule rule : ruleList) {
+            if (!passesAgeAndPrefix(upload, keyName, rule)) {
+              continue;
+            }
+            if (!rule.isTagEnable()) {
+              matchedRuleWithoutTags = rule;
+              break;
+            }
+            needsTagMatch = true;
           }
 
-          OmKeyInfo openKeyInfo = omMetadataManager.getOpenKeyTable(bucketInfo.getBucketLayout())
-              .get(multipartOpenKey);
-          if (openKeyInfo == null) {
-            LOG.warn("Open key not found for multipart upload {}/{}/{}, skipping",
-                volumeName, bucketName, keyName);
+          if (matchedRuleWithoutTags == null && !needsTagMatch) {
             continue;
           }
 
-          for (OmLCRule rule : ruleList) {
-            if (shouldAbortUpload(openKeyInfo, upload, keyName, rule)) {
-              if (expiredUploads.isFull()) {
-                LOG.info("Multipart upload batch reached part count limit {}, aborting current batch " +
-                    "({} uploads, {} parts) for bucket {}/{}",
-                    mpuAbortLimitPerTask, expiredUploads.size(), expiredUploads.getPartCount(),
-                    volumeName, bucketName);
-                abortExpiredMultipartUploadsAndClear(bucketInfo, expiredUploads);
-              }
+          OmLCRule matchingRule = matchedRuleWithoutTags;
 
-              // Split-schema MPUs keep parts in multipartPartsTable (the embedded map
-              // is empty); legacy MPUs use the embedded map. An MPU with no uploaded
-              // parts is valid (S3 allows aborting it with an empty parts list).
-              int uploadedParts;
-              try {
-                uploadedParts = mpuKeyInfo.getSchemaVersion()
-                    == OmMultipartKeyInfo.SPLIT_PARTS_TABLE_SCHEMA_VERSION
-                    ? OMMultipartUploadUtils.countParts(omMetadataManager, upload.getUploadId())
-                    : mpuKeyInfo.getPartKeyInfoMap().size();
-              } catch (IOException e) {
-                LOG.warn("Failed to count parts for MPU {}/{}/{} uploadId {}, skipping",
-                    volumeName, bucketName, keyName, upload.getUploadId(), e);
-                break;
+          if (matchingRule == null) {
+            String multipartOpenKey;
+            try {
+              multipartOpenKey = OMMultipartUploadUtils.getMultipartOpenKey(
+                  volumeName, bucketName, keyName, upload.getUploadId(),
+                  omMetadataManager, bucketInfo.getBucketLayout());
+            } catch (OMException e) {
+              LOG.warn("Failed to get multipart open key for {}/{}/{}, skipping",
+                  volumeName, bucketName, keyName, e);
+              continue;
+            }
+
+            OmKeyInfo openKeyInfo;
+            try {
+              openKeyInfo = omMetadataManager.getOpenKeyTable(bucketInfo.getBucketLayout())
+                  .get(multipartOpenKey);
+            } catch (IOException e) {
+              LOG.warn("Failed to read open key table for {}/{}/{}, skipping",
+                  volumeName, bucketName, keyName, e);
+              continue;
+            }
+
+            if (openKeyInfo == null) {
+              for (OmLCRule rule : ruleList) {

Review Comment:
   Good catch. Updated the PR



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [PR] HDDS-16089. Improve MPU table evaluation efficiency in KeyLifeCycleService [ozone]

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #10949:
URL: https://github.com/apache/ozone/pull/10949#discussion_r3764340117


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyLifecycleService.java:
##########
@@ -1109,55 +1109,93 @@ private void processMultipartUploads(OmBucketInfo bucketInfo, List<OmLCRule> rul
           upload.setCreationTime(Instant.ofEpochMilli(mpuKeyInfo.getCreationTime()));
           String keyName = upload.getKeyName();
 
-          String multipartOpenKey;
-          try {
-            multipartOpenKey = OMMultipartUploadUtils.getMultipartOpenKey(
-                volumeName, bucketName, keyName, upload.getUploadId(),
-                omMetadataManager, bucketInfo.getBucketLayout());
-          } catch (OMException e) {
-            LOG.warn("Failed to get multipart open key for {}/{}/{}, skipping",
-                volumeName, bucketName, keyName, e);
-            continue;
+          OmLCRule matchedRuleWithoutTags = null;
+          boolean needsTagMatch = false;
+          for (OmLCRule rule : ruleList) {
+            if (!passesAgeAndPrefix(upload, keyName, rule)) {
+              continue;
+            }
+            if (!rule.isTagEnable()) {

Review Comment:
   This reorder the rule evaluation order.  It prefer a rule without tag over a rule with tag first. 



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [PR] HDDS-16089. Improve MPU table evaluation efficiency in KeyLifeCycleService [ozone]

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi merged PR #10949:
URL: https://github.com/apache/ozone/pull/10949


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [PR] HDDS-16089. Improve MPU table evaluation efficiency in KeyLifeCycleService [ozone]

Posted by "rich7420 (via GitHub)" <gi...@apache.org>.
rich7420 commented on code in PR #10949:
URL: https://github.com/apache/ozone/pull/10949#discussion_r3740207155


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyLifecycleService.java:
##########
@@ -1109,55 +1109,103 @@ private void processMultipartUploads(OmBucketInfo bucketInfo, List<OmLCRule> rul
           upload.setCreationTime(Instant.ofEpochMilli(mpuKeyInfo.getCreationTime()));
           String keyName = upload.getKeyName();
 
-          String multipartOpenKey;
-          try {
-            multipartOpenKey = OMMultipartUploadUtils.getMultipartOpenKey(
-                volumeName, bucketName, keyName, upload.getUploadId(),
-                omMetadataManager, bucketInfo.getBucketLayout());
-          } catch (OMException e) {
-            LOG.warn("Failed to get multipart open key for {}/{}/{}, skipping",
-                volumeName, bucketName, keyName, e);
-            continue;
+          OmLCRule matchedRuleWithoutTags = null;
+          boolean needsTagMatch = false;
+          for (OmLCRule rule : ruleList) {
+            if (!passesAgeAndPrefix(upload, keyName, rule)) {
+              continue;
+            }
+            if (!rule.isTagEnable()) {
+              matchedRuleWithoutTags = rule;
+              break;
+            }
+            needsTagMatch = true;
           }
 
-          OmKeyInfo openKeyInfo = omMetadataManager.getOpenKeyTable(bucketInfo.getBucketLayout())
-              .get(multipartOpenKey);
-          if (openKeyInfo == null) {
-            LOG.warn("Open key not found for multipart upload {}/{}/{}, skipping",
-                volumeName, bucketName, keyName);
+          if (matchedRuleWithoutTags == null && !needsTagMatch) {
             continue;
           }
 
-          for (OmLCRule rule : ruleList) {
-            if (shouldAbortUpload(openKeyInfo, upload, keyName, rule)) {
-              if (expiredUploads.isFull()) {
-                LOG.info("Multipart upload batch reached part count limit {}, aborting current batch " +
-                    "({} uploads, {} parts) for bucket {}/{}",
-                    mpuAbortLimitPerTask, expiredUploads.size(), expiredUploads.getPartCount(),
-                    volumeName, bucketName);
-                abortExpiredMultipartUploadsAndClear(bucketInfo, expiredUploads);
-              }
+          OmLCRule matchingRule = matchedRuleWithoutTags;
 
-              // Split-schema MPUs keep parts in multipartPartsTable (the embedded map
-              // is empty); legacy MPUs use the embedded map. An MPU with no uploaded
-              // parts is valid (S3 allows aborting it with an empty parts list).
-              int uploadedParts;
-              try {
-                uploadedParts = mpuKeyInfo.getSchemaVersion()
-                    == OmMultipartKeyInfo.SPLIT_PARTS_TABLE_SCHEMA_VERSION
-                    ? OMMultipartUploadUtils.countParts(omMetadataManager, upload.getUploadId())
-                    : mpuKeyInfo.getPartKeyInfoMap().size();
-              } catch (IOException e) {
-                LOG.warn("Failed to count parts for MPU {}/{}/{} uploadId {}, skipping",
-                    volumeName, bucketName, keyName, upload.getUploadId(), e);
-                break;
+          if (matchingRule == null) {
+            String multipartOpenKey;
+            try {
+              multipartOpenKey = OMMultipartUploadUtils.getMultipartOpenKey(
+                  volumeName, bucketName, keyName, upload.getUploadId(),
+                  omMetadataManager, bucketInfo.getBucketLayout());
+            } catch (OMException e) {
+              LOG.warn("Failed to get multipart open key for {}/{}/{}, skipping",
+                  volumeName, bucketName, keyName, e);
+              continue;
+            }
+
+            OmKeyInfo openKeyInfo;
+            try {
+              openKeyInfo = omMetadataManager.getOpenKeyTable(bucketInfo.getBucketLayout())
+                  .get(multipartOpenKey);
+            } catch (IOException e) {
+              LOG.warn("Failed to read open key table for {}/{}/{}, skipping",
+                  volumeName, bucketName, keyName, e);
+              continue;
+            }
+
+            if (openKeyInfo == null) {
+              for (OmLCRule rule : ruleList) {

Review Comment:
   This re-scan looks unreachable. We only reach here with `matchingRule == null`, which means the first loop above already found no tag-free rule passing age+prefix (otherwise `matchedRuleWithoutTags` is set and this whole read block is skipped). This loop tests the same `!rule.isTagEnable() && passesAgeAndPrefix(...)` condition, so `matchingRule` stays null and we always fall through to the skip at the `if (matchingRule == null)` below — the `scheduling abort` log a few lines down is never hit.
   
   The orphan tag-free case is actually handled by that first loop (it sets `matchedRuleWithoutTags` and skips the open-key read entirely, which is why `testOrphanMpuAbortedByAgeAndPrefixRule` passes). Could we drop this loop (and the unreachable success log) and just keep the skip + `continue` for the orphan-with-only-tag-rules case?



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [PR] HDDS-16089. Improve MPU table evaluation efficiency in KeyLifeCycleService [ozone]

Posted by "priyeshkaratha (via GitHub)" <gi...@apache.org>.
priyeshkaratha commented on code in PR #10949:
URL: https://github.com/apache/ozone/pull/10949#discussion_r3772279072


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyLifecycleService.java:
##########
@@ -1109,55 +1109,93 @@ private void processMultipartUploads(OmBucketInfo bucketInfo, List<OmLCRule> rul
           upload.setCreationTime(Instant.ofEpochMilli(mpuKeyInfo.getCreationTime()));
           String keyName = upload.getKeyName();
 
-          String multipartOpenKey;
-          try {
-            multipartOpenKey = OMMultipartUploadUtils.getMultipartOpenKey(
-                volumeName, bucketName, keyName, upload.getUploadId(),
-                omMetadataManager, bucketInfo.getBucketLayout());
-          } catch (OMException e) {
-            LOG.warn("Failed to get multipart open key for {}/{}/{}, skipping",
-                volumeName, bucketName, keyName, e);
-            continue;
+          OmLCRule matchedRuleWithoutTags = null;
+          boolean needsTagMatch = false;
+          for (OmLCRule rule : ruleList) {
+            if (!passesAgeAndPrefix(upload, keyName, rule)) {
+              continue;
+            }
+            if (!rule.isTagEnable()) {

Review Comment:
   Done



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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