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/24 07:28:30 UTC
[GitHub] [pulsar] yangl opened a new pull request #11068: ignore the NotAllowedException when general metric data.
yangl opened a new pull request #11068:
URL: https://github.com/apache/pulsar/pull/11068
### Motivation
There are a lot of unnecessary warn logs when the prometheus get the metric data with the "/metrics" url, as follows
```xml
15:16:17.295 [pulsar-web-43-8] INFO org.eclipse.jetty.server.RequestLog - 10.206.28.67 - - [24/Jun/2021:15:16:17 +0800] "GET /metrics HTTP/1.1" 302 0 "-" "Prometheus/2.26.0" 1
15:16:17.306 [prometheus-stats-44-1] WARN org.apache.pulsar.broker.stats.prometheus.TransactionAggregator - Transaction pending ack generate managedLedgerStats fail!
java.util.concurrent.ExecutionException: org.apache.pulsar.broker.service.BrokerServiceException$NotAllowedException: Pending ack handle don't use managedLedger!
at java.util.concurrent.CompletableFuture.reportGet(CompletableFuture.java:395) ~[?:?]
at java.util.concurrent.CompletableFuture.get(CompletableFuture.java:1999) ~[?:?]
at org.apache.pulsar.broker.stats.prometheus.TransactionAggregator.lambda$generate$0(TransactionAggregator.java:66) ~[org.apache.pulsar-pulsar-broker-2.9.0-SNAPSHOT.jar:2.9.0-SNAPSHOT]
at java.util.ArrayList.forEach(ArrayList.java:1541) ~[?:?]
at org.apache.pulsar.broker.stats.prometheus.TransactionAggregator.lambda$generate$1(TransactionAggregator.java:62) ~[org.apache.pulsar-pulsar-broker-2.9.0-SNAPSHOT.jar:2.9.0-SNAPSHOT]
at org.apache.pulsar.common.util.collections.ConcurrentOpenHashMap$Section.forEach(ConcurrentOpenHashMap.java:387) ~[org.apache.pulsar-pulsar-common-2.9.0-SNAPSHOT.jar:2.9.0-SNAPSHOT]
at org.apache.pulsar.common.util.collections.ConcurrentOpenHashMap.forEach(ConcurrentOpenHashMap.java:159) ~[org.apache.pulsar-pulsar-common-2.9.0-SNAPSHOT.jar:2.9.0-SNAPSHOT]
at org.apache.pulsar.broker.stats.prometheus.TransactionAggregator.lambda$generate$2(TransactionAggregator.java:60) ~[org.apache.pulsar-pulsar-broker-2.9.0-SNAPSHOT.jar:2.9.0-SNAPSHOT]
at org.apache.pulsar.common.util.collections.ConcurrentOpenHashMap$Section.forEach(ConcurrentOpenHashMap.java:387) ~[org.apache.pulsar-pulsar-common-2.9.0-SNAPSHOT.jar:2.9.0-SNAPSHOT]
at org.apache.pulsar.common.util.collections.ConcurrentOpenHashMap.forEach(ConcurrentOpenHashMap.java:159) ~[org.apache.pulsar-pulsar-common-2.9.0-SNAPSHOT.jar:2.9.0-SNAPSHOT]
at org.apache.pulsar.broker.stats.prometheus.TransactionAggregator.lambda$generate$3(TransactionAggregator.java:59) ~[org.apache.pulsar-pulsar-broker-2.9.0-SNAPSHOT.jar:2.9.0-SNAPSHOT]
at org.apache.pulsar.common.util.collections.ConcurrentOpenHashMap$Section.forEach(ConcurrentOpenHashMap.java:387) ~[org.apache.pulsar-pulsar-common-2.9.0-SNAPSHOT.jar:2.9.0-SNAPSHOT]
at org.apache.pulsar.common.util.collections.ConcurrentOpenHashMap.forEach(ConcurrentOpenHashMap.java:159) ~[org.apache.pulsar-pulsar-common-2.9.0-SNAPSHOT.jar:2.9.0-SNAPSHOT]
at org.apache.pulsar.broker.stats.prometheus.TransactionAggregator.generate(TransactionAggregator.java:57) ~[org.apache.pulsar-pulsar-broker-2.9.0-SNAPSHOT.jar:2.9.0-SNAPSHOT]
at org.apache.pulsar.broker.stats.prometheus.PrometheusMetricsGenerator.generate(PrometheusMetricsGenerator.java:110) ~[org.apache.pulsar-pulsar-broker-2.9.0-SNAPSHOT.jar:2.9.0-SNAPSHOT]
at org.apache.pulsar.broker.stats.prometheus.PrometheusMetricsServlet.lambda$doGet$0(PrometheusMetricsServlet.java:75) ~[org.apache.pulsar-pulsar-broker-2.9.0-SNAPSHOT.jar:2.9.0-SNAPSHOT]
at org.apache.bookkeeper.mledger.util.SafeRun$1.safeRun(SafeRun.java:32) [org.apache.pulsar-managed-ledger-2.9.0-SNAPSHOT.jar:2.9.0-SNAPSHOT]
at org.apache.bookkeeper.common.util.SafeRunnable.run(SafeRunnable.java:36) [org.apache.bookkeeper-bookkeeper-common-4.14.1.jar:4.14.1]
at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515) [?:?]
at java.util.concurrent.FutureTask.run(FutureTask.java:264) [?:?]
at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304) [?:?]
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) [?:?]
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) [?:?]
at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) [io.netty-netty-common-4.1.63.Final.jar:4.1.63.Final]
at java.lang.Thread.run(Thread.java:834) [?:?]
Caused by: org.apache.pulsar.broker.service.BrokerServiceException$NotAllowedException: Pending ack handle don't use managedLedger!
at org.apache.pulsar.broker.service.persistent.PersistentSubscription.getPendingAckManageLedger(PersistentSubscription.java:1159) ~[org.apache.pulsar-pulsar-broker-2.9.0-SNAPSHOT.jar:2.9.0-SNAPSHOT]
... 23 more
```
the root cause is: pendingAckHandle may be instance of `PendingAckHandleDisabled` or `PendingAckHandleImpl`, but only the `PendingAckHandleImpl` can general metric data.
```java
public CompletableFuture<ManagedLedger> getPendingAckManageLedger() {
if (this.pendingAckHandle instanceof PendingAckHandleImpl) {
return ((PendingAckHandleImpl) this.pendingAckHandle).getStoreManageLedger();
} else {
return FutureUtil.failedFuture(new NotAllowedException("Pending ack handle don't use managedLedger!"));
}
}
```
### Modifications
ignore the special NotAllowedException.
--
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] hangc0276 removed a comment on pull request #11068: Fix the NotAllowedException when general metric data.
Posted by GitBox <gi...@apache.org>.
hangc0276 removed a comment on pull request #11068:
URL: https://github.com/apache/pulsar/pull/11068#issuecomment-893225881
@yangl Could you please address @congbobo184 's comments?
--
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] congbobo184 commented on pull request #11068: Fix the NotAllowedException when general metric data.
Posted by GitBox <gi...@apache.org>.
congbobo184 commented on pull request #11068:
URL: https://github.com/apache/pulsar/pull/11068#issuecomment-882589909
@yangl Could you please add some test for it？
--
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] hangc0276 commented on pull request #11068: Fix the NotAllowedException when general metric data.
Posted by GitBox <gi...@apache.org>.
hangc0276 commented on pull request #11068:
URL: https://github.com/apache/pulsar/pull/11068#issuecomment-893225881
@yangl Could you please address @congbobo184 's comments?
--
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] hangc0276 commented on pull request #11068: Fix the NotAllowedException when general metric data.
Posted by GitBox <gi...@apache.org>.
hangc0276 commented on pull request #11068:
URL: https://github.com/apache/pulsar/pull/11068#issuecomment-893226726
@yangl I am cutting 2.8.1, could you please address @congbobo184 's comments? If you can't get a change to handle it, we can move the fix to 2.8.2
--
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] yangl closed pull request #11068: Fix the NotAllowedException when general metric data.
Posted by GitBox <gi...@apache.org>.
yangl closed pull request #11068:
URL: https://github.com/apache/pulsar/pull/11068
--
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] merlimat commented on a change in pull request #11068: ignore the NotAllowedException when general metric data.
Posted by GitBox <gi...@apache.org>.
merlimat commented on a change in pull request #11068:
URL: https://github.com/apache/pulsar/pull/11068#discussion_r657789798
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/stats/prometheus/TransactionAggregator.java
##########
@@ -67,7 +68,11 @@ public static void generate(PulsarService pulsar, SimpleTextOutputStream stream,
generateManageLedgerStats(managedLedger,
stream, cluster, namespace, name, subscription.getName());
} catch (Exception e) {
- log.warn("Transaction pending ack generate managedLedgerStats fail!", e);
+ if (e.getCause() instanceof BrokerServiceException.NotAllowedException) {
Review comment:
Instead of throwing exception which has a high cost (since it has to generate the entire stack trace), we could change the method to return `CompletableFuture<Optional<ManagedLedger>>` instead
--
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] yangl commented on pull request #11068: Fix the NotAllowedException when general metric data.
Posted by GitBox <gi...@apache.org>.
yangl commented on pull request #11068:
URL: https://github.com/apache/pulsar/pull/11068#issuecomment-882197967
/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] codelipenghui commented on pull request #11068: Fix the NotAllowedException when general metric data.
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #11068:
URL: https://github.com/apache/pulsar/pull/11068#issuecomment-868561422
@congbobo184 Could you please help review this one?
--
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] yangl commented on pull request #11068: Fix the NotAllowedException when general metric data.
Posted by GitBox <gi...@apache.org>.
yangl commented on pull request #11068:
URL: https://github.com/apache/pulsar/pull/11068#issuecomment-893300654
> @yangl I am cutting 2.8.1, could you please address @congbobo184 's comments? If you can't get a change to handle it, we can move the fix to 2.8.2
please move to 2.8.2, thanks.
--
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] yangl commented on pull request #11068: Fix the NotAllowedException when general metric data.
Posted by GitBox <gi...@apache.org>.
yangl commented on pull request #11068:
URL: https://github.com/apache/pulsar/pull/11068#issuecomment-893300654
> @yangl I am cutting 2.8.1, could you please address @congbobo184 's comments? If you can't get a change to handle it, we can move the fix to 2.8.2
please move to 2.8.2, thanks.
--
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] yangl commented on a change in pull request #11068: ignore the NotAllowedException when general metric data.
Posted by GitBox <gi...@apache.org>.
yangl commented on a change in pull request #11068:
URL: https://github.com/apache/pulsar/pull/11068#discussion_r658449241
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/stats/prometheus/TransactionAggregator.java
##########
@@ -67,7 +68,11 @@ public static void generate(PulsarService pulsar, SimpleTextOutputStream stream,
generateManageLedgerStats(managedLedger,
stream, cluster, namespace, name, subscription.getName());
} catch (Exception e) {
- log.warn("Transaction pending ack generate managedLedgerStats fail!", e);
+ if (e.getCause() instanceof BrokerServiceException.NotAllowedException) {
Review comment:
yeah, good idea. done. Thanks a lot.
--
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] hangc0276 removed a comment on pull request #11068: Fix the NotAllowedException when general metric data.
Posted by GitBox <gi...@apache.org>.
hangc0276 removed a comment on pull request #11068:
URL: https://github.com/apache/pulsar/pull/11068#issuecomment-893225881
@yangl Could you please address @congbobo184 's comments?
--
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] hangc0276 commented on pull request #11068: Fix the NotAllowedException when general metric data.
Posted by GitBox <gi...@apache.org>.
hangc0276 commented on pull request #11068:
URL: https://github.com/apache/pulsar/pull/11068#issuecomment-893225881
--
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] congbobo184 commented on pull request #11068: Fix the NotAllowedException when general metric data.
Posted by GitBox <gi...@apache.org>.
congbobo184 commented on pull request #11068:
URL: https://github.com/apache/pulsar/pull/11068#issuecomment-882589909
@yangl Could you please add some test for it？
--
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] congbobo184 commented on pull request #11068: Fix the NotAllowedException when general metric data.
Posted by GitBox <gi...@apache.org>.
congbobo184 commented on pull request #11068:
URL: https://github.com/apache/pulsar/pull/11068#issuecomment-882589909
@yangl Could you please add some test for it？
--
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