clintropolis commented on code in PR #18005:
URL: https://github.com/apache/druid/pull/18005#discussion_r2093630140
##########
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:
it is not important at all that this is using `WrappedSegment` nothing bad
happens if i just make this fully implement `Segment`. At first in this branch
I was just fully implementing `Segment` before i realized that we only need to
override a couple of things of `WrappedSegment` and changed to do that instead.
I can change it back :shrug:
--
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]
