ijuma commented on code in PR #12228:
URL: https://github.com/apache/kafka/pull/12228#discussion_r944876379
##########
clients/src/main/java/org/apache/kafka/common/network/PlaintextChannelBuilder.java:
##########
@@ -54,12 +54,18 @@ public void configure(Map<String, ?> configs) throws
KafkaException {
@Override
public KafkaChannel buildChannel(String id, SelectionKey key, int
maxReceiveSize,
MemoryPool memoryPool,
ChannelMetadataRegistry metadataRegistry) throws KafkaException {
+ PlaintextTransportLayer transportLayer = null;
try {
- PlaintextTransportLayer transportLayer = buildTransportLayer(key);
- Supplier<Authenticator> authenticatorCreator = () -> new
PlaintextAuthenticator(configs, transportLayer, listenerName);
+ transportLayer = buildTransportLayer(key);
+ final PlaintextTransportLayer finalTransportLayer = transportLayer;
+ Supplier<Authenticator> authenticatorCreator = () -> new
PlaintextAuthenticator(configs, finalTransportLayer, listenerName);
return buildChannel(id, transportLayer, authenticatorCreator,
maxReceiveSize,
memoryPool != null ? memoryPool : MemoryPool.NONE,
metadataRegistry);
} catch (Exception e) {
+ // Ideally these resources are closed by the KafkaChannel but this
builder should close the resources instead
+ // if an error occurs due to which KafkaChannel is not created.
+ Utils.closeQuietly(transportLayer, "transport layer for channel
Id: " + id);
+ Utils.closeQuietly(metadataRegistry, "metadataRegistry");
Review Comment:
I looked into this a bit more and it's a bit odd how we're doing clean-up.
`buildAndAttach` currently closes the socket channel and cancels the key. It
also creates the selector channel metadata registry. Shouldn't that take care
of closing the metadata registry since it actually creates it?
With regards to the transport layer, I wonder if we could move the cleanup
logic to `KafkaChannel` instead. We could change `KafkaChannel` to take a
`TransportLayer` supplier instead (or add a static factory method) and then
have it handle the cleanup (instead of doing it in every implementation of
channel builder.
--
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]
