Copilot commented on code in PR #11685:
URL: https://github.com/apache/ozone/pull/11685#discussion_r4485759381
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/ReconGlobalMetricsService.java:
##########
@@ -102,6 +102,18 @@ public Map<String, Long> getOpenKeySummary() {
return keysSummary;
}
+ /**
+ * Returns {@link #getOpenKeySummary()} with the committed part sizes of
incomplete multipart uploads added.
+ * The count is unchanged because each incomplete upload is already counted
once through its multipart open key.
+ */
+ public Map<String, Long> getOpenKeySummaryWithMPU() {
+ Map<String, Long> keysSummary = getOpenKeySummary();
+ Map<String, Long> mpuSummary = getMPUKeySummary();
+ keysSummary.merge("totalReplicatedDataSize",
mpuSummary.get("totalReplicatedDataSize"), Long::sum);
+ keysSummary.merge("totalUnreplicatedDataSize",
mpuSummary.get("totalDataSize"), Long::sum);
Review Comment:
`mpuSummary.get("totalReplicatedDataSize")` /
`mpuSummary.get("totalDataSize")` can return null (or the key name may differ),
which will cause `Map.merge(...)` to throw a NPE. Consider using
`getOrDefault(..., 0L)` and/or aligning the key names so the “unreplicated”
field is fetched from a consistently named entry (to avoid silently breaking if
`getMPUKeySummary()` changes keys).
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/OMDBInsightEndpoint.java:
##########
@@ -1167,14 +1171,38 @@ private KeyEntityInfo
createKeyEntityInfoFromOmKeyInfo(String dbKey,
keyEntityInfo.setIsKey(keyInfo.isFile());
String fullKeyPath = ReconUtils.constructFullPath(keyInfo,
reconNamespaceSummaryManager);
keyEntityInfo.setPath(fullKeyPath.isEmpty() ? keyInfo.getKeyName() :
fullKeyPath);
- keyEntityInfo.setSize(keyInfo.getDataSize());
+ if (OMMultipartUploadUtils.isMultipartKeySet(keyInfo)) {
+ Pair<Long, Long> uploadSize = getMultipartUploadSize(dbKey, keyInfo);
+ keyEntityInfo.setSize(uploadSize.getLeft());
+ keyEntityInfo.setReplicatedSize(uploadSize.getRight());
+ } else {
+ keyEntityInfo.setSize(keyInfo.getDataSize());
+ keyEntityInfo.setReplicatedSize(keyInfo.getReplicatedSize());
+ }
keyEntityInfo.setCreationTime(keyInfo.getCreationTime());
Review Comment:
`getMultipartUploadSize(...)` performs extra DB reads (and for split-schema,
scans parts) per multipart row. On pages containing many multipart open keys,
this becomes an N+1 pattern and can significantly increase endpoint latency.
Consider caching per-request (e.g., `Map<uploadId, Pair<sizes>>`) and/or
prefetching sizes for the page’s upload IDs so each upload’s committed-parts
size is computed at most once per request.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/MultipartInfoInsightHandler.java:
##########
@@ -332,6 +337,26 @@ private void applyPartSize(OmMultipartPartKey partKey,
OmMultipartPartInfo partI
"replicated");
}
+ /**
+ * Returns the (unreplicated, replicated) size of the committed parts of a
single MPU, read from the embedded
+ * parts for legacy-schema MPUs and from {@code multipartPartsTable} for
split-schema MPUs.
+ */
+ public static Pair<Long, Long> getCommittedPartsSize(OmMultipartKeyInfo
multipartKeyInfo,
+ OMMetadataManager omMetadataManager) throws IOException {
+ if (isLegacySchema(multipartKeyInfo)) {
+ final MutablePair<Long, Long> sizes = MutablePair.of(0L, 0L);
+ forEachLegacyPart(multipartKeyInfo, (dataSize, replicatedSize) -> {
+ sizes.setLeft(sizes.getLeft() + dataSize);
+ sizes.setRight(sizes.getRight() + replicatedSize);
+ });
+ return sizes;
+ }
+ SortedMap<Integer, OmMultipartPartInfo> parts =
+ OMMultipartUploadUtils.scanParts(omMetadataManager,
multipartKeyInfo.getUploadID());
+ long dataSize =
parts.values().stream().mapToLong(OmMultipartPartInfo::getDataSize).sum();
+ return Pair.of(dataSize, OMMultipartUploadUtils.getReplicatedSize(parts,
multipartKeyInfo.getReplicationConfig()));
Review Comment:
This computes unreplicated size with a stream pass over `parts.values()` and
then likely iterates parts again inside
`OMMultipartUploadUtils.getReplicatedSize(...)`. For MPUs with many parts,
consider accumulating both unreplicated and replicated sizes in a single
iteration to avoid a second traversal.
--
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]
