You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "spacemonkd (via GitHub)" <gi...@apache.org> on 2026/07/23 09:42:36 UTC
[PR] HDDS-15949. Handle split MPU part counting for abort batch sizing and container-key-mapping CLI [ozone]
spacemonkd opened a new pull request, #10849:
URL: https://github.com/apache/ozone/pull/10849
## 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)
`processMultipartUpload` branches on schema version:
- legacy MPUs still use embedded PartKeyInfo
- split MPUs scan multipartPartsTable by uploadId and map block containers from each part’s locations.
Without fix the `ozone debug om container-key-mapping --in-progress` reported 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
--
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-15949. Handle split MPU part counting for abort batch sizing and container-key-mapping CLI [ozone]
Posted by "rakeshadr (via GitHub)" <gi...@apache.org>.
rakeshadr commented on code in PR #10849:
URL: https://github.com/apache/ozone/pull/10849#discussion_r3637546895
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyLifecycleService.java:
##########
@@ -1138,8 +1138,15 @@ private void processMultipartUploads(OmBucketInfo bucketInfo, List<OmLCRule> rul
abortExpiredMultipartUploadsAndClear(bucketInfo, expiredUploads);
}
- // Get part count for this MPU (at least 1 even if no parts uploaded yet)
- int partCount = Math.max(1, mpuKeyInfo.getPartKeyInfoMap().size());
+ // Get part count for this MPU (at least 1 even if no parts uploaded yet).
+ // Split-schema MPUs keep parts in the separate multipartPartsTable
+ // (the embedded map is empty), so count those rows; legacy MPUs use
+ // the embedded map.
+ int uploadedParts = mpuKeyInfo.getSchemaVersion()
+ == OmMultipartKeyInfo.SPLIT_PARTS_TABLE_SCHEMA_VERSION
+ ? OMMultipartUploadUtils.countParts(omMetadataManager, upload.getUploadId())
Review Comment:
Error for ONE bad upload's part prefix propagates IOException out of the for-loop and break. Can you wrap the `#countParts()` call in its own `try/catch`. This would allow loop to continue for next MPU.
```
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; // break out of the ruleList loop, continue outer while
}
```
##########
hadoop-ozone/cli-debug/src/main/java/org/apache/hadoop/ozone/debug/om/ContainerToKeyMapping.java:
##########
@@ -313,11 +318,18 @@ private void processMultipartUpload(Set<Long> containerIds, Map<Long, List<Strin
String dbKey = entry.getKey();
OmMultipartKeyInfo mpuInfo = entry.getValue();
- // Collect all target containers that have parts of this MPU
+ // Collect all target containers that have parts of this MPU. Legacy
+ // (schemaVersion 0) MPUs embed their parts in the multipartInfoTable
+ // value, whereas split (schemaVersion 1) MPUs store each part as a
+ // separate row in the multipartPartsTable.
Set<Long> matchedContainers = new HashSet<>();
- for (PartKeyInfo partKeyInfo : mpuInfo.getPartKeyInfoMap()) {
- OmKeyInfo partKey = OmKeyInfo.getFromProtobuf(partKeyInfo.getPartKeyInfo());
- matchedContainers.addAll(getKeyContainers(partKey, containerIds));
+ if (mpuInfo.getSchemaVersion() == OmMultipartKeyInfo.SPLIT_PARTS_TABLE_SCHEMA_VERSION) {
+ matchedContainers.addAll(getSplitPartContainers(dbKey, containerIds));
Review Comment:
Please add test for the new code path. Thanks!
TestOmMetadataManager
TestContainerToKeyMapping
TestKeyLifecycleService
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyLifecycleService.java:
##########
@@ -1138,8 +1138,15 @@ private void processMultipartUploads(OmBucketInfo bucketInfo, List<OmLCRule> rul
abortExpiredMultipartUploadsAndClear(bucketInfo, expiredUploads);
}
- // Get part count for this MPU (at least 1 even if no parts uploaded yet)
- int partCount = Math.max(1, mpuKeyInfo.getPartKeyInfoMap().size());
+ // Get part count for this MPU (at least 1 even if no parts uploaded yet).
+ // Split-schema MPUs keep parts in the separate multipartPartsTable
+ // (the embedded map is empty), so count those rows; legacy MPUs use
+ // the embedded map.
+ int uploadedParts = mpuKeyInfo.getSchemaVersion()
Review Comment:
can you add test for MPU part count failure case as well.
Note: LLM generated unit test, please use it as reference and validate it before adding to the PR.
```
@Test
public void testPartCountIoExceptionSkipsUploadContinuesBucket() throws Exception {
// Upload A: valid, 2 parts in multipartPartsTable
// Upload B: corrupt — no table entry but we simulate IOException via mock
// Upload C: valid, 1 part
// Expectation: A and C are added to expiredUploads; B is skipped with warn
OMMetadataManager mockMM = Mockito.spy(omMetadataManager);
String uploadA = OMMultipartUploadUtils.getMultipartUploadId();
String uploadB = OMMultipartUploadUtils.getMultipartUploadId();
// Write 2 parts for A
for (int p = 1; p <= 2; p++) {
omMetadataManager.getMultipartPartsTable().put(
OmMultipartPartKey.of(uploadA, p),
new OmMultipartPartInfo.Builder()
.setPartName("a/p" + p).setPartNumber(p)
.setDataSize(100L).setObjectID(p).setUpdateID(p).build());
}
// Simulate IOException when accessing parts for uploadB
Table<OmMultipartPartKey, OmMultipartPartInfo> mockPartsTable =
Mockito.spy(omMetadataManager.getMultipartPartsTable());
Mockito.when(mockMM.getMultipartPartsTable()).thenReturn(mockPartsTable);
Mockito.when(mockPartsTable.iterator(
Mockito.eq(OmMultipartPartKey.prefix(uploadB))))
.thenThrow(new IOException("simulated corruption"));
assertEquals(2, OMMultipartUploadUtils.countParts(mockMM, uploadA));
// countParts for B must throw, not propagate bucket-level
assertThrows(IOException.class,
() -> OMMultipartUploadUtils.countParts(mockMM, uploadB));
// The fix: bucket processing does NOT stop when B throws
KeyLifecycleService.PartCountLimitedList list =
new KeyLifecycleService.PartCountLimitedList(10);
for (String uid : List.of(uploadA, uploadB)) {
try {
int c = OMMultipartUploadUtils.countParts(mockMM, uid);
list.add(new OmMultipartUpload("v", "b", "k", uid), Math.max(1, c));
} catch (IOException e) {
// per-MPU skip — bucket loop continues
}
}
assertEquals(1, list.size()); // only A; B was skipped
assertEquals(2, list.getPartCount());
}
```
--
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-15949. Handle split MPU part counting for abort batch sizing and container-key-mapping CLI [ozone]
Posted by "spacemonkd (via GitHub)" <gi...@apache.org>.
spacemonkd commented on PR #10849:
URL: https://github.com/apache/ozone/pull/10849#issuecomment-5100108862
Thanks @sarvekshayr for the reviews and merging this PR and @rakeshadr for the reviews.
--
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-15949. Handle split MPU part counting for abort batch sizing and container-key-mapping CLI [ozone]
Posted by "sarvekshayr (via GitHub)" <gi...@apache.org>.
sarvekshayr commented on code in PR #10849:
URL: https://github.com/apache/ozone/pull/10849#discussion_r3643609528
##########
hadoop-ozone/cli-debug/src/main/java/org/apache/hadoop/ozone/debug/om/ContainerToKeyMapping.java:
##########
@@ -332,8 +344,43 @@ private void processMultipartUpload(Set<Long> containerIds, Map<Long, List<Strin
}
private Set<Long> getKeyContainers(OmKeyInfo keyInfo, Set<Long> targetContainerIds) {
+ return getContainers(keyInfo.getKeyLocationVersions(), targetContainerIds);
+ }
+
+ /**
+ * Scans the split multipartPartsTable for all parts belonging to the given
+ * multipart upload (its uploadId is the last path component of the
+ * multipartInfoTable db key) and returns the target containers referenced by
+ * those parts' block locations.
+ */
+ private Set<Long> getSplitPartContainers(String multipartInfoDbKey, Set<Long> targetContainerIds) {
+ Set<Long> matchedContainers = new HashSet<>();
+ String uploadId = multipartInfoDbKey.substring(
+ multipartInfoDbKey.lastIndexOf(OM_KEY_PREFIX) + OM_KEY_PREFIX.length());
Review Comment:
Use the below approach instead of manual substring parsing -
```suggestion
String uploadId;
try {
uploadId = OmMultipartUpload.from(multipartInfoDbKey).getUploadId();
} catch (IllegalArgumentException e) {
err().println("Invalid multipartInfoTable key " + multipartInfoDbKey + ", " + e);
return matchedContainers;
}
```
--
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-15949. Handle split MPU part counting for abort batch sizing and container-key-mapping CLI [ozone]
Posted by "sarvekshayr (via GitHub)" <gi...@apache.org>.
sarvekshayr commented on PR #10849:
URL: https://github.com/apache/ozone/pull/10849#issuecomment-5100044302
Thanks @spacemonkd for the patch and @rakeshadr for the review.
--
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-15949. Handle split MPU part counting for abort batch sizing and container-key-mapping CLI [ozone]
Posted by "rakeshadr (via GitHub)" <gi...@apache.org>.
rakeshadr commented on code in PR #10849:
URL: https://github.com/apache/ozone/pull/10849#discussion_r3649353606
##########
hadoop-ozone/cli-debug/src/test/java/org/apache/hadoop/ozone/debug/om/TestContainerToKeyMapping.java:
##########
@@ -189,6 +192,18 @@ public void testContainerToKeyMappingWithMPUOnlyFileNames() {
assertThat(output).contains("/vol1/obs-bucket/mpuKey/test-upload-id");
}
+ @Test
+ public void testContainerToKeyMappingWithSplitSchemaMPU() {
+ int exitCode = execute("--containers", String.valueOf(CONTAINER_ID_4), "--in-progress");
Review 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`
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestKeyLifecycleService.java:
##########
@@ -3340,4 +3350,111 @@ private OmMultipartInfo createTestMultipartUpload(String volumeName, String buck
public static String uniqueObjectName(String prefix) {
return prefix + OBJECT_COUNTER.getAndIncrement();
}
+
+ @Test
+ public void testPartCountIoExceptionSkipsUploadContinuesBucket(@TempDir File tempDir)
+ throws Exception {
+ OzoneConfiguration omConf = new OzoneConfiguration();
+ omConf.set(OZONE_OM_DB_DIRS, tempDir.getAbsolutePath());
+ OMMetadataManager omMetadataManager = new OmMetadataManagerImpl(omConf, null);
+ try {
+ String uploadA = OMMultipartUploadUtils.getMultipartUploadId();
+ String uploadB = OMMultipartUploadUtils.getMultipartUploadId();
+ String uploadC = OMMultipartUploadUtils.getMultipartUploadId();
+
+ addSplitSchemaPart(omMetadataManager, uploadA, 1);
+ addSplitSchemaPart(omMetadataManager, uploadA, 2);
+ addSplitSchemaPart(omMetadataManager, uploadC, 1);
+
+ OMMetadataManager mockMM = Mockito.spy(omMetadataManager);
+ Table<OmMultipartPartKey, OmMultipartPartInfo> mockPartsTable =
+ Mockito.spy(omMetadataManager.getMultipartPartsTable());
+ when(mockMM.getMultipartPartsTable()).thenReturn(mockPartsTable);
+ when(mockPartsTable.iterator(eq(OmMultipartPartKey.prefix(uploadB))))
+ .thenThrow(new IOException("simulated corruption"));
+
+ assertEquals(2, OMMultipartUploadUtils.countParts(mockMM, uploadA));
+ assertThrows(IOException.class,
+ () -> OMMultipartUploadUtils.countParts(mockMM, uploadB));
+
+ KeyLifecycleService.PartCountLimitedList list =
+ new KeyLifecycleService.PartCountLimitedList(10);
+ for (String uploadId : Arrays.asList(uploadA, uploadB, uploadC)) {
+ try {
+ int partCount = OMMultipartUploadUtils.countParts(mockMM, uploadId);
+ list.add(new OmMultipartUpload("v", "b", "k", uploadId), partCount);
+ } catch (IOException e) {
+ // per-MPU skip — bucket loop continues
+ }
+ }
+ assertEquals(2, list.size());
Review 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)
```
--
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-15949. Handle split MPU part counting for abort batch sizing and container-key-mapping CLI [ozone]
Posted by "sarvekshayr (via GitHub)" <gi...@apache.org>.
sarvekshayr merged PR #10849:
URL: https://github.com/apache/ozone/pull/10849
--
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