LakshSingla commented on code in PR #13974:
URL: https://github.com/apache/druid/pull/13974#discussion_r1147820956
##########
processing/src/main/java/org/apache/druid/frame/channel/ComposingWritableFrameChannel.java:
##########
@@ -118,14 +123,17 @@ public void close() throws IOException
{
if (currentIndex < writableChannelSuppliers.size()) {
writableChannelSuppliers.get(currentIndex).get().close();
+ convertChannelSuppliersToReadOnly(currentIndex);
currentIndex = writableChannelSuppliers.size();
}
}
@Override
public boolean isClosed()
{
- return currentIndex == writableChannelSuppliers.size();
+ return currentIndex == writableChannelSuppliers.size() ||
writableChannelSuppliers.get(currentIndex)
Review Comment:
While discussing with @cryptoe, we had the same assumption, however on
revisiting the implementation of close(), we are manually setting the index to
`writableChannelSuppliers.size()` on calling it, so this extra check isn't
reached / needed.
##########
processing/src/main/java/org/apache/druid/frame/channel/ComposingWritableFrameChannel.java:
##########
@@ -105,6 +100,16 @@ public void write(FrameWithPartition frameWithPartition)
throws IOException
}
}
+ private void convertChannelSuppliersToReadOnly(int index)
Review Comment:
Regarding the common interface, I assume that there can be a lot more common
code paths. OutputChannel is essentially a PartitionedOutputChannel where the
partition is fixed. This would require refactoring of ReadableFrameChannel as
well.
We can do without storing the readOnly() version in the
`ComposingOutputChannelFactory`, though I think we can leave it as is, since
its immutable and conveys the intent that we on fetching the readable channel,
we don't want other references to be stored (if they are). WDYT?
--
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]
