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/02/10 01:10:38 UTC
[GitHub] [pulsar] Demogorgon314 opened a new pull request #14196: [Broker] Change broker producer fence log level
Demogorgon314 opened a new pull request #14196:
URL: https://github.com/apache/pulsar/pull/14196
Fixes #12290
### Motivation
Fixes #12290
### Modifications
* Change broker producer fence log level to debug level.
* Change others producer exception log level to warn level.
### Documentation
Need to update docs?
- [ ] `doc-required`
(If you need help on updating docs, create a doc issue)
- [x] `no-need-doc`
(Please explain why)
- [ ] `doc`
(If this PR contains doc changes)
--
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 change in pull request #14196: [Broker] Change broker producer fence log level
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on a change in pull request #14196:
URL: https://github.com/apache/pulsar/pull/14196#discussion_r805800333
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java
##########
@@ -1353,8 +1353,15 @@ private void buildProducerAndAddTopic(Topic topic, long producerId, String produ
producers.remove(producerId, producerFuture);
}).exceptionally(ex -> {
- log.error("[{}] Failed to add producer to topic {}: producerId={}, {}",
- remoteAddress, topicName, producerId, ex.getMessage());
+ if (ex.getCause() instanceof BrokerServiceException.ProducerFencedException) {
+ if (log.isDebugEnabled()) {
+ log.debug("[{}] Failed to add producer to topic {}: producerId={}, {}",
+ remoteAddress, topicName, producerId, ex.getMessage());
+ }
+ } else {
+ log.warn("[{}] Failed to add producer to topic {}: producerId={}, {}",
Review comment:
Ok, LGTM.
--
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] Demogorgon314 commented on pull request #14196: [Broker] Change broker producer fence log level
Posted by GitBox <gi...@apache.org>.
Demogorgon314 commented on pull request #14196:
URL: https://github.com/apache/pulsar/pull/14196#issuecomment-1034581458
/pulsarbot run-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] yuruguo commented on a change in pull request #14196: [Broker] Change broker producer fence log level
Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #14196:
URL: https://github.com/apache/pulsar/pull/14196#discussion_r806470964
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java
##########
@@ -1353,8 +1353,15 @@ private void buildProducerAndAddTopic(Topic topic, long producerId, String produ
producers.remove(producerId, producerFuture);
}).exceptionally(ex -> {
- log.error("[{}] Failed to add producer to topic {}: producerId={}, {}",
- remoteAddress, topicName, producerId, ex.getMessage());
+ if (ex.getCause() instanceof BrokerServiceException.ProducerFencedException) {
+ if (log.isDebugEnabled()) {
+ log.debug("[{}] Failed to add producer to topic {}: producerId={}, {}",
+ remoteAddress, topicName, producerId, ex.getCause().getMessage());
+ }
+ } else {
+ log.warn("[{}] Failed to add producer to topic {}: producerId={}, {}",
+ remoteAddress, topicName, producerId, ex.getMessage());
Review comment:
`ex.getMessage()` -> `exception.getCause().getMessage()`？
--
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] yuruguo commented on a change in pull request #14196: [Broker] Change broker producer fence log level
Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #14196:
URL: https://github.com/apache/pulsar/pull/14196#discussion_r806470964
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java
##########
@@ -1353,8 +1353,15 @@ private void buildProducerAndAddTopic(Topic topic, long producerId, String produ
producers.remove(producerId, producerFuture);
}).exceptionally(ex -> {
- log.error("[{}] Failed to add producer to topic {}: producerId={}, {}",
- remoteAddress, topicName, producerId, ex.getMessage());
+ if (ex.getCause() instanceof BrokerServiceException.ProducerFencedException) {
+ if (log.isDebugEnabled()) {
+ log.debug("[{}] Failed to add producer to topic {}: producerId={}, {}",
+ remoteAddress, topicName, producerId, ex.getCause().getMessage());
+ }
+ } else {
+ log.warn("[{}] Failed to add producer to topic {}: producerId={}, {}",
+ remoteAddress, topicName, producerId, ex.getMessage());
Review comment:
`ex.getMessage()` -> `ex.getCause().getMessage()`？
--
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] Demogorgon314 commented on pull request #14196: [Broker] Change broker producer fence log level
Posted by GitBox <gi...@apache.org>.
Demogorgon314 commented on pull request #14196:
URL: https://github.com/apache/pulsar/pull/14196#issuecomment-1036984874
/pulsarbot run-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] yuruguo commented on a change in pull request #14196: [Broker] Change broker producer fence log level
Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #14196:
URL: https://github.com/apache/pulsar/pull/14196#discussion_r806470964
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java
##########
@@ -1353,8 +1353,15 @@ private void buildProducerAndAddTopic(Topic topic, long producerId, String produ
producers.remove(producerId, producerFuture);
}).exceptionally(ex -> {
- log.error("[{}] Failed to add producer to topic {}: producerId={}, {}",
- remoteAddress, topicName, producerId, ex.getMessage());
+ if (ex.getCause() instanceof BrokerServiceException.ProducerFencedException) {
+ if (log.isDebugEnabled()) {
+ log.debug("[{}] Failed to add producer to topic {}: producerId={}, {}",
+ remoteAddress, topicName, producerId, ex.getCause().getMessage());
+ }
+ } else {
+ log.warn("[{}] Failed to add producer to topic {}: producerId={}, {}",
+ remoteAddress, topicName, producerId, ex.getMessage());
Review comment:
`ex.getMessage()` -> `exception.getCause().getMessage()`？
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java
##########
@@ -1353,8 +1353,15 @@ private void buildProducerAndAddTopic(Topic topic, long producerId, String produ
producers.remove(producerId, producerFuture);
}).exceptionally(ex -> {
- log.error("[{}] Failed to add producer to topic {}: producerId={}, {}",
- remoteAddress, topicName, producerId, ex.getMessage());
+ if (ex.getCause() instanceof BrokerServiceException.ProducerFencedException) {
+ if (log.isDebugEnabled()) {
+ log.debug("[{}] Failed to add producer to topic {}: producerId={}, {}",
+ remoteAddress, topicName, producerId, ex.getCause().getMessage());
+ }
+ } else {
+ log.warn("[{}] Failed to add producer to topic {}: producerId={}, {}",
+ remoteAddress, topicName, producerId, ex.getMessage());
Review comment:
`ex.getMessage()` -> `ex.getCause().getMessage()`？
--
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] Demogorgon314 commented on a change in pull request #14196: [Broker] Change broker producer fence log level
Posted by GitBox <gi...@apache.org>.
Demogorgon314 commented on a change in pull request #14196:
URL: https://github.com/apache/pulsar/pull/14196#discussion_r806497475
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java
##########
@@ -1353,8 +1353,15 @@ private void buildProducerAndAddTopic(Topic topic, long producerId, String produ
producers.remove(producerId, producerFuture);
}).exceptionally(ex -> {
- log.error("[{}] Failed to add producer to topic {}: producerId={}, {}",
- remoteAddress, topicName, producerId, ex.getMessage());
+ if (ex.getCause() instanceof BrokerServiceException.ProducerFencedException) {
+ if (log.isDebugEnabled()) {
+ log.debug("[{}] Failed to add producer to topic {}: producerId={}, {}",
+ remoteAddress, topicName, producerId, ex.getCause().getMessage());
+ }
+ } else {
+ log.warn("[{}] Failed to add producer to topic {}: producerId={}, {}",
+ remoteAddress, topicName, producerId, ex.getMessage());
Review comment:
Thanks, addressed.
--
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] Demogorgon314 commented on a change in pull request #14196: [Broker] Change broker producer fence log level
Posted by GitBox <gi...@apache.org>.
Demogorgon314 commented on a change in pull request #14196:
URL: https://github.com/apache/pulsar/pull/14196#discussion_r805460138
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java
##########
@@ -1353,8 +1353,15 @@ private void buildProducerAndAddTopic(Topic topic, long producerId, String produ
producers.remove(producerId, producerFuture);
}).exceptionally(ex -> {
- log.error("[{}] Failed to add producer to topic {}: producerId={}, {}",
- remoteAddress, topicName, producerId, ex.getMessage());
+ if (ex.getCause() instanceof BrokerServiceException.ProducerFencedException) {
+ if (log.isDebugEnabled()) {
+ log.debug("[{}] Failed to add producer to topic {}: producerId={}, {}",
+ remoteAddress, topicName, producerId, ex.getMessage());
+ }
+ } else {
+ log.warn("[{}] Failed to add producer to topic {}: producerId={}, {}",
Review comment:
I see consume `handleSubscribe` are using warn log, should we change to error too? https://github.com/apache/pulsar/blob/e220e7934654bf8ce673ce447810455e27cfc282/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java#L1083
--
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 change in pull request #14196: [Broker] Change broker producer fence log level
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on a change in pull request #14196:
URL: https://github.com/apache/pulsar/pull/14196#discussion_r805457388
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java
##########
@@ -1353,8 +1353,15 @@ private void buildProducerAndAddTopic(Topic topic, long producerId, String produ
producers.remove(producerId, producerFuture);
}).exceptionally(ex -> {
- log.error("[{}] Failed to add producer to topic {}: producerId={}, {}",
- remoteAddress, topicName, producerId, ex.getMessage());
+ if (ex.getCause() instanceof BrokerServiceException.ProducerFencedException) {
+ if (log.isDebugEnabled()) {
+ log.debug("[{}] Failed to add producer to topic {}: producerId={}, {}",
+ remoteAddress, topicName, producerId, ex.getMessage());
+ }
+ } else {
+ log.warn("[{}] Failed to add producer to topic {}: producerId={}, {}",
Review comment:
Should be an error here? It's not expected behavior.
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java
##########
@@ -1353,8 +1353,15 @@ private void buildProducerAndAddTopic(Topic topic, long producerId, String produ
producers.remove(producerId, producerFuture);
}).exceptionally(ex -> {
- log.error("[{}] Failed to add producer to topic {}: producerId={}, {}",
- remoteAddress, topicName, producerId, ex.getMessage());
+ if (ex.getCause() instanceof BrokerServiceException.ProducerFencedException) {
+ if (log.isDebugEnabled()) {
+ log.debug("[{}] Failed to add producer to topic {}: producerId={}, {}",
+ remoteAddress, topicName, producerId, ex.getMessage());
Review comment:
```suggestion
remoteAddress, topicName, producerId, ex.getCause().getMessage());
```
--
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 change in pull request #14196: [Broker] Change broker producer fence log level
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on a change in pull request #14196:
URL: https://github.com/apache/pulsar/pull/14196#discussion_r805457388
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java
##########
@@ -1353,8 +1353,15 @@ private void buildProducerAndAddTopic(Topic topic, long producerId, String produ
producers.remove(producerId, producerFuture);
}).exceptionally(ex -> {
- log.error("[{}] Failed to add producer to topic {}: producerId={}, {}",
- remoteAddress, topicName, producerId, ex.getMessage());
+ if (ex.getCause() instanceof BrokerServiceException.ProducerFencedException) {
+ if (log.isDebugEnabled()) {
+ log.debug("[{}] Failed to add producer to topic {}: producerId={}, {}",
+ remoteAddress, topicName, producerId, ex.getMessage());
+ }
+ } else {
+ log.warn("[{}] Failed to add producer to topic {}: producerId={}, {}",
Review comment:
Should be an error here? It's not expected behavior.
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java
##########
@@ -1353,8 +1353,15 @@ private void buildProducerAndAddTopic(Topic topic, long producerId, String produ
producers.remove(producerId, producerFuture);
}).exceptionally(ex -> {
- log.error("[{}] Failed to add producer to topic {}: producerId={}, {}",
- remoteAddress, topicName, producerId, ex.getMessage());
+ if (ex.getCause() instanceof BrokerServiceException.ProducerFencedException) {
+ if (log.isDebugEnabled()) {
+ log.debug("[{}] Failed to add producer to topic {}: producerId={}, {}",
+ remoteAddress, topicName, producerId, ex.getMessage());
Review comment:
```suggestion
remoteAddress, topicName, producerId, ex.getCause().getMessage());
```
--
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] Demogorgon314 commented on pull request #14196: [Broker] Change broker producer fence log level
Posted by GitBox <gi...@apache.org>.
Demogorgon314 commented on pull request #14196:
URL: https://github.com/apache/pulsar/pull/14196#issuecomment-1039844935
/pulsarbot run-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] Demogorgon314 commented on pull request #14196: [Broker] Change broker producer fence log level
Posted by GitBox <gi...@apache.org>.
Demogorgon314 commented on pull request #14196:
URL: https://github.com/apache/pulsar/pull/14196#issuecomment-1039844935
/pulsarbot run-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] Demogorgon314 commented on a change in pull request #14196: [Broker] Change broker producer fence log level
Posted by GitBox <gi...@apache.org>.
Demogorgon314 commented on a change in pull request #14196:
URL: https://github.com/apache/pulsar/pull/14196#discussion_r806497475
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java
##########
@@ -1353,8 +1353,15 @@ private void buildProducerAndAddTopic(Topic topic, long producerId, String produ
producers.remove(producerId, producerFuture);
}).exceptionally(ex -> {
- log.error("[{}] Failed to add producer to topic {}: producerId={}, {}",
- remoteAddress, topicName, producerId, ex.getMessage());
+ if (ex.getCause() instanceof BrokerServiceException.ProducerFencedException) {
+ if (log.isDebugEnabled()) {
+ log.debug("[{}] Failed to add producer to topic {}: producerId={}, {}",
+ remoteAddress, topicName, producerId, ex.getCause().getMessage());
+ }
+ } else {
+ log.warn("[{}] Failed to add producer to topic {}: producerId={}, {}",
+ remoteAddress, topicName, producerId, ex.getMessage());
Review comment:
Thanks, addressed.
--
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] Demogorgon314 commented on a change in pull request #14196: [Broker] Change broker producer fence log level
Posted by GitBox <gi...@apache.org>.
Demogorgon314 commented on a change in pull request #14196:
URL: https://github.com/apache/pulsar/pull/14196#discussion_r805460138
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java
##########
@@ -1353,8 +1353,15 @@ private void buildProducerAndAddTopic(Topic topic, long producerId, String produ
producers.remove(producerId, producerFuture);
}).exceptionally(ex -> {
- log.error("[{}] Failed to add producer to topic {}: producerId={}, {}",
- remoteAddress, topicName, producerId, ex.getMessage());
+ if (ex.getCause() instanceof BrokerServiceException.ProducerFencedException) {
+ if (log.isDebugEnabled()) {
+ log.debug("[{}] Failed to add producer to topic {}: producerId={}, {}",
+ remoteAddress, topicName, producerId, ex.getMessage());
+ }
+ } else {
+ log.warn("[{}] Failed to add producer to topic {}: producerId={}, {}",
Review comment:
I see consume `handleSubscribe` are using warn log, should we change to error too? https://github.com/apache/pulsar/blob/e220e7934654bf8ce673ce447810455e27cfc282/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java#L1083
--
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 merged pull request #14196: [Broker] Change broker producer fence log level
Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #14196:
URL: https://github.com/apache/pulsar/pull/14196
--
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