Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Java Client] Send CloseConsumer on client timeout #16616

Merged
merged 1 commit into from
Jul 30, 2022

Conversation

michaeljmarshall
Copy link
Member

@michaeljmarshall michaeljmarshall commented Jul 15, 2022

Related: #13161 and #12948

Motivation

The current Java Client consumer code does not clean up a pending consumer on the server when the client's timeout expires. This creates problems when the client tries to recreate the consumer with a Subscribe command. In this case, we see warnings like the following:

2022-07-13T23:51:53,323+0000 [pulsar-io-12-6] WARN org.apache.pulsar.broker.service.ServerCnx - [/10.249.212.73:41536][persistent://public/default/test][test] Consumer with id is already present on the connection, consumerId=66

Modifications

  • Send CloseConsumer command when the Subscribe command times out from the client perspective.

Verifying this change

This change includes new tests.

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

This change introduces new recommendation for the Pulsar Protocol, but it does not make any breaking changes.

Note also that the C++ client already follows this behavior.

Documentation

  • doc

@michaeljmarshall michaeljmarshall self-assigned this Jul 15, 2022
@michaeljmarshall michaeljmarshall changed the title [Java Client] Send CloseConsumer on timeout [Java Client] Send CloseConsumer on client timeout Jul 15, 2022
@github-actions github-actions bot added the doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. label Jul 15, 2022
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

LGTM

we should port this to 2.10 and other active branches

@lhotari
Copy link
Member

lhotari commented Jul 15, 2022

Just wondering if the underlying problem could be related to #14970 ?
There is also #15051 which made changes in this area.

@lhotari
Copy link
Member

lhotari commented Jul 15, 2022

This creates problems when the client tries to recreate the consumer with a Subscribe command. In this case, we see warnings like the following:

2022-07-13T23:51:53,323+0000 [pulsar-io-12-6] WARN org.apache.pulsar.broker.service.ServerCnx - [/10.249.212.73:41536][persistent://public/default/test][test] Consumer with id is already present on the connection, consumerId=66

@michaeljmarshall were there other issues or was it only about a warning in the logs?

@shibd
Copy link
Member

shibd commented Jul 17, 2022

@michaeljmarshall were there other issues or was it only about a warning in the logs?

+1, and a question, what if the close consumer command times out?

@eolivelli eolivelli closed this Jul 29, 2022
@eolivelli eolivelli reopened this Jul 29, 2022
@eolivelli
Copy link
Contributor

closed/reopened to trigger CI

@codelipenghui codelipenghui added this to the 2.11.0 milestone Jul 29, 2022
@Technoboy- Technoboy- merged commit 8f31655 into apache:master Jul 30, 2022
BewareMyPower pushed a commit that referenced this pull request Aug 2, 2022
@BewareMyPower BewareMyPower added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Aug 2, 2022
Gleiphir2769 pushed a commit to Gleiphir2769/pulsar that referenced this pull request Aug 4, 2022
momo-jun added a commit to momo-jun/pulsar that referenced this pull request Aug 5, 2022
codelipenghui pushed a commit that referenced this pull request Aug 8, 2022
@mattisonchao mattisonchao added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label Aug 10, 2022
mattisonchao pushed a commit that referenced this pull request Aug 10, 2022
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Aug 16, 2022
(cherry picked from commit 8f31655)
(cherry picked from commit c2bb553)
@michaeljmarshall michaeljmarshall deleted the close-consumer branch August 23, 2022 17:04
RobertIndie pushed a commit to apache/pulsar-client-go that referenced this pull request Jul 20, 2023
### Motivation

This change is the same as apache/pulsar#13161 and apache/pulsar#16616, and is justified by these lines of our binary protocol spec:

* https://github.com/apache/pulsar-site/blob/9b4b3d39014bd47c0bb9f66742b89bcb40ed7f07/docs/developing-binary-protocol.md?plain=1#L301-L304
* https://github.com/apache/pulsar-site/blob/9b4b3d39014bd47c0bb9f66742b89bcb40ed7f07/docs/developing-binary-protocol.md?plain=1#L468-L471

### Modifications

* When a producer or a consumer times out during creation, make an attempt to close the producer or consumer by sending the appropriate close command. Failures can safely be ignored because the only time that the close will actually matter is when the TCP connection is open for other protocol messages. The one nuance is that we send the close command to the same address pair that we send the create command.
RobertIndie pushed a commit to apache/pulsar-client-go that referenced this pull request Sep 7, 2023
### Motivation

This change is the same as apache/pulsar#13161 and apache/pulsar#16616, and is justified by these lines of our binary protocol spec:

* https://github.com/apache/pulsar-site/blob/9b4b3d39014bd47c0bb9f66742b89bcb40ed7f07/docs/developing-binary-protocol.md?plain=1#L301-L304
* https://github.com/apache/pulsar-site/blob/9b4b3d39014bd47c0bb9f66742b89bcb40ed7f07/docs/developing-binary-protocol.md?plain=1#L468-L471

### Modifications

* When a producer or a consumer times out during creation, make an attempt to close the producer or consumer by sending the appropriate close command. Failures can safely be ignored because the only time that the close will actually matter is when the TCP connection is open for other protocol messages. The one nuance is that we send the close command to the same address pair that we send the create command.

(cherry picked from commit d4e08c6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/client cherry-picked/branch-2.8 Archived: 2.8 is end of life cherry-picked/branch-2.9 Archived: 2.9 is end of life cherry-picked/branch-2.10 doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. release/2.8.4 release/2.9.4 release/2.10.2
Projects
None yet
Development

Successfully merging this pull request may close these issues.