Skip to content

Conversation

@anuchandy
Copy link
Member

@anuchandy anuchandy commented Nov 7, 2024

The synchronous acceptNextSession method retries the session requests if the broker signals no session is available (broker-timeout). This retry loop continues until a session is available or a client-side timeout occurs. When client-side timeout happens, an IllegalStateException with a "timeout on blocking read for" message is thrown. It's impossible to cancel a sent session request. If the client-side timeout triggers after sending a session request, but the broker finds a session before broker-timeout, the session may be temporarily locked until the lock expires.

To address this, synchronous acceptNextSession (and acceptSession(sessionId)) has been updated to propagate the broker-timeout error. To avoid breaking change, library will continue to throw IllegalStateException but with cause as TimeoutException. The client-side timeout is still applied which is useful in case of any unexpected network delay.

The .block(timeout) method is replaced with a new chain: .timeout(timeout, Mono.error(..)).onErrorMap(..).block(). The new chain will continue to throw the same error IllegalStateException but with TimeoutException as the cause. In cases of broker-side timeout, the cause will be TimeoutException("com.microsoft:timeout"), while for client-side timeout, the cause will be TimeoutException("Timeout on blocking read for .. (client-timeout)").

Also reduced the few log levels in ServiceBusSessionAcquirer to verbose from info.

@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

@anuchandy anuchandy force-pushed the propagate-broker-session-timeout branch from 3f489ba to 2ee331a Compare November 7, 2024 19:29
Copy link
Member

@conniey conniey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@anuchandy anuchandy force-pushed the propagate-broker-session-timeout branch from f663718 to 310eb52 Compare November 11, 2024 00:29
@anuchandy anuchandy marked this pull request as ready for review November 11, 2024 02:13
@anuchandy anuchandy requested a review from lmolkova as a code owner November 11, 2024 02:13
@anuchandy anuchandy self-assigned this Nov 11, 2024
@anuchandy
Copy link
Member Author

/azp run java - servicebus - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@anuchandy
Copy link
Member Author

/azp run java - servicebus - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@anuchandy anuchandy merged commit c81f596 into Azure:main Nov 11, 2024
34 checks passed
@mwasaccess
Copy link

Hi @anuchandy,
we have a large number of TimeoutException entries in Application Insights logs when using azure-messaging-servicebus:7.17.7

My understanding is that TimeoutException is published to bounded-elastic thread by the publishError method of ServiceBusSessionAcquirer, but doOnError does not consume the exception - because of that it is automatically instrumented and logged by Application Insights agent. Shouldn't it use onError?

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.

4 participants