-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Use OTP 26 features #8553
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
Use OTP 26 features #8553
Conversation
b8f6d14
to
e411da5
Compare
@ansd should we bump the minimum required version then and delete Erlang 25.3 runs from Actions? |
We'd have to also bump the minimum required version in |
@pjk25 does the PR contain everything needed to bump the minimum OTP to 26? I'm not familiar with what exactly needs to be updated in the Bazel files. It seems we will still require OTP 25 for mixed version tests because the lower version nodes will continue to be OTP 25? |
I think you have covered what is necessary. Mixed version tests use the same erlang for all nodes, so the erlang version must be supported by both. Furthermore, we don't currently differentiate between the erlang version used for compilation, vs the version used to run tests. So, for mixed version tests on this branch, as currently configured, compilation of both versions of the broker, and running of tests all happens on 25.3. Supporting something more complicated will require some further enhancements to the build system. |
Thank you. This means mixed version tests must run on OTP 26. I bumped that version in the latest commit. |
@ansd we can bump to |
Routing to nowhere with the direct exchange is now a bit faster: ``` java -jar target/perf-test.jar -x 2 -y 0 -k route-nowhere -z 60 ``` Remove dead code rabbit_db_binding:route_v2/3
as selective receives are efficient in OTP 26: ``` OTP-18431 Application(s): compiler, stdlib Related Id(s): PR-6739 Improved the selective receive optimization, which can now be enabled for references returned from other functions. This greatly improves the performance of gen_server:send_request/3, gen_server:wait_response/2, and similar functions. ```
Since Erlang/OTP 26: ``` OTP-18445 Application(s): erts, stdlib It is no longer necessary to enable a feature in the runtime system in order to load modules that are using it. It is sufficient to enable the feature in the compiler when compiling it. That means that to use feature maybe_expr in Erlang/OTP 26, it is sufficient to enable it during compilation. In Erlang/OTP 27, feature maybe_expr will be enabled by default, but it will be possible to disable it. ```
and delete Erlang 25.3 runs from Actions
Mixed version tests use the same erlang for all nodes, so the erlang version must be supported by both new and old version.
This test fails when MQTT client ID tracking is performed in Ra, and the higher version node gets compiled with a different OTP version (26) than the lower version node (25). The reason is described in 83eede7 ``` An interesting side note learned here is that the compiled file rabbit_mqtt_collector must not be changed. This commit only modifies function specs. However as soon as the compiled code is changed, this module becomes a new version. The new version causes the anonymous ra query function to fail in mixed clusters: When the old node does a ra:leader_query where the leader is on the new node, the query function fails on the new node with `badfun` because the new node does not have the same module version. For more context, read: https://web.archive.org/web/20181017104411/http://www.javalimit.com/2010/05/passing-funs-to-other-erlang-nodes.html ``` We shouldn’t use an anonymous function for ra:leader_query or ra:consistent_query. Instead we should use the {M,F,A} form. https://github.com/rabbitmq/ra/blob/9e5d437a0a76cc126f396be93645a290e758ac75/src/ra.erl#L102-L103 In MQTT the anonymous function is used in https://github.com/rabbitmq/rabbitmq-server/blob/bcb95c949d8622fb7ba6d3c1b4a12fdb73b0fa66/deps/rabbitmq_mqtt/src/rabbit_mqtt_collector.erl#L50 This causes the query to return a bad fun error (silently ignored in https://github.com/rabbitmq/rabbitmq-server/blob/bcb95c949d8622fb7ba6d3c1b4a12fdb73b0fa66/deps/rabbitmq_mqtt/src/rabbit_mqtt_collector.erl#L70-L71 ) when executed on a different node and either: 1.) Any code in file rabbit_mqtt_collector.erl changed, or 2.) The code gets compiled with a different OTP version. 2.) is the reason for a failing mixed version test in #8553 because both higher and lower versions run OTP 26, but the higher version node got compiled with 26 while the lower version node got compiled with 25. The same file compiled with OTP 26.0.1 ``` 1> rabbit_mqtt_collector:module_info(attributes). [{vsn,[30045739264236496640687548892374951597]}] ``` compiled with OTP 25.3.2 ``` 1> rabbit_mqtt_collector:module_info(attributes). [{vsn,[168144385419873449889532520247510637232]}] ``` Due to the very low impact that maintenance mode will not close all MQTT client connections with feature flag delete_ra_cluster_mqtt_node being disabled, we skip this test.
I revert this PR to a draft as I opened it too early and we ended up mixing multiple issues in this single PR. We should first create a separate PR which only bumps the minimum required version to OTP 26 fixing all tests such as the failing |
This test fails when MQTT client ID tracking is performed in Ra, and the higher version node gets compiled with a different OTP version (26) than the lower version node (25). The reason is described in 83eede7 ``` An interesting side note learned here is that the compiled file rabbit_mqtt_collector must not be changed. This commit only modifies function specs. However as soon as the compiled code is changed, this module becomes a new version. The new version causes the anonymous ra query function to fail in mixed clusters: When the old node does a ra:leader_query where the leader is on the new node, the query function fails on the new node with `badfun` because the new node does not have the same module version. For more context, read: https://web.archive.org/web/20181017104411/http://www.javalimit.com/2010/05/passing-funs-to-other-erlang-nodes.html ``` We shouldn’t use an anonymous function for ra:leader_query or ra:consistent_query. Instead we should use the {M,F,A} form. https://github.com/rabbitmq/ra/blob/9e5d437a0a76cc126f396be93645a290e758ac75/src/ra.erl#L102-L103 In MQTT the anonymous function is used in https://github.com/rabbitmq/rabbitmq-server/blob/bcb95c949d8622fb7ba6d3c1b4a12fdb73b0fa66/deps/rabbitmq_mqtt/src/rabbit_mqtt_collector.erl#L50 This causes the query to return a bad fun error (silently ignored in https://github.com/rabbitmq/rabbitmq-server/blob/bcb95c949d8622fb7ba6d3c1b4a12fdb73b0fa66/deps/rabbitmq_mqtt/src/rabbit_mqtt_collector.erl#L70-L71 ) when executed on a different node and either: 1.) Any code in file rabbit_mqtt_collector.erl changed, or 2.) The code gets compiled with a different OTP version. 2.) is the reason for a failing mixed version test in #8553 because both higher and lower versions run OTP 26, but the higher version node got compiled with 26 while the lower version node got compiled with 25. The same file compiled with OTP 26.0.1 ``` 1> rabbit_mqtt_collector:module_info(attributes). [{vsn,[30045739264236496640687548892374951597]}] ``` compiled with OTP 25.3.2 ``` 1> rabbit_mqtt_collector:module_info(attributes). [{vsn,[168144385419873449889532520247510637232]}] ``` Due to the very low impact that maintenance mode will not close all MQTT client connections with feature flag delete_ra_cluster_mqtt_node being disabled, we skip this test.
See commit messages.
RabbitMQ 3.13 will require OTP 26 as minimum required version.
The OTP 25 GitHub Action tests are expected to fail.