rabbitmq / rabbitmq-dotnet-client

RabbitMQ .NET client for .NET Standard 2.0+ and .NET 4.6.2+
https://www.rabbitmq.com/dotnet.html
Other
2.07k stars 579 forks source link

Removed ReceiveBufferSize and SendBufferSize to improve message rates #1414

Closed MarcoZuiderwijkBS closed 10 months ago

MarcoZuiderwijkBS commented 10 months ago

Proposed Changes

When connecting our application running in the Western Europe region to a RabbitMQ instance in the East-US region we got a very low message rate (12 messages per second). Message size is around 80kb. We also tested this with a Python (using pika 1.3.2) and a Java consumer (using amqp-client 5.16.0) which had a much higher message rate (> 500 messages per second). We also did a test with RabbitMQ version 5.2.0 and to our surprise this also had a much higher message rate.

My colleague Joey Bleeker investigated the differences between version 5.2.0 and 6.6.0 of the RabbitMQ Client and found this commit. We removed the setting for the ReceiveBufferSize and SendBufferSize. This gives the latest RabbitMQ.Client the same performance as the version 5.2.0. The previous changes seems to conflict with the TCP Window Scaling.

Types of Changes

What types of changes does your code introduce to this project? Put an x in the boxes that apply

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask on the mailing list. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

Further Comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc.

michaelklishin commented 10 months ago

@MarcoZuiderwijkBS thank you for digging in.

As https://github.com/rabbitmq/rabbitmq-dotnet-client/commit/ff627be358269985da306ca0e76a0e7a962b4366 mentions, some workload may benefit from these settings, while your workload with messages much larger than average suffers.

Sounds like it should be a configurable settings and not something we flip back and forth.

At least several other clients allow you to "post-configure" a socket before it connects using a lambda. I'd suggest something like that, with the default being "no configuration besides Nagle's algorithm".

lukebakken commented 10 months ago

Converting to draft just to ensure this isn't merged prematurely.

lukebakken commented 10 months ago

We should adopt what the Java client does to configure the underlying Socket:

https://github.com/rabbitmq/rabbitmq-java-client/blob/main/src/main/java/com/rabbitmq/client/SocketConfigurator.java

That way, if someone wants to adjust buffer sizes, they can. The only thing we should do is disable Nagle's algo.

cc @stebet on this just so he's in the loop.

lukebakken commented 10 months ago

@MarcoZuiderwijkBS I've merged your code into https://github.com/rabbitmq/rabbitmq-dotnet-client/pull/1415. Since your PR is on the main branch I can't easily make modifications to it.

pivotal-cla commented 10 months ago

@MarcoZuiderwijkBS Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

lukebakken commented 10 months ago

@MarcoZuiderwijkBS https://www.nuget.org/packages/RabbitMQ.Client/6.7.0-rc.1

pivotal-cla commented 10 months ago

@MarcoZuiderwijkBS Thank you for signing the Contributor License Agreement!

pivotal-cla commented 10 months ago

@MarcoZuiderwijkBS Thank you for signing the Contributor License Agreement!