[
https://issues.apache.org/jira/browse/KAFKA-14565?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17675640#comment-17675640
]
Chris Egerton commented on KAFKA-14565:
---------------------------------------
[~beardt] sorry, to be clear, I was suggesting _either_ altering
{{{}getConfiguredInstances{}}}, _or_ altering the code that currently calls
that method (in the various client classes). With the latter option, we could
manually instantiate and configure each interceptor in the calling code instead
of relying on the API provided by the {{AbstractConfig}} class.
IMO the proposed changes to the interceptor interfaces aren't significantly
clearer to developers and are really only convenient as a workaround to issues
with the {{AbstractConfig}} API. And to reiterate a point raised above, they
will require changes to existing interceptor classes in order to provide any
benefit to users, which means that every existing interceptor release out there
would still cause leaked resources in the failure scenario described by this
issue.
I think if we're going to go the route of proposing a KIP to address this issue
(which I don't believe is necessary, but I'm not entirely opposed to), it
should address the root issue in the {{AbstractConfig}} API, which is that when
multiple classes are instantiated and configured at once, there's a potential
for leaked resources. Off the top of my head, there are a few options to
accomplish this:
* Automatically invoke {{close}} on any {{Closeable}} or {{AutoCloseable}}
instances if/when a failure occurs
* Add a new overloaded {{getConfiguredInstance}} / {{getConfiguredInstances}}
variant that allows users to specify whether already-instantiated classes
should be closed or not when a failure occurs
* Add a new exception type to the public API that includes a list of all of
the successfully-instantiated (and/or successfully-configured) instances before
the error was encountered so that callers can choose how to handle the failure
however they want (and possibly so that instantiation/configuration can be
attempted on every class before throwing the exception)
But I'm still not sure we need to go this far in-depth right now; why not just
tweak the internal logic in the clients classes to transparently account for
this issue, with no effort required on the part of users or developers beyond a
simple version bump of their clients library, and no additions to public API?
> Improve Interceptor Resource Leakage Prevention
> -----------------------------------------------
>
> Key: KAFKA-14565
> URL: https://issues.apache.org/jira/browse/KAFKA-14565
> Project: Kafka
> Issue Type: Improvement
> Components: clients
> Reporter: Terry Beard
> Assignee: Terry Beard
> Priority: Major
> Labels: needs-kip
> Fix For: 3.5.0
>
>
> The Consumer and Producer interceptor interfaces and their corresponding
> Kafka Consumer and Producer constructors do not adequately support cleanup of
> underlying interceptor resources.
> Currently within the Kafka Consumer and Kafka Producer constructors, the
> AbstractConfig.getConfiguredInstances() is delegated responsibility for both
> creating and configuring each interceptor listed in the interceptor.classes
> property and returns a configured List<ConsumerInterceptor<K,V>>
> interceptors.
> This dual responsibility for both creation and configuration is problematic
> when it involves multiple interceptors where at least one interceptor's
> configure method implementation creates and/or depends on objects which
> creates threads, connections or other resources which requires clean up and
> the subsequent interceptor's configure method raises a runtime exception.
> This raising of the runtime exception produces a resource leakage in the
> first interceptor as the interceptor container i.e.
> ConsumerInterceptors/ProducerInterceptors is never created and therefore the
> first interceptor's and really any interceptor's close method are never
> called.
> To help ensure the respective container interceptors are able to invoke their
> respective interceptor close methods for proper resource clean up, I propose
> defining a default open method with no implementation and check exception on
> the respective Consumer/Producer interceptor interfaces. This open method
> will be responsible for creating threads and/or objects which utilizes
> threads, connections or other resource which requires clean up.
> Additionally, the default open method enables implementation optionality as
> it's empty default behavior means it will do nothing when unimplemented.
> Additionally, the Kafka Consumer/Producer Interceptor containers will
> implement a corresponding maybeOpen method which throws a checked exception.
> In order to maintain backwards compatibility with earlier developed
> interceptors the maybeOpen will check whether the interceptor's interface
> contains the newer open method before calling it accordingly.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
