(Initial, incomplete) OTP26 compatibility #7900
Merged
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
OTP-26 changed the default version for binary_to_term from 1 to 2. There's one place where we explicitly ask for version 1 anyway (in the STOMP plugin) and seems like we need to keep it like this.
These algorithms, introduced in OTP-26, are not compatible with what we do in this test.
Fix for OTP-26+. We don't care about the security of the TLS connection in that test.
We don't care about the security of the TLS connection in that test.
While at it, refactor `rabbit_misc:plmerge/2` to use the same precedence as maps:merge and lists:merge (the second argument supersedes the first one)
This no longer works with OTP-26 and will not work moving forward. So we do like ssl and simulate a real port command.
An inadvertent dependency on small map atom key ordering had been taken inside the stream election code in the stream coordinator state machine. With the key ordering changes in OTP 26 this resulted in divergent state in different stream coordinator members which resulted in stream recovery failing.
The test was also not testing what it claimed to test (it was using verify_none so not sending the client certificates). This commit fixed that as well.
OTP-26 sets verify_peer by default so the warning is no longer relevant. The function can be removed when we support OTP-26+ only.
michaelklishin
changed the title
mkuratczyk
force-pushed
the
otp26-compatibility
branch
from
99e8d6d
to
69f48a0
Compare
ansd
previously requested changes
Apr 17, 2023
Similarly to #7663, increase the message size and decrease the client buffer sizes. This change is needed because we switched from erlang:port_command/2 to gen_tcp:send/2. The former is a bit more asynchronous than the latter because the latter waits for the inet_reply from the port.
In OTP 25, gen_tcp:send/2 has poor performance when the Erlang mailbox is large because the selective receive is not optimised. https://erlang.org/download/otp_src_26.0-rc3.readme OTP-18520 Application(s): erts Related Id(s): GH-6455 gen_tcp:send/*, gen_udp:send/* and gen_sctp:send/* have been optimized to use the infamous receive reference optimization, so now sending should not have bad performance when the calling process has a large message queue.
michaelklishin
dismissed
ansd’s stale review
David has addressed his own review
michaelklishin
added a commit
that referenced
this pull request
Apr 18, 2023
(Initial, incomplete) OTP26 compatibility (backport #7900)
ansd
added a commit
that referenced
this pull request
Apr 18, 2023
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" ```
mergify bot
pushed a commit
that referenced
this pull request
Apr 18, 2023
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" ``` (cherry picked from commit 81d21f5)
8 tasks
|
@Mergifyio backport v3.11.x |
✅ Backports have been created
|
|
Backporting this to |
michaelklishin
pushed a commit
that referenced
this pull request
May 28, 2023
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" ``` (cherry picked from commit 81d21f5)
|
I have backported all commits from this PR except for c8b4742. The delta in stream coordinator code is non-trivial. However, with these improvements nodes can accept connections and generally work fine (all Bunny tests pass, for example). So this is a sufficient improvement to ship, right now RabbitMQ 3.11.16 on Erlang 26 cannot accept client connections, which makes us look really bad to the Windows users who download the latest Erlang installer. |
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.
NOTE: do not merge yet (PR created to run tests on OTP25)
WIP: changes needed for OTP26 compatibility. This doesn't address all the issues at this point