Skip to content

[fix][client] Client interceptors close method not called after producer/consumer/reader close#23830

Open
Shawyeok wants to merge 1 commit intoapache:masterfrom
Shawyeok:client-interceptors-close-method-not-called
Open

[fix][client] Client interceptors close method not called after producer/consumer/reader close#23830
Shawyeok wants to merge 1 commit intoapache:masterfrom
Shawyeok:client-interceptors-close-method-not-called

Conversation

@Shawyeok
Copy link
Contributor

@Shawyeok Shawyeok commented Jan 9, 2025

Motivation

The close methods in client interceptors (ProducerInterceptor#close, ConsumerInterceptor#close, ReaderInterceptor#close) allow interceptor developers to release resources allocated in the interceptor when a producer, consumer, or reader is closed.

However, these close methods were not invoked in the implementation of PIP-23.

Modifications

  • Ensure the close methods of interceptors are called during the final stage of the closing process.
  • Add unit tests to verify this behavior.

Verifying this Change

  • Verified that the change passes all CI checks.

This change includes the following tests:

  • Integration tests for ProducerInterceptor with non-partitioned and partitioned topics.
  • Integration tests for ConsumerInterceptor with non-partitioned and partitioned topics.
  • Integration tests for ConsumerInterceptor with ZeroQueueConsumerImpl
  • Integration tests for ReaderInterceptor with non-partitioned and partitioned topics.

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: Shawyeok#23

…cer/consumer/reader close

Call interceptors close method before close methods return.
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jan 9, 2025
@Shawyeok
Copy link
Contributor Author

Shawyeok commented Jan 9, 2025

@codelipenghui Please feel free take a look, thanks.


ArrayList<CompletableFuture<Void>> closeFutures = new ArrayList<>(4);
closeFutures.add(closeFuture);
closeFutures.add(closeFuture.whenCompleteAsync((nil, ex) -> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Shawyeok @lhotari
In the case where the State is Closing or Closed, should interceptors.close() also be executed?

It looks like the code for closeAsync() has been updated after #23824, so the whenCompleteAsync here should be set on the compositeCloseFuture?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-not-needed Your PR changes do not impact docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants