Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(bridge/mqtt): respect client ID prefix #13216

Merged

Conversation

zmstone
Copy link
Member

@zmstone zmstone commented Jun 10, 2024

Fixes #13214, #13186, #13185 and EMQX-12525

Release version: v/e5.7.1

Summary

Introduced in 5.4.1 PR: #12236
The client ID shortener did not respect client ID prefix.
After this fix, client ID prefix will be kept:

  • If there is no prefix, EMQX behaves the same way as before (since 5.4.1)
  • If the prefix is 19 bytes or less, EMQX will make use of the rest 4 bytes as shortener's hash space
  • If the prefix is 20 bytes or longer, EMQX will NOT try to keep the client ID within 23 bytes

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:

  • Added tests for the changes
  • Added property-based tests for code which performs user input validation
  • Changed lines covered in coverage report
  • Change log has been added to changes/(ce|ee)/(feat|perf|fix|breaking)-<PR-id>.en.md files
  • For internal contributor: there is a jira ticket to track this change
  • Created PR to emqx-docs if documentation update is required, or link to a follow-up jira ticket
  • Schema changes are backward compatible

Checklist for CI (.github/workflows) changes

  • If changed package build workflow, pass this action (manual trigger)
  • Change log has been added to changes/ dir for user-facing artifacts update

mk_clientid(WorkerId, {Prefix, ClientId}) when ?IS_NO_PREFIX(Prefix) ->
%% When there is no prefix, try to keep the client ID length within 23 bytes
emqx_bridge_mqtt_lib:bytes23(ClientId, WorkerId);
mk_clientid(WorkerId, {Prefix, ClientId}) when size(Prefix) < 20 ->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit. Put 20 behind a -define?

changes/ce/fix-13216.en.md Show resolved Hide resolved
@zmstone zmstone merged commit f911347 into emqx:release-57 Jun 10, 2024
174 checks passed
@zmstone zmstone deleted the 0610-mqtt-bridge-respect-clientid-prefix branch June 10, 2024 12:07
@emqxqa
Copy link

emqxqa commented Jun 10, 2024

TestExecution

Change Details:
- Without Prefix: Behavior remains unchanged; EMQX will hash the entire Client ID into a 23-byte space (when longer than 23 bytes).
- With Prefix:
- Prefix no more than 19 bytes: The prefix is preserved, and the remaining suffix is hashed into a 4-byte space.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be easier to understand if it is changed to:

Prefix no more than 19 bytes: The prefix is preserved, and the remaining suffix is hashed, the total length of the clientid is 23 bytes.

Since when the prefix is "org_", the clientid will be something like "org_de50546ff2da307d11d".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tahnk you. fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants