ITADN

Hoist AMQP session window defaults (u32::MAX) into a shared azure_core_amqp helper

#4570Openj7nw4r 创建于 2026-06-10
ClientAzure.CoreService BusEvent Hubsneeds-team-triage
J
j7nw4rcommented
### Background Both messaging crates set the AMQP session flow-control windows to `u32::MAX` when beginning a session, independently and identically: - Event Hubs: [`connection.rs` `get_session`](https://github.com/Azure/azure-sdk-for-rust/blob/main/sdk/eventhubs/azure_messaging_eventhubs/src/common/recoverable/connection.rs) sets `incoming_window: Some(u32::MAX)` / `outgoing_window: Some(u32::MAX)`. - Service Bus: `sdk/servicebus/azure_messaging_servicebus/src/receiver.rs` (~L799) sets the same two values. This was raised in review on #4446, where the question was whether these should be the defaults for `AmqpSessionOptions`, or whether the values reflect a Service Bus versus Event Hubs tuning difference. The values are not divergent tuning: Event Hubs and Service Bus both use `u32::MAX`, so the setting is duplicated across both crates with no single source of truth or documented rationale. ### Current state `AmqpSessionOptions` is `#[derive(Default)]`, so `incoming_window` / `outgoing_window` default to `None`. At the fe2o3 layer (`sdk/core/azure_core_amqp/src/fe2o3/session.rs`), `None` causes the builder setter to be skipped, so the window falls through to fe2o3-amqp's own builder default. Setting the windows to the maximum disables session-level flow control and defers all backpressure to per-link credit, which is the behavior both messaging crates require. ### Proposal Introduce a shared definition in `azure_core_amqp` rather than repeating the literal at each call site, for example an `AmqpSessionOptions::with_unbounded_windows()` constructor or a documented `pub const`, and update Event Hubs and Service Bus to use it. This removes the duplication and documents the intent in a single location. Changing the derived `Default` to `Some(u32::MAX)` is not recommended. `azure_core_amqp` is the generic AMQP transport layer, and maximal windows (no session-level flow control) is a messaging-specific choice. Retaining `None` as the generic default (defer to the AMQP implementation) keeps behavior conservative for non-messaging consumers, while a named constructor still provides Event Hubs and Service Bus a single tuned entry point. Should the team consider `azure_core_amqp` effectively Azure-internal, a hand-written `impl Default` would also be acceptable, though more opinionated. ### Notes - Out of scope for #4446; those lines are pre-existing and not part of that PR's diff. - Low impact: the only two in-repo construction sites already set the windows explicitly.
0 条评论