HDDS-15949. Handle split MPU part counting for abort batch sizing and container-key-mapping CLI - #10849
Conversation
… container-key-mapping CLI
rakeshadr
left a comment
There was a problem hiding this comment.
Thanks @spacemonkd for handling this case. Added a few comments, pls check it.
sarvekshayr
left a comment
There was a problem hiding this comment.
Thanks @spacemonkd for working on this.
| String uploadId = multipartInfoDbKey.substring( | ||
| multipartInfoDbKey.lastIndexOf(OM_KEY_PREFIX) + OM_KEY_PREFIX.length()); |
There was a problem hiding this comment.
Use the below approach instead of manual substring parsing -
| String uploadId = multipartInfoDbKey.substring( | |
| multipartInfoDbKey.lastIndexOf(OM_KEY_PREFIX) + OM_KEY_PREFIX.length()); | |
| String uploadId; | |
| try { | |
| uploadId = OmMultipartUpload.from(multipartInfoDbKey).getUploadId(); | |
| } catch (IllegalArgumentException e) { | |
| err().println("Invalid multipartInfoTable key " + multipartInfoDbKey + ", " + e); | |
| return matchedContainers; | |
| } |
rakeshadr
left a comment
There was a problem hiding this comment.
Added a few minor comments. Please look at other comments and resolve once its fixed. Thanks!
|
|
||
| @Test | ||
| public void testContainerToKeyMappingWithSplitSchemaMPU() { | ||
| int exitCode = execute("--containers", String.valueOf(CONTAINER_ID_4), "--in-progress"); |
There was a problem hiding this comment.
Not a bug, but using a distinct CONTAINER_ID_5 for the new tests would make the split-schema test self-contained and easier to reason about in future.
CONTAINER_ID_4 for both legacy and split MPU testContainerToKeyMappingWithSplitSchemaMPU
| // per-MPU skip — bucket loop continues | ||
| } | ||
| } | ||
| assertEquals(2, list.size()); |
There was a problem hiding this comment.
Can you please add comments to help maintainers.
// A (2 parts) and C (1 part) are added; B is skipped due to IOException
assertEquals(2, list.size()); // uploadA and uploadC only
assertEquals(3, list.getPartCount()); // 2 (uploadA) + 1 (uploadC)
rakeshadr
left a comment
There was a problem hiding this comment.
+1LGTM, thanks @spacemonkd for taking care this case.
|
Thanks @spacemonkd for the patch and @rakeshadr for the review. |
|
Thanks @sarvekshayr for the reviews and merging this PR and @rakeshadr for the reviews. |
What changes were proposed in this pull request?
HDDS-15949. Handle split MPU part counting for abort batch sizing and container-key-mapping CLI
Please describe your PR in detail:
Schema-aware MPU part counting for abort batch sizing (
OmMetadataManagerImpl.getExpiredMultipartUploads(),KeyLifecycleService)For split-schema MPUs (schemaVersion == 1), part count now comes from multipartPartsTable via OMMultipartUploadUtils.countParts() instead of getPartKeyInfoMap().size() (which is always 0 for split MPUs).
Without fix the expired MPU cleanup and lifecycle AbortIncompleteMultipartUpload treated every split MPU as having 0–1 parts when sizing abort batches. Batches could exceed the intended part limit (maxParts / mpuAbortLimitPerTask), causing larger-than-planned abort work per iteration. The abort logic itself was already schema-aware; only batch sizing was wrong.
Shared countParts() helper (OMMultipartUploadUtils)
Added countParts(omMetadataManager, uploadId) wrapping the existing cache-aware scanParts() logic.
Without fix it is the same as above, plus duplicated/incorrect counting if each call site reimplemented split-table reads differently from abort/complete paths.
Schema-aware split MPU handling in debug CLI (ContainerToKeyMapping)
processMultipartUploadbranches on schema version:Without fix the
ozone debug om container-key-mapping --in-progressreported no open MPU keys for split-schema uploads, even when their parts used the queried containers, this causes misleading output for debugging container to key mappings on upgraded clusters.What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-15949
How was this patch tested?
Patch was tested via unit tests