You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/06/10 03:30:50 UTC
[GitHub] [pulsar] nodece opened a new pull request, #15761: [fix][client] Remove consumer when close consumer command is received
nodece opened a new pull request, #15761:
URL: https://github.com/apache/pulsar/pull/15761
Signed-off-by: Zixuan Liu <no...@gmail.com>
### Motivation
When the close consumer command is received, we just disconnected and didn't remove the cache, which will cause us to receive repeated messages.
This case happened in `org.apache.pulsar.broker.service.SubscriptionSeekTest#testSeekByFunctionAndMultiTopic` test of #15568.
### Modifications
- Use `remove()` instead of `get()`
### Documentation
- [x] `no-need-doc`
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mattisonchao merged pull request #15761: [fix][client] Remove consumer when close consumer command is received
Posted by GitBox <gi...@apache.org>.
mattisonchao merged PR #15761:
URL: https://github.com/apache/pulsar/pull/15761
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nodece closed pull request #15761: [fix][client] Remove consumer when close consumer command is received
Posted by GitBox <gi...@apache.org>.
nodece closed pull request #15761: [fix][client] Remove consumer when close consumer command is received
URL: https://github.com/apache/pulsar/pull/15761
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nodece commented on pull request #15761: [fix][client] Remove consumer when close consumer command is received
Posted by GitBox <gi...@apache.org>.
nodece commented on PR #15761:
URL: https://github.com/apache/pulsar/pull/15761#issuecomment-1137027889
@RobertIndie It should be stable, maybe I add the message inclusive feature breaks this, but I cannot find the root cause.
I tried this code to test the `org.apache.pulsar.broker.service.SubscriptionSeekTest#testSeekByFunctionAndMultiTopic` of #14883, it is passed.
If you are interested in this, we can find the root cause together.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] RobertIndie commented on pull request #15761: [fix][client] Remove consumer when close consumer command is received
Posted by GitBox <gi...@apache.org>.
RobertIndie commented on PR #15761:
URL: https://github.com/apache/pulsar/pull/15761#issuecomment-1137012676
Do you mean that testSeekByFunctionAndMultiTopic is a flaky test?
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on a diff in pull request #15761: [fix][client] Remove consumer when close consumer command is received
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on code in PR #15761:
URL: https://github.com/apache/pulsar/pull/15761#discussion_r885166888
##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ClientCnx.java:
##########
@@ -747,7 +747,7 @@ protected void handleCloseProducer(CommandCloseProducer closeProducer) {
protected void handleCloseConsumer(CommandCloseConsumer closeConsumer) {
log.info("[{}] Broker notification of Closed consumer: {}", remoteAddress, closeConsumer.getConsumerId());
final long consumerId = closeConsumer.getConsumerId();
- ConsumerImpl<?> consumer = consumers.get(consumerId);
+ ConsumerImpl<?> consumer = consumers.remove(consumerId);
Review Comment:
Nice catch! Could you please add a test?
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nodece commented on pull request #15761: [fix][client] Remove consumer when close consumer command is received
Posted by GitBox <gi...@apache.org>.
nodece commented on PR #15761:
URL: https://github.com/apache/pulsar/pull/15761#issuecomment-1143081476
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nodece commented on pull request #15761: [fix][client] Remove consumer when close consumer command is received
Posted by GitBox <gi...@apache.org>.
nodece commented on PR #15761:
URL: https://github.com/apache/pulsar/pull/15761#issuecomment-1141643999
> It seems that the producer has the same problem. We can fix it in another PR.
Yes, I can make a PR to fix the `closeHandleProducer`.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on a diff in pull request #15761: [fix][client] Remove consumer when close consumer command is received
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on code in PR #15761:
URL: https://github.com/apache/pulsar/pull/15761#discussion_r894376775
##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ClientCnx.java:
##########
@@ -115,6 +115,7 @@ public class ClientCnx extends PulsarHandler {
.expectedItems(16)
.concurrencyLevel(1)
.build();
+ @Getter
Review Comment:
It's better to return an immutable map to prevent anyone to modify it externally.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nodece commented on pull request #15761: [fix][client] Remove consumer when close consumer command is received
Posted by GitBox <gi...@apache.org>.
nodece commented on PR #15761:
URL: https://github.com/apache/pulsar/pull/15761#issuecomment-1151859651
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nodece closed pull request #15761: [fix][client] Remove consumer when close consumer command is received
Posted by GitBox <gi...@apache.org>.
nodece closed pull request #15761: [fix][client] Remove consumer when close consumer command is received
URL: https://github.com/apache/pulsar/pull/15761
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nodece commented on a diff in pull request #15761: [fix][client] Remove consumer when close consumer command is received
Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #15761:
URL: https://github.com/apache/pulsar/pull/15761#discussion_r894640933
##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ClientCnx.java:
##########
@@ -115,6 +115,7 @@ public class ClientCnx extends PulsarHandler {
.expectedItems(16)
.concurrencyLevel(1)
.build();
+ @Getter
Review Comment:
Done.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nodece commented on pull request #15761: [fix][client] Remove consumer when close consumer command is received
Posted by GitBox <gi...@apache.org>.
nodece commented on PR #15761:
URL: https://github.com/apache/pulsar/pull/15761#issuecomment-1136645171
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nodece commented on pull request #15761: [fix][client] Remove consumer when close consumer command is received
Posted by GitBox <gi...@apache.org>.
nodece commented on PR #15761:
URL: https://github.com/apache/pulsar/pull/15761#issuecomment-1141704719
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nodece commented on pull request #15761: [fix][client] Remove consumer when close consumer command is received
Posted by GitBox <gi...@apache.org>.
nodece commented on PR #15761:
URL: https://github.com/apache/pulsar/pull/15761#issuecomment-1143181041
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nodece commented on a diff in pull request #15761: [fix][client] Remove consumer when close consumer command is received
Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #15761:
URL: https://github.com/apache/pulsar/pull/15761#discussion_r894115135
##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ClientCnx.java:
##########
@@ -747,7 +747,7 @@ protected void handleCloseProducer(CommandCloseProducer closeProducer) {
protected void handleCloseConsumer(CommandCloseConsumer closeConsumer) {
log.info("[{}] Broker notification of Closed consumer: {}", remoteAddress, closeConsumer.getConsumerId());
final long consumerId = closeConsumer.getConsumerId();
- ConsumerImpl<?> consumer = consumers.get(consumerId);
+ ConsumerImpl<?> consumer = consumers.remove(consumerId);
Review Comment:
Done.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nodece commented on pull request #15761: [fix][client] Remove consumer when close consumer command is received
Posted by GitBox <gi...@apache.org>.
nodece commented on PR #15761:
URL: https://github.com/apache/pulsar/pull/15761#issuecomment-1139237820
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org