Denovo1998 commented on PR #26111: URL: https://github.com/apache/pulsar/pull/26111#issuecomment-4843691189
> It seems like the original logic before #26075 is incorrect. `Consumer.topicMigrated` should only get called by this code: > > https://github.com/apache/pulsar/blob/808133dfca01a0b94b7295314778e654613ca322/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java#L2017-L2027 > > One possibility would be to add this method to `Consumer` so that it could be used in `NonPersistentTopic.internalSubscribe` instead of using `checkAndApplyTopicMigrationAsync` > > ```java > public CompletableFuture<Boolean> checkTopicMigrationAsync() { > if (!subscription.isSubscriptionMigrated()) { > return CompletableFuture.completedFuture(false); > } > return AbstractTopic.getMigratedClusterUrlAsync(cnx.getBrokerService().getPulsar(), topicName) > .thenApply(clusterUrl -> { > return clusterUrl.isPresent(); > }); > } > ``` @lhotari Thanks for the suggestion. I updated the PR to keep `Consumer.topicMigrated(...)` applied only from the common `ServerCnx` post-subscribe path. `NonPersistentTopic.internalSubscribe` now uses a new pure `Consumer.checkTopicMigrationAsync()` helper to detect migrated subscriptions and skip `addConsumerToSubscription` without sending the redirect or closing the consumer. The existing `checkAndApplyTopicMigrationAsync()` still owns the actual redirect/close behavior. -- 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]
