[GitHub] [pulsar] congbobo184 commented on a diff in pull request #19236: [improve][monitoring]PIP-231: Add `topic_load_failed` metric

[GitBox]

congbobo184 commented on code in PR #19236:
URL: https://github.com/apache/pulsar/pull/19236#discussion_r1070641433


##
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/PulsarStats.java:
##
@@ -239,6 +239,14 @@ public void recordTopicLoadTimeValue(String topic, long 
topicLoadLatencyMs) {
 }
 }
 
+public void recordTopicLoadFailed(String topic) {
+try {
+brokerOperabilityMetrics.recordTopicLoadFailed();
+} catch (Exception ex) {

Review Comment:
   I don't think it needs catch exception, because we don't need to catch any 
RuntimeException here.



-- 
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-unsubscr...@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [pulsar] congbobo184 commented on a diff in pull request #19236: [improve][monitoring]PIP-231: Add `topic_load_failed` metric

[GitBox]

congbobo184 commented on code in PR #19236:
URL: https://github.com/apache/pulsar/pull/19236#discussion_r1070568329


##
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java:
##
@@ -1176,6 +1176,10 @@ public void deleteTopicAuthenticationWithRetry(String 
topic, CompletableFuture> createNonPersistentTopic(String 
topic) {
 CompletableFuture> topicFuture = new 
CompletableFuture<>();
+topicFuture.exceptionally(t -> {
+pulsarStats.recordTopicLoadFailed(topic);
+return Optional.empty();

Review Comment:
   ```suggestion
   return null;
   ```



##
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/PulsarStats.java:
##
@@ -239,6 +239,14 @@ public void recordTopicLoadTimeValue(String topic, long 
topicLoadLatencyMs) {
 }
 }
 
+public void recordTopicLoadFailed(String topic) {
+try {
+brokerOperabilityMetrics.recordTopicLoadFailed();
+} catch (Exception ex) {

Review Comment:
   why catch exception?



##
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java:
##
@@ -1522,6 +1526,11 @@ private void createPersistentTopic(final String topic, 
boolean createIfMissing,
 TopicName topicName = TopicName.get(topic);
 final long topicCreateTimeMs = 
TimeUnit.NANOSECONDS.toMillis(System.nanoTime());
 
+topicFuture.exceptionally(t -> {
+pulsarStats.recordTopicLoadFailed(topic);
+return Optional.empty();

Review Comment:
   ```suggestion
   return null;
   ```



-- 
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-unsubscr...@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org