pdolif commented on code in PR #23449:
URL: https://github.com/apache/pulsar/pull/23449#discussion_r1801655763
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/nonpersistent/NonPersistentSubscription.java:
##########
@@ -160,7 +160,29 @@ public synchronized CompletableFuture<Void>
addConsumer(Consumer consumer) {
}
} else {
if (consumer.subType() != dispatcher.getType()) {
- return FutureUtil.failedFuture(new
SubscriptionBusyException("Subscription is of different type"));
+ return FutureUtil.failedFuture(new SubscriptionBusyException(
+ String.format("Subscription is of different type.
Active subscription type of '%s' "
+ + "is different than the connecting
consumer's type '%s'.",
+ dispatcher.getType(), consumer.subType())));
+ } else if (dispatcher.getType() == SubType.Key_Shared) {
+ KeySharedMeta dispatcherKsm =
dispatcher.getConsumers().get(0).getKeySharedMeta();
+ KeySharedMeta consumerKsm = consumer.getKeySharedMeta();
+ if (dispatcherKsm.getKeySharedMode() !=
consumerKsm.getKeySharedMode()) {
+ return FutureUtil.failedFuture(new
SubscriptionBusyException(
+ String.format("Subscription is of different type.
Active subscription key_shared "
+ + "mode of '%s' is different than
the connecting consumer's "
+ + "key_shared mode '%s'.",
+ dispatcherKsm.getKeySharedMode(),
consumerKsm.getKeySharedMode())));
+ }
+ if (dispatcherKsm.isAllowOutOfOrderDelivery() !=
consumerKsm.isAllowOutOfOrderDelivery()) {
+ return FutureUtil.failedFuture(new
SubscriptionBusyException(
+ String.format("Subscription is of different type.
%s",
+ dispatcherKsm.isAllowOutOfOrderDelivery()
+ ? "Active subscription allows out
of order delivery while the "
+ + "connecting consumer does not
allow it." :
+ "Active subscription does not
allow out of order delivery while "
+ + "the connecting consumer
allows it.")));
+ }
}
Review Comment:
That's an excellent idea. I moved it to AbstractSubscription. The return
type is `Optional<CompletableFuture<Void>>` now. What do you think, would it
make sense to add additional tests for the extracted method to
`AbstractSubscriptionTest`?
--
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]
