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 2021/06/21 09:18:25 UTC
[GitHub] [pulsar] Shoothzj opened a new pull request #10997: [Issue 10996] release multiTopicsConsumerImpl when unsubscribe
Shoothzj opened a new pull request #10997:
URL: https://github.com/apache/pulsar/pull/10997
Fixes 10996
### Motivation
MutliConsumer not released after calling method unsubscribe, and can't be released by other method.
### Modifications
Make MutliConsumer release completely on method unsuscribe.
### Verifying this change
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaoran10 commented on pull request #10997: [Issue 10996] release multiTopicsConsumerImpl when unsubscribe
Posted by GitBox <gi...@apache.org>.
gaoran10 commented on pull request #10997:
URL: https://github.com/apache/pulsar/pull/10997#issuecomment-865504984
Only need to change docs?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] merlimat merged pull request #10997: [Issue 10996] release multiTopicsConsumerImpl when unsubscribe
Posted by GitBox <gi...@apache.org>.
merlimat merged pull request #10997:
URL: https://github.com/apache/pulsar/pull/10997
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] merlimat merged pull request #10997: [Issue 10996] release multiTopicsConsumerImpl when unsubscribe
Posted by GitBox <gi...@apache.org>.
merlimat merged pull request #10997:
URL: https://github.com/apache/pulsar/pull/10997
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] 315157973 commented on a change in pull request #10997: [Issue 10996] release multiTopicsConsumerImpl when unsubscribe
Posted by GitBox <gi...@apache.org>.
315157973 commented on a change in pull request #10997:
URL: https://github.com/apache/pulsar/pull/10997#discussion_r655438083
##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/MultiTopicsConsumerImpl.java
##########
@@ -560,12 +560,9 @@ public void negativeAcknowledge(MessageId messageId) {
.whenComplete((r, ex) -> {
if (ex == null) {
setState(State.Closed);
- unAckedMessageTracker.close();
+ cleanupMultiConsumer();
closeFuture.complete(null);
log.info("[{}] [{}] Closed Topics Consumer", topic, subscription);
Review comment:
This log seems to be repeated
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on a change in pull request #10997: [Issue 10996] release multiTopicsConsumerImpl when unsubscribe
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on a change in pull request #10997:
URL: https://github.com/apache/pulsar/pull/10997#discussion_r655847450
##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/MultiTopicsConsumerImpl.java
##########
@@ -560,12 +560,9 @@ public void negativeAcknowledge(MessageId messageId) {
.whenComplete((r, ex) -> {
if (ex == null) {
setState(State.Closed);
- unAckedMessageTracker.close();
+ cleanupMultiConsumer();
closeFuture.complete(null);
log.info("[{}] [{}] Closed Topics Consumer", topic, subscription);
Review comment:
Your are right. Fixed
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #10997: [Issue 10996] release multiTopicsConsumerImpl when unsubscribe
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on pull request #10997:
URL: https://github.com/apache/pulsar/pull/10997#issuecomment-865475498
> @Shoothzj thanks for your contribution. For this PR, do we need to update docs?
We don't need to update docs. Cause user can't not call `unsubscribe` and `close` at the same time. People who call `unsubscribe` will out of memory.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #10997: [Issue 10996] release multiTopicsConsumerImpl when unsubscribe
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on pull request #10997:
URL: https://github.com/apache/pulsar/pull/10997#issuecomment-865475498
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] 315157973 commented on a change in pull request #10997: [Issue 10996] release multiTopicsConsumerImpl when unsubscribe
Posted by GitBox <gi...@apache.org>.
315157973 commented on a change in pull request #10997:
URL: https://github.com/apache/pulsar/pull/10997#discussion_r655438083
##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/MultiTopicsConsumerImpl.java
##########
@@ -560,12 +560,9 @@ public void negativeAcknowledge(MessageId messageId) {
.whenComplete((r, ex) -> {
if (ex == null) {
setState(State.Closed);
- unAckedMessageTracker.close();
+ cleanupMultiConsumer();
closeFuture.complete(null);
log.info("[{}] [{}] Closed Topics Consumer", topic, subscription);
Review comment:
This log seems to be repeated
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #10997: [Issue 10996] release multiTopicsConsumerImpl when unsubscribe
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on pull request #10997:
URL: https://github.com/apache/pulsar/pull/10997#issuecomment-865525655
> Only need to change docs?
Sorry to push the wrong commit.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Anonymitaet commented on pull request #10997: [Issue 10996] release multiTopicsConsumerImpl when unsubscribe
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on pull request #10997:
URL: https://github.com/apache/pulsar/pull/10997#issuecomment-865470913
@Shoothzj thanks for your contribution. For this PR, do we need to update docs?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on a change in pull request #10997: [Issue 10996] release multiTopicsConsumerImpl when unsubscribe
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on a change in pull request #10997:
URL: https://github.com/apache/pulsar/pull/10997#discussion_r655847450
##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/MultiTopicsConsumerImpl.java
##########
@@ -560,12 +560,9 @@ public void negativeAcknowledge(MessageId messageId) {
.whenComplete((r, ex) -> {
if (ex == null) {
setState(State.Closed);
- unAckedMessageTracker.close();
+ cleanupMultiConsumer();
closeFuture.complete(null);
log.info("[{}] [{}] Closed Topics Consumer", topic, subscription);
Review comment:
Your are right. Fixed
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaoran10 commented on pull request #10997: [Issue 10996] release multiTopicsConsumerImpl when unsubscribe
Posted by GitBox <gi...@apache.org>.
gaoran10 commented on pull request #10997:
URL: https://github.com/apache/pulsar/pull/10997#issuecomment-865504984
Only need to change docs?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Anonymitaet commented on pull request #10997: [Issue 10996] release multiTopicsConsumerImpl when unsubscribe
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on pull request #10997:
URL: https://github.com/apache/pulsar/pull/10997#issuecomment-865470913
@Shoothzj thanks for your contribution. For this PR, do we need to update docs?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org