[Service Bus] Bound sync acceptNextSession hang and add session-acquire retry backoff (#49093)#49806
Open
ksalazar-91 wants to merge 3 commits into
Open
Conversation
…re retry backoff (Azure#49093) Bounds a single synchronous session-acquire attempt with a client-side guard of 2x tryTimeout (disposing the half-open link when it fires) so a hung acceptNextSession()/acceptSession() fails in bounded time instead of blocking for the full ~245s operation timeout. Adds a bounded backoff between session-acquire retries on the session processor/async receiver path to avoid a CPU-burning tight loop when acquire attempts fail fast. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 406b56c0-6215-4747-b9bf-c7d7b8ab1e89
ksalazar-91
requested review from
a team,
EldertGrootenboer and
skarri-microsoft
as code owners
July 14, 2026 17:02
github-actions
Bot
added
Community Contribution
Contributor
|
Thank you for your contribution @ksalazar-91! We will review the pull request and get back to you soon. |
|
Azure Pipelines: Successfully started running 1 pipeline(s). 34 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses intermittent hangs and CPU spikes during Service Bus session acquisition by bounding the synchronous session-acquire attempt duration and adding a non-zero backoff between retry attempts on the async/processor path.
Changes:
- Added a client-side timeout guard for the non-retry (sync) session-acquire path and ensured half-open receive links are disposed on cancellation.
- Replaced the zero-delay retry scheduling with a bounded backoff to avoid tight retry loops.
- Added virtual-time regression tests and documented the behavior in CHANGELOG and
acceptNextSession()Javadoc.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusSessionAcquirer.java | Adds client-side guard for hung acquires, cancellation disposal, and retry backoff for retry-enabled session acquisition. |
| sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusSessionReceiverClient.java | Clarifies acceptNextSession() behavior in Javadoc regarding operation-timeout behavior. |
| sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/ServiceBusSessionAcquirerIsolatedTest.java | Adds virtual-time tests covering hung-acquire timeout/dispose and retry backoff behavior. |
| sdk/servicebus/azure-messaging-servicebus/CHANGELOG.md | Documents the bug fix and behavioral details for session acquisition. |
Comment on lines
159
to
163
| * @throws UnsupportedOperationException if the queue or topic subscription is not session-enabled. | ||
| * @throws IllegalStateException if no session becomes available within the operation timeout (derived from | ||
| * the {@link ServiceBusClientBuilder#retryOptions(AmqpRetryOptions)}); this is retriable. | ||
| * @throws AmqpException if the operation times out. The timeout duration is the tryTimeout | ||
| * of when you build this client with the {@link ServiceBusClientBuilder#retryOptions(AmqpRetryOptions)}. |
Comment on lines
+106
to
+109
| final Duration configuredDelay = connectionCacheWrapper.getRetryOptions().getDelay(); | ||
| this.retryBackoff = (configuredDelay == null || configuredDelay.isZero() || configuredDelay.isNegative()) | ||
| ? Duration.ofMillis(100) | ||
| : configuredDelay; |
…re#49093) Clarify acceptNextSession()/acceptSession() @throws Javadoc: the operation timeout surfaces as IllegalStateException, while AmqpException covers AMQP-level failures (not the operation timeout). Add regression tests asserting both exception paths. Clarify the retry-backoff comment to state it honors the customer's configured AmqpRetryOptions delay (defaulting to 100ms only when no usable delay is configured); no behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 406b56c0-6215-4747-b9bf-c7d7b8ab1e89
EldertGrootenboer
approved these changes
Jul 17, 2026
EldertGrootenboer
left a comment
EldertGrootenboer
left a comment
Member
There was a problem hiding this comment.
The fix looks right and the mechanism holds up.
The 2x tryTimeout guard surfaces as IllegalStateException and bounds the wait at ~120s instead of ~245s, matching the updated Javadoc.
The retry-path backoff covers the CPU spike on the pump.
The backoff takes the configured delay verbatim when it's above zero, so a sub-100ms delay can still spin.
A floor like max(configuredDelay, 100ms) would fix it. Non-blocking, but I'd take the one-liner.
Approving.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes intermittent
ServiceBusSessionReceiverClient.acceptNextSession()blocking for the full operation timeout (~245s with default retry options) and throwingIllegalStateException: Timeout on blocking read for 245600000000 NANOSECONDS (client-timeout), plus a CPU-burning tight retry loop on the session Processor / async session receiver.Resolves #49093.
Root cause
ServiceBusSessionReceiverClient.accept*Session()wraps the async accept with.timeout(operationTimeout)whereoperationTimeout = MessageUtils.getTotalTimeout(retryOptions). With defaultAmqpRetryOptions(tryTimeout=60s, maxRetries=3, delay=800ms, exponential) that is60 + 60.8 + 61.6 + 63.2 = 245.6s, which matches the reported245600000000ns exactly.timeoutRetryDisabled = true),ServiceBusSessionAcquirerhad no client-side per-attempt timeout. When the broker accepts the session-acquire link but never sends its detach (a hung acquire — half-open connection, network partition, or QPid-thread starvation across many idle pumps), the only safety net was the outer 245.6s sync timeout, so the caller blocked for the full duration before failing. The acquirer's own javadoc claimed the client-side timeout was "slightly longer than the broker's timeout", but it was effectively ~4× the try-timeout.ServiceBusProcessorClientandServiceBusSessionReceiverAsyncClient), retries were scheduled withMono.delay(Duration.ZERO)— no backoff. When acquire attempts fail fast (e.g. the broker repeatedly detaches the accept link quickly), this becomes a tight, CPU-burning retry loop.Fix (servicebus-only)
ServiceBusSessionAcquirer:2 × tryTimeout. The 2× margin keeps the broker's own timeout detach the usual trigger (so the broker-timeout error still propagates in the healthy "no session available" case), while a hung acquire now fails in bounded time (~120s with defaults) instead of ~245s. On the guard firing, the half-open receive link is disposed (only on cancellation) so the broker-side session lock is released rather than orphaned.Mono.delay(Duration.ZERO)with a bounded backoff (honoring the customer's configuredAmqpRetryOptionsdelay, and defaulting to 100ms only when no usable delay (null/zero/negative) is configured), still hopping toSchedulers.parallel()to free the QPid thread. This prevents the tight CPU loop.ServiceBusSessionReceiverClient: clarified theacceptNextSession()javadoc — it does not wait indefinitely; it throws a retriable timeout after the operation timeout, so callers typically invoke it in a loop.Reason for the change
Customers running idle session pumps saw
acceptNextSession()hang ~245s and CPU spike right around the failures, causing Kubernetes pods to be killed and restarted.How verified
ServiceBusSessionAcquirerIsolatedTest:shouldClientSideTimeoutAndDisposeLinkWhenAcquireHangsIfRetryDisabled— a hung acquire fails withTimeoutExceptionat the2 × tryTimeoutguard and the half-open link is disposed.shouldBackOffBetweenRetriesIfRetryEnabled— the second acquire attempt is not made until the backoff elapses (no busy loop).ServiceBusSessionAcquirerIsolatedTest(6/6) and the related session unit test classes pass; checkstyle + spotbugs clean (JDK 21).Are there any breaking changes?
No. Behavior on the healthy path is unchanged (broker timeout still propagates); only the hung-acquire bound is tightened and retry spacing is added.