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(rebalance): fix start order of rebalance applications #12871

Merged

Conversation

savonarola
Copy link
Contributor

@savonarola savonarola commented Apr 12, 2024

Fixes EMQX-12184

Release version: v/e5.7.0

Summary

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
  • [na] Added property-based tests for code which performs user input validation
  • [na] 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
  • Schema changes are backward compatible

@savonarola savonarola force-pushed the 0412-fix-rebalance-app-dependencies branch from 760a7c6 to 500d4fe Compare April 12, 2024 12:59
@savonarola savonarola marked this pull request as ready for review April 12, 2024 14:35
@savonarola savonarola requested a review from a team as a code owner April 12, 2024 14:35
@zmstone
Copy link
Member

zmstone commented Apr 12, 2024

Thank you for the fix.
I however still don't understand what caused the start order.
If I boot the node with debug level log, I can see agent app is started first.

@savonarola
Copy link
Contributor Author

@zmstone
If we boot 5.6.0, the order is wrong:

[notice] msg: (re)starting_emqx_apps, apps: [emqx_conf,esockd,gproc,emqx_http_lib,bcrypt,ranch,cowboy,esasl,emqx_bridge_kinesis,emqx_utils,emqx_durable_storage,emqx,emqx_modules,emqx_ft,emqx_audit,emqx_retainer,emqx_plugins,emqx_slow_subs,emqx_resource,emqx_bridge_sqlserver,emqx_bridge_opents,emqx_bridge_clickhouse,emqx_bridge_rabbitmq,emqx_redis,emqx_bridge_gcp_pubsub,emqx_bridge_tdengine,emqx_bridge_influxdb,emqx_bridge_timescale,emqx_bridge_http,emqx_management,emqx_opentelemetry,emqx_bridge_es,emqx_bridge_cassandra,emqx_bridge_rocketmq,emqx_bridge_kafka,emqx_bridge_pulsar,emqx_bridge_redis,emqx_bridge_syskeeper,emqx_bridge_s3,emqx_bridge_greptimedb,emqx_bridge_matrix,emqx_bridge_dynamo,emqx_bridge_mqtt,emqx_bridge_confluent,emqx_bridge_hstreamdb,emqx_postgresql,emqx_bridge_pgsql,emqx_exhook,emqx_mongodb,emqx_bridge_mongodb,emqx_oracle,emqx_bridge_oracle,emqx_bridge_azure_event_hub,emqx_auto_subscribe,emqx_bridge_iotdb,emqx_auth,emqx_auth_mongodb,emqx_auth_mnesia,emqx_auth_redis,emqx_auth_postgresql,emqx_prometheus,emqx_gateway,emqx_gateway_coap,emqx_gateway_ocpp,emqx_gateway_lwm2m,emqx_gateway_stomp,emqx_gateway_gbt32960,emqx_gateway_exproto,emqx_gcp_device,emqx_gateway_jt808,emqx_license,emqx_dashboard,emqx_dashboard_rbac,emqx_mysql,emqx_auth_mysql,emqx_bridge_mysql,emqx_connector,emqx_auth_jwt,emqx_auth_http,emqx_ldap,emqx_dashboard_sso,emqx_auth_ldap,emqx_bridge,emqx_rule_engine,emqx_gateway_mqttsn,emqx_node_rebalance,emqx_eviction_agent,quicer,emqx_enterprise,emqx_schema_registry,emqx_psk]

The order was not fixed, so after some app was added, the order produced by topological sort changed.

git diff v5.5.1..e5.6.0 -- apps/emqx_machine/priv/reboot_lists.eterm
diff --git a/apps/emqx_machine/priv/reboot_lists.eterm b/apps/emqx_machine/priv/reboot_lists.eterm
index f7e78c360..a6559bcab 100644
--- a/apps/emqx_machine/priv/reboot_lists.eterm
+++ b/apps/emqx_machine/priv/reboot_lists.eterm
@@ -114,6 +114,7 @@
             emqx_bridge_oracle,
             emqx_bridge_rabbitmq,
             emqx_bridge_azure_event_hub,
+            emqx_bridge_s3,
             emqx_schema_registry,
             emqx_eviction_agent,
             emqx_node_rebalance,

@savonarola savonarola merged commit b7a4536 into emqx:master Apr 19, 2024
169 checks passed
@savonarola savonarola deleted the 0412-fix-rebalance-app-dependencies branch April 19, 2024 10:27
@emqxqa
Copy link

emqxqa commented May 8, 2024

Test Execution

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