Add stomp.max_connections per-node connection limit#16368
Merged
Conversation
stomp.max_connections per-node connection limit
Introduces the stomp.max_connections setting to cap the number of STOMP connections accepted per node, matching the pattern already established by stream.max_connections. The cuttlefish translation uses cuttlefish:unset() when the setting is absent so that application:get_env/3 returns the infinity default without an explicit env entry.
Threads the Ranch listener reference through initial_state/3 (new; /2 is
kept as a wrapper passing undefined so rabbit_web_stomp_handler needs no
change) into #cfg{ranch_ref}. check_node_connection_limit/1 is called as
the first check in process_connect/3, using ranch:info/1 to compare active
connections against the configured limit and returning an ERROR frame when
the limit is exceeded.
Verifies that when stomp.max_connections is set to 0 a connecting client receives a STOMP ERROR frame rather than a CONNECTED frame.
Without this, a failed assertion would leave max_connections set to 0, causing all subsequent test cases that open a connection to fail.
2b3a41e to
b310346
Compare
7 tasks
Collaborator
|
I am extending this to be very similar to the final design of #16367. |
…mqtt.max_connections` behavior
Collaborator
|
Give that the delta in |
This file contains hidden or 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
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.
Closes #16347 (partial - adds
stomp.max_connections).What this PR does
Adds
stomp.max_connections, a per-node connection limit for the plain STOMP plugin, matching the existingstream.max_connectionsandmqtt.max_connectionssettings. When the limit is reached, the broker sends a STOMP ERROR frame and closes the connection.How it works
The enforcement follows the same application-level pattern used by
stream.max_connectionsandrabbit.connection_max- it does not operate at the Ranch transport layer, so the TCP listen queue is unaffected and Ranch continues accepting connections normally.rabbit_stomp_processor:initial_state/2is kept as a one-line wrapper delegating to a newinitial_state/3that accepts the Ranch listener reference, sorabbit_web_stomp_handler(Web STOMP) requires no change and Web STOMP connections are never subject to the plain STOMP plugin limit.Inside
process_connect/3,check_node_connection_limit/1is the first check in themaybechain. It callsranch:info(RanchRef)to getactive_connectionsfor the specific listener and compares it againstapplication:get_env(rabbitmq_stomp, max_connections, infinity).The limit is per-listener: TCP and TLS listeners are enforced independently.
Configuration
stomp.max_connections = 1000The default is
infinity(no limit). The setting accepts a non-negative integer orinfinity.Testing
Adds
node_connection_limittoconnections_SUITE. The test sets the limit to 0 via RPC, sends a CONNECT frame over a raw TCP socket, and asserts that an ERROR frame is received.