FrankChen021 commented on code in PR #19682:
URL: https://github.com/apache/druid/pull/19682#discussion_r3578873826
##########
server/src/main/java/org/apache/druid/segment/loading/PartialSegmentMetadataCacheEntry.java:
##########
@@ -620,6 +620,31 @@ public long getSize()
}
}
+ /**
+ * The rule-declared on-disk footprint for this segment: this metadata
entry's own reservation (V10 header plus
+ * post-mount adjustment) plus the sum of every rule-held bundle's
reservation. Deliberately excludes bundles that
+ * happen to be linked for other reasons (e.g. load on demand). Used by the
historical to stamp accurate
+ * {@code loadedBytes} on partial-load announcements, so inventory-view
accounting reflects what the rule was
+ * configured to pin rather than incidental cache residency.
+ */
+ public long getRealizedBytes()
+ {
+ // Read sizes straight from ruleBundleHolds under entryLock: each hold
owns a final reference to its bundle entry
+ // and PartialSegmentBundleCacheEntry.getSize() reads a final long, no
lock nesting, no null case, no reliance
+ // on external segment-lock ordering.
+ entryLock.lock();
+ try {
+ long total = currentSize;
+ for (StorageLocation.ReservationHold<PartialSegmentBundleCacheEntry>
hold : ruleBundleHolds.values()) {
Review Comment:
[P1] Count dependency bundles in loadedBytes
`ruleBundleHolds` contains only explicitly selected bundle names, while
projection bundles indirectly pin dependencies such as `__base`. Since a
bundle's `getSize()` excludes dependency sizes, `getRealizedBytes()`
underreports the actual pinned footprint and coordinator capacity accounting
overestimates available space. Sum the unique dependency closure as well and
test metadata plus selected-bundle plus dependency bytes.
##########
server/src/main/java/org/apache/druid/server/SegmentManager.java:
##########
@@ -298,19 +306,27 @@ public void loadSegmentOnBootstrap(
*
* @param dataSegment segment to load
*
+ * @return the input {@code dataSegment}, or a
+ * {@link org.apache.druid.client.DataSegmentAndLoadProfile}
wrapping it when the historical actually
+ * materialized a partial-load footprint. Callers pass the returned
value to the announcement layer so
+ * partial-load announcements carry accurate {@code loadedBytes}.
* @throws SegmentLoadingException if the segment cannot be loaded
* @throws IOException if the segment info cannot be cached on disk
*/
- public void loadSegment(final DataSegment dataSegment) throws
SegmentLoadingException, IOException
+ public DataSegment loadSegment(final DataSegment dataSegment) throws
SegmentLoadingException, IOException
Review Comment:
[P1] Update mocks for the non-void load result
Changing `loadSegment()` to return `DataSegment` leaves
`SegmentLoadDropHandlerTest` using `Mockito.doNothing()` in two setups. Mockito
rejects `doNothing()` on non-void methods, so those tests fail during setup.
Stub the method to return its input segment, including after the chained
first-call exception.
--
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]
