feat(mqttconn): employ ecpool instead of single worker #10754
Merged
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
|
Hi @keynslug |
keynslug
changed the title
|
Hi @zmstone, not yet I'm afraid. Was invested into this |
keynslug
force-pushed
the
fix/EEC-856/mqtt
branch
7 times, most recently
from
3dfc950
to
6cd0e38
Compare
Also drop fiddling with `mountpoint` since this option seems not to be used anywhere.
Inline `emqx_connector_mqtt_msg` module code into `emqx_connector_mqtt_worker` module, since it's not really used anywhere else and does not provide any reusable abstractions.
Also move this logic to the mqtt connector itself, in order to avoid dealing with extra callback layer.
Also rename `subscriptions` -> `ingress` and `forwards` -> `egress` for consistency with the config schema.
keynslug
force-pushed
the
fix/EEC-856/mqtt
branch
from
6cd0e38
to
f96891b
Compare
zmstone
reviewed
May 30, 2023
It adds no value: the only mode was `cluster_shareload` and we just as well can decide to "share" the load across cluster just by looking if the remote topic is shared subcription filter or not.
That currently tunes the number of MQTT clients employed both for subscriptions (if shared subscription is used) and for publishing to a remote broker.
Each with a more refined set of responsibilities, at the cost of slight code duplication. Also provide two different config fields for each pool size.
With fixed typings and empty pool handling.
* emqx_connector 0.1.25 * emqx_rule_engine 5.0.19 * emqx_ee_bridge 0.1.15
keynslug
force-pushed
the
fix/EEC-856/mqtt
branch
from
2ac25bf
to
d738897
Compare
keynslug
force-pushed
the
fix/EEC-856/mqtt
branch
from
d738897
to
95cc9b9
Compare
zmstone
reviewed
May 30, 2023
keynslug
force-pushed
the
fix/EEC-856/mqtt
branch
from
ab59147
to
1783b1b
Compare
|
Need a changelog for this? |
Co-authored-by: Zaiming (Stone) Shi <zmstone@gmail.com>
keynslug
force-pushed
the
fix/EEC-856/mqtt
branch
from
1783b1b
to
42a4c02
Compare
zmstone
approved these changes
May 30, 2023
Enhancements
|
增强
|
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.
Part of EEC-856
Fixes EMQX-10056
This PR adds a ecpool of workers to the MQTT connector. This should alleviate the current bottleneck of using single
emqttworker.TODOs / Followups.
pool_size/reconnect_interval).pool_sizetimes.Summary
🤖 Generated by Copilot at ae3ef14
Refactored the
emqx_connector_mqttmodule and its worker to improve code quality and consistency. Moved schema validation and parsing to a separate module. Deleted the unusedemqx_connector_mqtt_msgmodule. Updated the test suite to reflect the changes.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 changesChange log has been added tochanges/{ce,ee}/(feat|perf|fix)-<PR-id>.en.mdfilesIf there should be document changes, a PR to emqx-docs.git is sent, or a jira ticket is created to follow up