fix(shared_sub): restore support for $queue/ shared subscription
#11281
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Partially addresses: https://emqx.atlassian.net/browse/EMQX-4589 There's still a problem with the handling of shared groups that is _not particular to the `$queue/` syntax_ and preexistent that the same client cannot subscribe to _different groups_ that have the same _topic filter_.
thalesmg
force-pushed
the
restore-queue-shared-sub-r51-20230717
branch
from
1e7e080
to
1fdcfba
Compare
|
The issue about unsubscribing to shared topics and subscribing to different groups/same topic filters is tracked by https://emqx.atlassian.net/browse/EMQX-9876, and there's some progress here: #10976 |
keynslug
reviewed
Jul 18, 2023
Comment on lines
191
to
193
| emqx_topic:join([<<"$share">>, Group, Topic]); | ||
| filename:join([<<"$share">>, Group, Topic]); |
There was a problem hiding this comment.
This is probably not what we want, i.e. emqx_topic:join/1 is the proper way to do things here. Otherwise, the results will be different between Linux and Windows for example.
ieQu1
approved these changes
Jul 18, 2023
paulozulato
approved these changes
Jul 18, 2023
| @@ -986,6 +988,112 @@ t_session_kicked(Config) when is_list(Config) -> | |||
| ?assertEqual([], collect_msgs(0)), | |||
| ok. | |||
|
|
|||
| %% FIXME: currently doesn't work | |||
There was a problem hiding this comment.
nit: maybe add why it's not working (or what it needs to work)?
There was a problem hiding this comment.
There are a few comments in other places, and the PR number can be referenced from the merge commit, I guess.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
targeting
release-51Partially addresses: https://emqx.atlassian.net/browse/EMQX-4589
There's still a problem with the handling of shared groups that is not particular to the
$queue/syntax and preexistent that the same client cannot subscribe to different groups that have the same topic filter.Summary
馃 Generated by Copilot at 1e7e080
This pull request implements the queue subscription feature, which allows clients to subscribe to a topic as a queue group and receive messages in a load-balanced way. It modifies the
emqx.hrl,emqx_topic.erl, andemqx_mgmt_api_subscriptions.erlfiles to handle the new$queuetopic prefix and the related options. It also updates and adds test cases inemqx_shared_sub_SUITE.erl,emqx_topic_SUITE.erl, andemqx_proper_types.erlto test the new feature and ensure its correctness.PR Checklist
Please convert it to a draft if any of the following conditions are not met. Reviewers may skip over until all the items are checked:
changes/(ce|ee)/(feat|perf|fix)-<PR-id>.en.mdfilesChecklist for CI (.github/workflows) changes
changes/dir for user-facing artifacts update