Setting MoreCreditAfter larger than InitialCredit may cause unrecoverable flow control

[JimmyWang6]

RabbitMQ series

4.0.x

Operating system (distribution) used

unbuntu 22.0.4

How is RabbitMQ deployed?

Generic binary package

What would you like to suggest for a future version of RabbitMQ?

Describe the bug

In our production environment, we set credit_flow_default_credit to {200, 400}. At the beginning, everything seemed well and nothing unusual happened. However, after the client was restarted, new connections entered a flow status and couldn't recover because the credit_flow_default_credit configuration had started to take effect.

Reproduction steps

1. Set up RabbitMQ with default configuration
2. Set credit_flow_default_credit to {200, 400} （MoreCretidAfter larger than InitialCredit）
3. Initiate a new connection to the broker.

Expected behavior

Flow control as usual

Additional context

Could we incorporate additional parameter validation to guarantee that `InitialCredit` is not set to a value less than `MoreCreditAfter` ? I will submit a PR to enhance validation later.

[michaelklishin]

@JimmyWang6 credit_flow_default_credit is not a setting we expect 99% of users to ever modify. That's why you can only set it via advanced.config and the docs very rarely mention it.

MoreCreditAfter is not tied to the initial value. You can use equal values and achieve whatever you are trying to do.

after the client was restarted, new connections entered a flow status and couldn't recover because
the credit_flow_default_credit configuration had started to take effect

You are not backing this claim with any evidence. How did you observe that the client "has entered an unrecoverable flow"?

I will wager a guess: you look at the management UI and see the connection reported as flow in yellow every time. As long as your publisher is making progress, this is a perfectly fine state of things because the metrics used by the UI are emitted every 5 seconds by default, and usually no more frequently than that.

Transient flow control can go in and out of effect many times, in fact, a hundred times a second. Which is why after trying to add logging for it twice, we have concluded that there is no practical way of doing that because both the logger will be overwhelmed, and the throughput will inevitably be affected.

The way MoreCreditAfter is used is not at all complicated. credit_flow (a module in RabbitMQ) simply grants that many credits to the sender process every so often, regardless of the initial value.

I am quite certain that there is no issue at all, you simply haven't loaded the connection/node enough before to see this "permanent" transient flow control in the UI, which is not permanent in the sense that the publisher is making constant progress at its peak capacity.

Which, by the way, you don't be able to significantly boost with the credit flow settings. It is
an idea as old as RabbitMQ but it does not consider the fact that a publisher will always do less work than the rest of RabbitMQ — in particular with replicated queue types — and thus will always be blocked and unblocked N times a second under load, but the management UI won't show you all of those events, as explained above.

If you need to boost per-connection throughput significantly, the answer is not to mess with credit flow. The answer is using streams or superstreams with their native protocol clients, some of which can sustain throughput of up to 1M messages per second per connection for as long as your disks and network links can keep up.

[michaelklishin]

@JimmyWang6 if you publish messages in a tight loop to a queue (and not a stream or superstream), you will sooner or later hit a resource alarm which can last 1 minute, 2 minutes or hours depending on when consumers come online.

Quorum queues and classic queues v2 are better are delaying such a scenario but they likely won't avoid it.

[michaelklishin]

@JimmyWang6 by definition, flow control and resource alarms exist so that a fast publisher could not "increase its throughput" and drive the node out of resources (both memory and disk).

[michaelklishin]

To simulate such workloads, it should be enough to use PerfTest and Stream PerfTest, with a predefined topology or not.

[JimmyWang6]

@michaelklishin I have searched the logs for all nodes in the cluster and confirmed that there are no resource alarm. While using PerfTest in such case with command
./runjava com.rabbitmq.perf.PerfTest -h amqp://root:[email protected] -x 1 -y 1, the comand get stuck and there is no any output for more than 5 minutes.

After modifying credit_flow_default_credit value back to {400, 200}, the command could run well.

The only difference is the change of credit_flow_default_credit.

[mkuratczyk]

I've seen in in the past when messing with credit settings and I can reproduce this on main right now:

perf-test -y 0 -c 1 -qq -u qq

gives me ~4000 msg/s published and perf-test keeps going as expected.

Then, if I do persistent_term:put(credit_flow_default_credit, {200,400}). in the Erlang shell and re-run the command:

> perf-test -y 0 -c 1 -qq -u qq
id: test-082231-804, starting producer #0
id: test-082231-804, starting producer #0, channel #0
time               sent    confirmed       nacked        confirm latency
1.006s        200 msg/s    199 msg/s      0 msg/s           0/0/1/1/1 ms

and that's it - perf-test just hangs at this point.

The queue type doesn't matter for this. I get the same results with a classic queue, quorum queue and a stream.

[michaelklishin]

Could we incorporate additional parameter validation to guarantee that InitialCredit is not set to > a value less than MoreCreditAfter ? I will submit a PR to enhance validation later

We can but I don't see why we would. Tweaking and bumping credit flow is usually the wrong thing to do. Such validation will not serve any purpose.

In a small number of cases where bumping credit flow makes a reasonable difference, it always was

1. Consumer throughput, not publisher throughput, because publishers inherently outpace the rest of the system and will be throttled with a certain frequency
2. Credit flow between classic queues and consumers/connection writer process

Again, people have been trying to bump credit flow as early as 2009 or so, and it very rarely makes a big difference without causing memory footprint swings that.

[ansd]

The default setting is {credit_flow_default_credit, {400, 200}} and has the form {InitialCredit, MoreCreditAfter}.

To provide an example: The reader proc sends messages to the channel proc. The reader proc has initially 400 credits. After the channel proc received the first 200 messages from the reader proc, the channel proc grants another 200 credits to the reader proc.

If you now flip {InitialCredit, MoreCreditAfter} as you did in this issue, it means that the reader proc has only 200 credits initially and that the channel will grant new credits only after the channel receives 400 messages from the reader. However, this will never happen because the reader blocks after having sent 200 messages to the channel. This is also demonstrated in #13041 (reply in thread).

Hence, setting InitialCredit to be smaller than MoreCreditAfter is simply a misconfiguration.

Now of course we can discuss whether RabbitMQ should refuse to boot if it notices this misconfiguration, and in my opinion we can add a check to prevent users from this misconfiguration.

[JimmyWang6]

@ansd
Thank you for your reply. That's exactly what I thought. Perhaps most of the people using RabbitMQ may don't have such deep understanding of how flow control works, and perhaps may make the same mistake.
My idea is to add a check in function persist_static_configuration, If this parameter is set incorrectly, change it to the default value.

[ansd]

Right. Or the node could even refuse to boot, if misconfigured.

[ansd]

... or change to defaults and print a warning

[michaelklishin]

We already emit warnings with a few settings that can be misconfigured, although some of them were likely deleted with CQv1 in 4.x.

However, if a misconfiguration leads to serious consequences, I would not object to a PR where nodes would refuse to boot, log and error and voluntarily stop).