Azure / Azure/azure-sdk-for-rust
eventhubs: concurrent partition attaches race on a duplicate $cbs link
- Dominant language
- Rust
- Stars
- 884
- Forks
- 365
- Avg merge
- 2d 19h
- Merged PRs (30d)
- 112
Description
## Summary
`ensure_amqp_cbs` does not cache the CBS client. Every call attaches a new `$cbs` link on the connection. AMQP permits one `$cbs` node per connection, so concurrent callers race and the broker rejects the duplicate attach.
The observed error is `AmqpError("Azure Core Error: Failed to ensure AMQP CBS")`, from a broker `NotAllowed` with the text `A link to connection ... $cbs node has already been opened`.
## Motivation
`sdk/eventhubs/azure_messaging_eventhubs/src/common/recoverable/connection.rs:601` calls `create_claims_based_security` on every invocation and returns the result. There is no cache. The `RecoverableConnection` struct caches every other resource. `connections`, `session_instances`, `sender_instances`, `receiver_instances` and `mgmt_client` each have a field. The three per-path caches use the lock-free `OnceCell` pattern. CBS has no field.
One consumer that attaches several partitions at the same time is enough to trigger this. Each partition receiver attaches on first poll, and each attach authorizes its path over CBS. `ensure_amqp_cbs` therefore runs once per partition, with no coordination between them.
A live run shows it. A consumer polled five partition streams concurrently through `select_all` on one connection. One partition failed with the duplicate `$cbs` attach, and the log carried 19 `NotAllowed` errors on that connection. The failure arrives as an untranslated `AmqpError`, so a caller that matches on a typed error kind cannot classify it.
This also hides other defects. The live test `second_processor_displaces_first_with_consumer_disconnected` fails on this error before it observes any displacement. The debug log contains no occurrence of `stolen`, so the test never reaches the behavior it exists to cover. That test already cannot pass for two other reasons, tracked with #4804 and in the epoch work, and this is a third.
## Proposal
Cache the CBS client on `RecoverableConnection`, in the same lock-free `OnceCell` shape the sender, session and receiver caches use. Concurrent callers then share one attach in flight, and a failed attach leaves the cell uninitialized so the next call retries.
Recovery must replace the cell, in the same way `apply_recovery_plan` clears the other caches. A CBS client bound to a torn-down connection must not survive a reconnect.
Add a test that drives several concurrent path authorizations on one connection and asserts a single `$cbs` attach. This defect appears only under concurrency, so a sequential test does not cover it.
Related to #4728 and #4806, which fix the management-client cache, and to #4810, which covers the connection lock. This is the same family: a per-connection resource whose caching does not match its AMQP cardinality.
Contributor guide
Research direction
Start in sdk/eventhubs/azure_messaging_eventhubs/src/common/recoverable/connection.rs around line 601, then compare the existing OnceCell caches and apply_recovery_plan. Add a concurrent authorization test that asserts one $cbs attach, and verify failed attaches can retry after recovery without retaining a client from the torn-down connection.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend, networking
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100