rohangarg commented on code in PR #13974:
URL: https://github.com/apache/druid/pull/13974#discussion_r1147781573
##########
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:
could create a common interface for `PartitionedOutputChannel` and
`OutputChannel` which encapsulates `convertToReadOnly` - that way we can only
give a single instance of the common interface to this object.
Also, is there a need to have `convertToReadOnly` in the
`ComposingOutputChannelFactory` now that we close the channels as soon as the
writes for them are done? cc @LakshSingla
These things could be followups.
##########
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:
Can you please explain the reason for adding the extra check?
--
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]
