Revert erlang:port_command/2 to gen_tcp:send/2 change #7921
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
Revert the change introduced in #7900 RabbitMQ 3.12 will require OTP 25 (not yet OTP 26). The use of macro `OTP_RELEASE` was wrong because RabbitMQ can be compiled with OTP 25, but run with OTP 26. Macros are evaluated at compile time. An alternative runtime equivalent would have been ``` 1> erlang:system_info(otp_release). "26" ```
ansd
added a commit
that referenced
this pull request
Apr 18, 2023
Revert erlang:port_command/2 to gen_tcp:send/2 change (backport #7921)
|
Since we seemingly cannot reproduce the original issue (a slow scan of process mailbox in |
ansd
added a commit
that referenced
this pull request
Apr 18, 2023
Follow up of #7913 and #7921 This commit uses the approach explained in erlang/otp#7130 (comment) We cannot use the macro `?OTP_RELEASE` since macros are evaluated at compile time. RabbitMQ can be compiled with OTP 25 and executed with OTP 26. Therefore, we use `erlang:system_info(otp_release)` instead. As `erlang:system_info/1` might be costly, we store the send function in persistent_term. For OTP 25, we use the "old tcp send workaround" (i.e. `erlang:port_command/2`) which avoids expensive selective receives. For OTP 26, we use `gen_tcp:send/2` which uses the optimised selective receive. Once the minimum required version becomes OTP 26, we can just switch to `gen_tcp:send/2` and delete the `inet_reply` handling code in the various RabbitMQ reader and writer processes. Note that `rabbit_net:port_command/2` is not only used by RabbitMQ server, but also by the AMQP 0.9.1 client. Therefore, instead of putting the OTP version (or send function) into persistent_term within the rabbit app, we just do it the first time `rabbit_net:port_command/2` is invoked. (`rabbit_common` is just a library without supervision hierarchy.)
mergify bot
pushed a commit
that referenced
this pull request
Apr 24, 2023
Follow up of #7913 and #7921 This commit uses the approach explained in erlang/otp#7130 (comment) We cannot use the macro `?OTP_RELEASE` since macros are evaluated at compile time. RabbitMQ can be compiled with OTP 25 and executed with OTP 26. Therefore, we use `erlang:system_info(otp_release)` instead. As `erlang:system_info/1` might be costly, we store the send function in persistent_term. For OTP 25, we use the "old tcp send workaround" (i.e. `erlang:port_command/2`) which avoids expensive selective receives. For OTP 26, we use `gen_tcp:send/2` which uses the optimised selective receive. Once the minimum required version becomes OTP 26, we can just switch to `gen_tcp:send/2` and delete the `inet_reply` handling code in the various RabbitMQ reader and writer processes. Note that `rabbit_net:port_command/2` is not only used by RabbitMQ server, but also by the AMQP 0.9.1 client. Therefore, instead of putting the OTP version (or send function) into persistent_term within the rabbit app, we just do it the first time `rabbit_net:port_command/2` is invoked. (`rabbit_common` is just a library without supervision hierarchy.) (cherry picked from commit 6b4d881)
michaelklishin
pushed a commit
that referenced
this pull request
May 28, 2023
Follow up of #7913 and #7921 This commit uses the approach explained in erlang/otp#7130 (comment) We cannot use the macro `?OTP_RELEASE` since macros are evaluated at compile time. RabbitMQ can be compiled with OTP 25 and executed with OTP 26. Therefore, we use `erlang:system_info(otp_release)` instead. As `erlang:system_info/1` might be costly, we store the send function in persistent_term. For OTP 25, we use the "old tcp send workaround" (i.e. `erlang:port_command/2`) which avoids expensive selective receives. For OTP 26, we use `gen_tcp:send/2` which uses the optimised selective receive. Once the minimum required version becomes OTP 26, we can just switch to `gen_tcp:send/2` and delete the `inet_reply` handling code in the various RabbitMQ reader and writer processes. Note that `rabbit_net:port_command/2` is not only used by RabbitMQ server, but also by the AMQP 0.9.1 client. Therefore, instead of putting the OTP version (or send function) into persistent_term within the rabbit app, we just do it the first time `rabbit_net:port_command/2` is invoked. (`rabbit_common` is just a library without supervision hierarchy.) (cherry picked from commit 6b4d881)
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.
Revert the change introduced in #7900
RabbitMQ 3.12 will require OTP 25 (not yet OTP 26).
The use of macro
OTP_RELEASEwas wrong because RabbitMQ can be compiled with OTP 25, but run with OTP 26. Macros are evaluated at compile time.An alternative runtime equivalent would have been