Fix #1378 #1380
Fix #1378 #1380
Conversation
MarcialRosales
force-pushed
the
fix-1378
branch
from
e58a8ed to
9cd542f
Compare
MarcialRosales
force-pushed
the
fix-1378
branch
from
9cd542f to
066e223
Compare
| @@ -109,7 +109,7 @@ public sealed class ConnectionFactory : ConnectionFactoryBase, IConnectionFactor | |||
| /// Corresponds to the <code>rabbit.max_message_size</code> setting. | |||
| /// Note: the default is 0 which means "unlimited". | |||
| /// </summary> | |||
| public const uint DefaultMaxMessageSize = 134217728; | |||
| public const uint DefaultMaxMessageSize = 536870912; | |||
There was a problem hiding this comment.
The comments state the the default value is 0, i.e. unlimited. Should we enforce this value?
There was a problem hiding this comment.
Yes, we should use a limited default as of 6.6.0. It's high enough to likely not affect many.
There was a problem hiding this comment.
The default max message size for RabbitMQ main (i.e. 3.13) is the following...
C:\Users\lbakken\development\rabbitmq\rabbitmq-server [rabbitmq-server-9258 ≡]
> git grep max_message_size
deps/rabbit/BUILD.bazel:138: {max_message_size, 134217728},
deps/rabbit/Makefile:120: {max_message_size, 134217728},
deps/rabbit/priv/schema/rabbit.schema:927:{mapping, "max_message_size", "rabbit.max_message_size"
...same with 3.12.x
C:\Users\lbakken\development\rabbitmq\rabbitmq-server_v3.12.x [v3.12.x ≡]
> git grep max_message_size
deps/rabbit/BUILD.bazel:135: {max_message_size, 134217728},
deps/rabbit/Makefile:118: {max_message_size, 134217728},
deps/rabbit/priv/schema/rabbit.schema:927:{mapping, "max_message_size", "rabbit.max_message_size",
@MarcialRosales @michaelklishin why did we decide to use 512MiB in this code?
There was a problem hiding this comment.
@lukebakken I decided it based on this value https://github.com/rabbitmq/rabbitmq-server/blob/main/deps/rabbit_common/include/rabbit.hrl#L257. and also the docs. The docs suggests a default value and a maximum allowed value of 512Mb. But I can change it. Let me know If I should use 134217728 as maximum message size value or keep 512Mb.
| @@ -76,7 +76,7 @@ public AmqpTcpEndpoint(string hostName, int portOrMinusOne, SslOption ssl, uint | |||
| HostName = hostName; | |||
| _port = portOrMinusOne; | |||
| Ssl = ssl; | |||
| _maxMessageSize = maxMessageSize; | |||
| _maxMessageSize = Math.Min(maxMessageSize, ConnectionFactory.DefaultMaxMessageSize); | |||
There was a problem hiding this comment.
Related to the comment above.
This change is not necessary - If we continue to use 0 as the default value, i.e. unlimited, based on the comment.
| @@ -269,5 +269,51 @@ public void TestCreateConnectioUsesValidEndpointWhenMultipleSupplied() | |||
| var ep = new AmqpTcpEndpoint("localhost"); | |||
| using (IConnection conn = cf.CreateConnection(new List<AmqpTcpEndpoint> { invalidEp, ep })) { }; | |||
| } | |||
|
|
|||
| [Fact] | |||
| public void TestCreateConnectionUsesConfiguredMaxMessageSize() | |||
There was a problem hiding this comment.
Great tests, this is exactly what is needed to enforce this variable :)
MaxMessageSizeis not honoured #1378ConnectionFactory.DefaultMaxMessageSizeto today's RabbitMQ max message size.verifiedfile because of the MaxMessageSize changed inConnectionFactory.cs.Types of Changes
What types of changes does your code introduce to this project?
Put an
xin the boxes that apply