BewareMyPower commented on code in PR #24658:
URL: https://github.com/apache/pulsar/pull/24658#discussion_r2296582885


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/SystemTopicBasedTopicPoliciesService.java:
##########
@@ -473,15 +478,16 @@ public boolean test(NamespaceBundle namespaceBundle) {
     private void initPolicesCache(SystemTopicClient.Reader<PulsarEvent> 
reader, CompletableFuture<Void> future) {
         if (closed.get()) {
             future.completeExceptionally(new 
BrokerServiceException(getClass().getName() + " is closed."));
-            
cleanCacheAndCloseReader(reader.getSystemTopic().getTopicName().getNamespaceObject(),
 false);
+            
cleanCache(reader.getSystemTopic().getTopicName().getNamespaceObject(), false, 
true);
             return;
         }
         reader.hasMoreEventsAsync().whenComplete((hasMore, ex) -> {
             if (ex != null) {
                 log.error("[{}] Failed to check the move events for the system 
topic",
                         reader.getSystemTopic().getTopicName(), ex);
                 future.completeExceptionally(ex);
-                
cleanCacheAndCloseReader(reader.getSystemTopic().getTopicName().getNamespaceObject(),
 false);
+                
cleanCache(reader.getSystemTopic().getTopicName().getNamespaceObject(), false,
+                        isAlreadyClosedException(ex));

Review Comment:
   > Thousands of get last message id failed was thrown due to No such ledger 
exception.
   
   This should fail the `hasMoreEventsAsync` method in `initPolicesCache`. 
After the change here, the reader won't be removed from the cache. Could you 
explain how it fixes the issue you encountered?
   
   If the failed reader was kept in cache after that, next time 
`hasMoreEventsAsync` should fail again because `No such ledger` should not be a 
temporary error. If this issue is not resolved, keeping the failed reader in 
cache could only save a reader creation but would still face the 
`hasMoreEventsAsync` error and GetLastMessageId RPC failure.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to