clintropolis commented on code in PR #18005:
URL: https://github.com/apache/druid/pull/18005#discussion_r2105672145


##########
server/src/main/java/org/apache/druid/server/coordination/ServerManager.java:
##########
@@ -258,21 +259,23 @@ protected <T> QueryRunner<T> buildQueryRunnerForSegment(
       return new ReportTimelineMissingSegmentQueryRunner<>(descriptor);
     }
 
-    final ReferenceCountingSegment segment = chunk.getObject();
-    return buildAndDecorateQueryRunner(
+    final ReferenceCountedSegmentProvider referenceCounter = chunk.getObject();
+
+    final Optional<Segment> maybeSegment = 
segmentMapFn.apply(referenceCounter);

Review Comment:
   good catch, I didn't actually mean to change this flow as part of this PR - 
i was doing some experiments in a branch on top of this involving acquiring the 
references up front, but this was a strange intermediary state and not even 
what that branch looks like right now (it pushes in a closer that the segments 
can register with to make it easier to handle the issues you're mentioning), I 
think i got branches mixed up. 
   
   I have switched this to what I meant to do for now, which is push the 
provider and map function into `ReferenceCountingSegmentQueryRunner`, which 
keeps the reference handling and closing inside the runner



-- 
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]

Reply via email to