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


##########
processing/src/main/java/org/apache/druid/segment/ReferenceCountedSegmentProvider.java:
##########
@@ -168,35 +139,63 @@ public short getAtomicUpdateGroupSize()
   }
 
   @Override
-  public Optional<Closeable> acquireReferences()
+  public Optional<Segment> acquireReference()
   {
-    return incrementReferenceAndDecrementOnceCloseable();
+    final Optional<Closeable> reference = 
incrementReferenceAndDecrementOnceCloseable();
+    return reference.map(ReferenceClosingSegment::new);
   }
 
-  @Override
-  public void validateOrElseThrow(PolicyEnforcer policyEnforcer)
+  private boolean includeRootPartitions(ReferenceCountedSegmentProvider other)
   {
-    policyEnforcer.validateOrElseThrow(this, null);
+    return startRootPartitionId <= other.startRootPartitionId && 
endRootPartitionId >= other.endRootPartitionId;
   }
 
-  @Override
-  public <T> T as(Class<T> clazz)
+  /**
+   * Wraps a {@link Segment} and closes reference to this segment, 
decrementing the counter instead of closing the
+   * segment itself
+   */
+  public final class ReferenceClosingSegment extends WrappedSegment
   {
-    if (isClosed()) {
-      return null;
+    private final Closeable referenceCloseable;
+    private boolean isClosed;
+
+    private ReferenceClosingSegment(Closeable referenceCloseable)
+    {
+      super(baseObject);
+      this.referenceCloseable = referenceCloseable;
     }
-    return baseObject.as(clazz);
-  }
 
-  @Override
-  public boolean isTombstone()
-  {
-    return baseObject.isTombstone();
-  }
+    @Nullable
+    @Override
+    public <T> T as(@Nonnull Class<T> clazz)
+    {
+      if (isClosed) {
+        return null;
+      }
+      return baseObject.as(clazz);
+    }
 
-  @Override
-  public String asString()
-  {
-    return baseObject.asString();
+    @Override
+    public void validateOrElseThrow(PolicyEnforcer policyEnforcer)
+    {
+      // a segment cannot directly have any policies, so use the enforcer 
directly
+      policyEnforcer.validateOrElseThrow(this, null);
+    }
+
+    @Override
+    public void close() throws IOException

Review Comment:
   I wonder maybe this class has changed the default behavior from 
WrappedSegment so much that it doesn't need to extend from it, but there might 
be too many compilation failure? Anyway, it's worth noting that the close here 
doesn't close the base segment, and only close the reference.



-- 
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: commits-unsubscr...@druid.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org

Reply via email to