Re: [PR] [fix][client] Schedule TableView tail-read retries instead of sleeping on the callback thread [pulsar]

["namest504 (via GitHub)" <gi...@apache.org>]

namest504 commented on code in PR #26672:
URL: https://github.com/apache/pulsar/pull/26672#discussion_r4162665317


##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/AbstractTableViewImpl.java:
##########


Review Comment:
   You're right, the stop only reached refreshes that were already registered. aa1483f tracks a refresh from the moment refreshAsync() is called, so a stop fails it while it is still looking up the last message ids, and a refresh started after the stop fails before any lookup, empty topic included. Your test is in for close, a closed reader and a rejected read; the lookup future stays incomplete in all three.
   
   While testing this against a broker I found one more hole. A retry already queued on the client's scheduler is dropped silently when that scheduler shuts down; only a schedule() call made afterwards is rejected. Nothing is reading while a retry waits, so the loop ended without a stop cause and a waiting refresh stayed pending. I reproduced it by closing the client during a 6 s retry delay.
   
   d6c30f3 moves the delay to CompletableFuture.delayedExecutor, like ScalableTopicProducer does, so the retry still runs and finds the closed reader. Its read now starts on a common-pool thread. The rejected-schedule branch is gone; two rules replace it: a read rejected because the reader's executor was shut down stops the loop, and a failure is not retried once the client is closed.
   



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