[fix][broker] Key_Shared subscription: Reject consumers with incompatible policy

[pdolif]

Fixes #23272

Motivation

When a consumer with a different Key_Shared policy connects, the current behavior is that the existing consumer will be closed and replaced with the consumer with the different policy. Since the replaced consumer gets disconnected, it will connect again and swap with the other consumer, and this replacement loop keeps going on.

Taken from #23272 by @lhotari:

When multiple consumers are using different policies, this should be properly handled.
One possibility is to keep the policy of the connected consumers and reject any other consumers and return a proper error message.

Modifications

• Updated PersistentSubscription and NonPersistentSubscription behavior to reject new Key_Shared consumers (added to a Key_Shared subscription) when the new consumer's policy is incompatible with the dispatcher's policy. This is the case if the Key_Shared mode (AUTO_SPLIT/STICKY) or allowOutOfOrderDelivery (true/false) differs.

Verifying this change

• Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

• Added unit test to PersistentSubscriptionTest that verifies that incompatible Key_Shared consumers are rejected by a persistent subscription.
• Added new unit test NonPersistentSubscriptionTest that verifies the same but for nonpersistent subscriptions.

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

• Dependencies (add or upgrade a dependency)
• The public API
• The schema
• The default values of configurations
• The threading model
• The binary protocol
• The REST endpoints
• The admin CLI options
• The metrics
• Anything that affects deployment

Documentation

• doc
• doc-required
• doc-not-needed
• doc-complete

Matching PR in forked repository

PR in forked repository: pdolif#4

[github-actions]

@pdolif Please add the following content to your PR description and select a checkbox:

- [ ] `doc` <!-- Your PR contains doc changes -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->

[lhotari]

LGTM. Great work, @pdolif!

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.29%. Comparing base (bbc6224) to head (6f80b55).
Report is 692 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #23449      +/-   ##
============================================
+ Coverage     73.57%   74.29%   +0.71%     
- Complexity    32624    34904    +2280     
============================================
  Files          1877     1943      +66     
  Lines        139502   146995    +7493     
  Branches      15299    16195     +896     
============================================
+ Hits         102638   109206    +6568     
- Misses        28908    29345     +437     
- Partials       7956     8444     +488     

Flag	Coverage Δ
inttests	27.28% <23.07%> (+2.70%)	⬆️
systests	24.35% <23.07%> (+0.02%)	⬆️
unittests	73.66% <100.00%> (+0.81%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...he/pulsar/broker/service/AbstractSubscription.java	100.00% <100.00%> (ø)
...rvice/nonpersistent/NonPersistentSubscription.java	55.55% <100.00%> (+2.28%)	⬆️
...ker/service/persistent/PersistentSubscription.java	77.28% <100.00%> (+0.59%)	⬆️

... and 654 files with indirect coverage changes

Dismissing review since improving the error messages could be useful.

[lhotari]

LGTM, Good work!

[lhotari]

there's some unrelated test flakiness such as #23486 which caused the first attempt to fail.

[lhotari]

Great work @pdolif, thanks for contributing!