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(machine_boot): use shared list of reboot apps and add bridges to reboot list #11309

Merged

Conversation

thalesmg
Copy link
Contributor

@thalesmg thalesmg commented Jul 19, 2023

Ensures all business applications are restarted when the node joins a cluster.

Also, unifies the apps lists in a single place to avoid repetition in mix.exs and rebar.config.erl.

iex(emqx@127.0.0.1)1> :emqx_machine_boot.sorted_reboot_apps
[:emqx_conf, :esockd, :gproc, :emqx_redis, :ranch, :cowboy, :emqx,
 :emqx_resource, :emqx_retainer, :emqx_slow_subs, :emqx_plugins, :emqx_modules,
 :emqx_ft, :emqx_exhook, :emqx_connector, :emqx_bridge, :emqx_bridge_mqtt,
 :emqx_bridge_kafka, :emqx_bridge_pulsar, :emqx_bridge_cassandra,
 :emqx_bridge_opents, :emqx_bridge_clickhouse, :emqx_bridge_dynamo,
 :emqx_bridge_hstreamdb, :emqx_bridge_influxdb, :emqx_bridge_matrix,
 :emqx_bridge_pgsql, :emqx_bridge_rocketmq, :emqx_bridge_tdengine,
 :emqx_bridge_timescale, :emqx_bridge_sqlserver, :emqx_bridge_rabbitmq,
 :emqx_bridge_http, :emqx_management, :emqx_bridge_gcp_pubsub,
 :emqx_bridge_redis, :emqx_mongodb, :emqx_bridge_mongodb, :emqx_dashboard,
 :emqx_oracle, :emqx_bridge_oracle, :emqx_auto_subscribe, :emqx_bridge_iotdb,
 :emqx_prometheus, :emqx_mysql, :emqx_authn, :emqx_gateway, :emqx_gateway_stomp,
 :emqx_gateway_coap, :emqx_gateway_lwm2m, ...]
e5.1.1-alpha.2-g3c679880(emqx@127.0.0.1)1> emqx_machine_boot:sorted_reboot_apps().
[emqx_conf,esockd,gproc,emqx_redis,ranch,cowboy,emqx,
 emqx_resource,emqx_retainer,emqx_slow_subs,emqx_plugins,
 emqx_modules,emqx_ft,emqx_exhook,emqx_connector,emqx_bridge,
 emqx_bridge_mqtt,emqx_bridge_kafka,emqx_bridge_pulsar,
 emqx_bridge_cassandra,emqx_bridge_opents,
 emqx_bridge_clickhouse,emqx_bridge_dynamo,
 emqx_bridge_hstreamdb,emqx_bridge_influxdb,
 emqx_bridge_matrix,emqx_bridge_pgsql,emqx_bridge_rocketmq,
 emqx_bridge_tdengine|...]

Summary

🤖 Generated by Copilot at 3191dd0

This pull request improves the application rebooting mechanism of the emqx_machine project by using a configurable eterm file instead of a fixed list. It also updates the mix.exs, rebar.config.erl and Makefile files to support the new mechanism and enhance the project compatibility and flexibility.

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
  • Changed lines covered in coverage report
  • Change log has been added to changes/(ce|ee)/(feat|perf|fix)-<PR-id>.en.md files
  • For internal contributor: there is a jira ticket to track this change
  • If there should be document changes, a PR to emqx-docs.git is sent, or a jira ticket is created to follow up
  • 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

@thalesmg thalesmg force-pushed the fix-machine-boot-reboot-r51-20230719 branch 2 times, most recently from 3c67988 to 0351c13 Compare July 19, 2023 20:25
BusinessApps = CommonBusinessApps ++ EditionSpecificApps,
?BASIC_REBOOT_APPS ++ BusinessApps.

filter(AppList, Filters) ->
Copy link
Member

Choose a reason for hiding this comment

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

We really need a ./configure step for the build...

@thalesmg thalesmg marked this pull request as ready for review July 19, 2023 21:07
@thalesmg thalesmg requested a review from a team as a code owner July 19, 2023 21:07
rebar.config.erl Outdated Show resolved Hide resolved
@thalesmg thalesmg force-pushed the fix-machine-boot-reboot-r51-20230719 branch from 0351c13 to b9b11d8 Compare July 19, 2023 23:16
observer
],
%% must always be of type `load'
ee_business_apps =>
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should split ee_business_apps to a different apps' prive dir. apps/emqx_enteriprise/priv/
In order to don't include enterprise in community version.

Copy link
Member

@ieQu1 ieQu1 Jul 20, 2023

Choose a reason for hiding this comment

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

emqx_machine_boot:basic_reboot_apps() on ce already mentions some ee apps on master.
But I agree that we can further improve it later.

ieQu1
ieQu1 previously approved these changes Jul 20, 2023
@thalesmg
Copy link
Contributor Author

I just noticed that starting a node might result in some noisy logs when there are existing bridges because their dependencies (e.g.: brod) haven't started up yet... I'll have to patch that as well.

ieQu1
ieQu1 previously approved these changes Jul 20, 2023
zhongwencool
zhongwencool previously approved these changes Jul 20, 2023
@thalesmg thalesmg dismissed stale reviews from zhongwencool and ieQu1 via 493f1e5 July 20, 2023 15:58
@thalesmg thalesmg force-pushed the fix-machine-boot-reboot-r51-20230719 branch from 8b81d48 to 493f1e5 Compare July 20, 2023 15:58
We need to reverse the dependency of `emqx_bridge` and `emqx_bridge_*`, because the former
loads and starts bridges during its application startup.  If the individual bridge
application being loaded has not started with its dependencies, the supervision tree will
not be ready for that.
@thalesmg thalesmg force-pushed the fix-machine-boot-reboot-r51-20230719 branch from 493f1e5 to 6cd5038 Compare July 20, 2023 16:11
@thalesmg thalesmg merged commit 54efc04 into emqx:release-51 Jul 20, 2023
117 checks passed
@thalesmg thalesmg deleted the fix-machine-boot-reboot-r51-20230719 branch July 20, 2023 17:33
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.

None yet

3 participants