Re: [PR] [fix] [broker] fix null lookup result when brokers are starting [pulsar]

[via GitHub]

lhotari commented on code in PR #23642:
URL: https://github.com/apache/pulsar/pull/23642#discussion_r1860677940


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/lookup/TopicLookupBase.java:
##########
@@ -342,6 +343,24 @@ public static CompletableFuture<ByteBuf> 
lookupTopicAsync(PulsarService pulsarSe
         return lookupfuture;
     }
 
+    /**
+     * Check if a internal client will get a null lookup result.
+     */
+    private static void printWarnLogIfLookupResUnexpected(TopicName topic, 
LookupData lookupData, LookupOptions options,
+                                                          PulsarService 
pulsar) {
+        if (!pulsar.getBrokerService().isSystemTopic(topic)) {
+            return;
+        }
+        boolean tlsEnabled = pulsar.getConfig().isBrokerClientTlsEnabled();
+        if (!tlsEnabled && StringUtils.isBlank(lookupData.getBrokerUrl())) {
+            log.warn("[{}] Unexpected lookup resul: brokerUrlTls is required 
when TLS isn't enabled. options: {},"
+                + " result {}", topic, options, lookupData);
+        } else if (tlsEnabled && 
StringUtils.isBlank(lookupData.getBrokerUrlTls())) {
+            log.warn("[{}] Unexpected lookup resul: brokerUrlTls is required 
when TLS is enabled. options: {},"

Review Comment:
   typo fix
   ```suggestion
               log.warn("[{}] Unexpected lookup result: brokerUrlTls is 
required when TLS is enabled. options: {},"
   ```



##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/lookup/TopicLookupBase.java:
##########
@@ -342,6 +343,24 @@ public static CompletableFuture<ByteBuf> 
lookupTopicAsync(PulsarService pulsarSe
         return lookupfuture;
     }
 
+    /**
+     * Check if a internal client will get a null lookup result.
+     */
+    private static void printWarnLogIfLookupResUnexpected(TopicName topic, 
LookupData lookupData, LookupOptions options,
+                                                          PulsarService 
pulsar) {
+        if (!pulsar.getBrokerService().isSystemTopic(topic)) {
+            return;
+        }
+        boolean tlsEnabled = pulsar.getConfig().isBrokerClientTlsEnabled();
+        if (!tlsEnabled && StringUtils.isBlank(lookupData.getBrokerUrl())) {
+            log.warn("[{}] Unexpected lookup resul: brokerUrlTls is required 
when TLS isn't enabled. options: {},"

Review Comment:
   typo fix
   ```suggestion
               log.warn("[{}] Unexpected lookup result: brokerUrl is required 
when TLS isn't enabled. options: {},"
   ```



-- 
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