Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto widen session incoming-window in AMQP 1.0 client (backport #13574) (backport #13576) #13578

Merged
merged 3 commits into from
Mar 20, 2025

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented Mar 19, 2025

This commit fixes a bug in the Erlang AMQP 1.0 client.

Prior to this commit, to repro this bug:

  1. Send more than 2^16 messages to a queue.
  2. Grant more than a total of 2^16 link credit initially (on a single link
    or across multiple links) on a single session without any
    auto or manual link credit renewal.

The expectation is that thanks to sufficiently granted initial link-credit,
the client will receive all messages.
However, consumption stops after exactly 2^16-1 messages.

That's because the client lib was never sending a flow frame to the server.
So, after the client received all 2^16-1 messages (the initial
incoming-window set by the client), the server's remote-incoming-window
reached 0 causing the server to stop delivering messages.

The expectation is that the client lib automatically handles session
flow control without any manual involvement of the client app.

This commit implements this fix:

  • We keep the server's remote-incoming window always large by default as
    explained in https://www.rabbitmq.com/blog/2024/09/02/amqp-flow-control#incoming-window
  • Hence, the client lib sets its incoming-window to 100,000 initially.
  • The client lib tracks its incoming-window decrementing it by 1 for
    every transfer it received. (This wasn't done prior to this commit.)
  • Whenever this window shrinks below 50,000, the client sends a flow
    frame without any link information widening its incoming-window back to 100,000.
  • For test cases (maybe later for apps as well), there is a new function
    amqp10_client_session:flow/3, which allows for a test case to do manual
    session flow control. Its API is designed very similar to
    amqp10_client_session:flow_link/4 in that the test can optionally request
    the lib to auto widen the session window whenever it falls below a certain threshold.

This is an automatic backport of pull request #13574 done by [Mergify](https://mergify.com).
This is an automatic backport of pull request #13576 done by [Mergify](https://mergify.com).

This commit fixes a bug in the Erlang AMQP 1.0 client.

Prior to this commit, to repro this bug:
1. Send more than 2^16 messages to a queue.
2. Grant more than a total of 2^16 link credit initially (on a single link
   or across multiple links) on a single session without any
   auto or manual link credit renewal.

The expectation is that thanks to sufficiently granted initial link-credit,
the client will receive all messages.
However, consumption stops after exactly 2^16-1 messages.

That's because the client lib was never sending a flow frame to the server.
So, after the client received all 2^16-1 messages (the initial
incoming-window set by the client), the server's remote-incoming-window
reached 0 causing the server to stop delivering messages.

The expectation is that the client lib automatically handles session
flow control without any manual involvement of the client app.

This commit implements this fix:
* We keep the server's remote-incoming window always large by default as
  explained in https://www.rabbitmq.com/blog/2024/09/02/amqp-flow-control#incoming-window
* Hence, the client lib sets its incoming-window to 100,000 initially.
* The client lib tracks its incoming-window decrementing it by 1 for
  every transfer it received. (This wasn't done prior to this commit.)
* Whenever this window shrinks below 50,000, the client sends a flow
  frame without any link information widening its incoming-window back to 100,000.
* For test cases (maybe later for apps as well), there is a new function
  `amqp10_client_session:flow/3`, which allows for a test case to do manual
  session flow control. Its API is designed very similar to
  `amqp10_client_session:flow_link/4` in that the test can optionally request
  the lib to auto widen the session window whenever it falls below a certain threshold.

(cherry picked from commit 32854e8)
(cherry picked from commit 3539462)

# Conflicts:
#	deps/amqp10_client/src/amqp10_client_session.erl
Copy link
Author

mergify bot commented Mar 19, 2025

Cherry-pick of 3539462 has failed:

On branch mergify/bp/v4.0.x/pr-13576
Your branch is up to date with 'origin/v4.0.x'.

You are currently cherry-picking commit 35394625a.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   deps/amqp10_client/src/amqp10_client.erl
	modified:   deps/rabbit/test/amqp_client_SUITE.erl
	modified:   deps/rabbitmq_amqp_client/test/management_SUITE.erl

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   deps/amqp10_client/src/amqp10_client_session.erl

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@mergify mergify bot added the conflicts label Mar 19, 2025
@michaelklishin michaelklishin added this to the 4.0.8 milestone Mar 19, 2025
ansd added 2 commits March 20, 2025 08:59
Module amqp_utils doesn't exist in `v4.0.x`.
@ansd ansd merged commit 7f683b6 into v4.0.x Mar 20, 2025
270 checks passed
@ansd ansd deleted the mergify/bp/v4.0.x/pr-13576 branch March 20, 2025 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants