cecemei commented on code in PR #18005:
URL: https://github.com/apache/druid/pull/18005#discussion_r2093502328
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/dart/worker/DartDataSegmentProvider.java:
##########
@@ -83,25 +82,23 @@ public Supplier<ResourceHolder<CompleteSegment>>
fetchSegment(
throw segmentNotFound(segmentId);
}
- final ReferenceCountingSegment segment = chunk.getObject();
- final Optional<Closeable> closeable = segment.acquireReferences();
- if (!closeable.isPresent()) {
+ final ReferenceCountedSegmentProvider segmentReference =
chunk.getObject();
+ final Optional<Segment> maybeSegment =
segmentReference.acquireReference();
+ if (!maybeSegment.isPresent()) {
// Segment has disappeared before we could acquire a reference to it.
throw segmentNotFound(segmentId);
}
+ final Segment segment = maybeSegment.get();
final Closer closer = Closer.create();
- closer.register(closeable.get());
closer.register(() -> {
final PhysicalSegmentInspector inspector =
segment.as(PhysicalSegmentInspector.class);
channelCounters.addFile(inspector != null ? inspector.getNumRows() :
0, 0);
+ // don't release the reference until after we get the rows
+ segment.close();
});
- // It isn't necessary to pass down the SegmentReference to
CompleteSegment, because we've already called
- // segment.acquireReferences() and have attached the releaser to
"closer".
- return new ReferenceCountingResourceHolder<>(new CompleteSegment(
- null,
- Objects.requireNonNull(segment.getBaseSegment())
- ), closer);
+ // we don't need to close CompleteSegment because the checked out
reference is registered with the closer
+ return new ReferenceCountingResourceHolder<>(new CompleteSegment(null,
segment), closer);
Review Comment:
Is the segment here a ReferenceClosingSegment though?
--
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]
