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

feat(mqtt): enable dialer pool for everyone #4565

Merged
merged 1 commit into from
Mar 16, 2022

Conversation

onelson
Copy link
Contributor

@onelson onelson commented Mar 16, 2022

Previously we had implemented a way to allow flux to reuse client
connections to mqtt across calls to mqtt.publish, but it was hidden
behind a feature flag pending verification of the new behavior.

After some manual testing, things appear to be in good shape. By
inspecting a mosquitto mqtt broker's server log, and toggling the flag
off/on, we could see that when the feature is enabled there's a single
client connection and serveral publishes (versus one connection per
publish when the flag is off).

This diff removes the feature flag, turning the new code path on for
everyone.

Done checklist

  • docs/SPEC.md updated
  • Test cases written

Previously we had implemented a way to allow flux to reuse client
connections to mqtt across calls to `mqtt.publish`, but it was hidden
behind a feature flag pending verification of the new behavior.

After some manual testing, things appear to be in good shape. By
inspecting a mosquitto mqtt broker's server log, and toggling the flag
off/on, we could see that when the feature is enabled there's a single
client connection and serveral publishes (versus one connection per
publish when the flag is off).

This diff removes the feature flag, turning the new code path on for
everyone.
@onelson onelson requested a review from a team as a code owner March 16, 2022 15:24
@onelson onelson requested review from rockstar and removed request for a team March 16, 2022 15:24
@nathanielc
Copy link
Contributor

@samhld Once this merges and with the next Flux release(next week) the mqtt work will be live for everyone

@onelson onelson merged commit dc08c57 into master Mar 16, 2022
@onelson onelson deleted the onelson/feat/enable-mqtt-conn-pooling branch March 16, 2022 19:52
jsternberg added a commit that referenced this pull request Mar 22, 2022
jsternberg added a commit that referenced this pull request Mar 22, 2022
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.

3 participants