void-ptr974 commented on code in PR #26109:
URL: https://github.com/apache/pulsar/pull/26109#discussion_r3492518476
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/ReplicatedSubscriptionsController.java:
##########
@@ -223,13 +226,35 @@ private void
receiveSubscriptionUpdated(ReplicatedSubscriptionsUpdate update) {
.log("Creating subscription at: after receiving update
from replicated subscription");
topic.createSubscription(update.getSubscriptionName(),
InitialPosition.Earliest,
true /* replicateSubscriptionState */,
Collections.emptyMap())
- .thenAccept(subscriptionCreated -> {
-
subscriptionCreated.acknowledgeMessageAsync(Collections.singletonList(pos),
- AckType.Cumulative, Collections.emptyMap());
+ .thenAccept(subscriptionCreated ->
Review Comment:
Non-blocking: this branch is also changed by the PR, but the new test only
covers the case where the subscription already exists. It would be useful to
add a small test where `topic.getSubscription(...)` returns null,
`createSubscription(...)` completes successfully, and the ack future returned
by the newly-created subscription is observed. That would protect the
create-then-ack path from regressing.
--
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]
