Azure / Azure/azure-sdk-for-cpp

[EventHubs] The receiver rebuild backoff does not honor cancellation

Open
#7,332 1 comment 0 reactions 0 assignees View on GitHub
Client Event Hubs needs-team-attention Service Attention
Dominant language
C++
Stars
205
Forks
172
Avg merge
1d 3h
Merged PRs (30d)
37

Description

## Summary

The receiver rebuild path in `PartitionClient::ReceiveEvents` sleeps for the whole backoff period before it tests the context. A caller that cancels its context during a rebuild storm waits for that whole period before the call returns.

## Motivation

The `recover` lambda calls `std::this_thread::sleep_for(retryAfter)` and it tests the context only after that sleep (`sdk/eventhubs/azure-messaging-eventhubs/src/partition_client.cpp:348-349`). The sleep is one uninterruptible block, so a cancel that arrives at the start of the sleep has no effect until the sleep ends.

`retryAfter` comes from `RetryOperation::ShouldRetry` (`sdk/eventhubs/azure-messaging-eventhubs/src/partition_client.cpp:329-331`). `CalculateExponentialDelay` doubles the delay for each attempt, and it caps the result at `RetryOptions::MaxRetryDelay` (`sdk/eventhubs/azure-messaging-eventhubs/src/retry_operation.cpp:148-173`). So a late rebuild attempt holds the calling thread for the whole cap after the caller asked it to stop.

The send retry loop already solves this problem. `WaitForRetryDelay` sleeps in steps of 100 ms, and it calls `context.ThrowIfCancelled()` in each step (`sdk/eventhubs/azure-messaging-eventhubs/src/retry_operation.cpp:14-32`). `RetryOperation::Execute` uses that helper for every backoff that it takes (`sdk/eventhubs/azure-messaging-eventhubs/src/retry_operation.cpp:92`). The helper sits in an anonymous namespace, so it is file local, and the rebuild path cannot call it.

This path came in with #7328, which is the link reattach part of #7254.

## Proposal

Move `WaitForRetryDelay` and its `CancellationCheckInterval` constant out of the anonymous namespace in `sdk/eventhubs/azure-messaging-eventhubs/src/retry_operation.cpp`. Declare the function in the `_detail` namespace in `sdk/eventhubs/azure-messaging-eventhubs/src/private/retry_operation.hpp`, so both call sites use one implementation.

Replace the `sleep_for` call and the `ThrowIfCancelled` call in the `recover` lambda with a call to that helper. The helper throws `OperationCancelledException` from inside the wait, and the lambda already lets that type through, so the cancel path needs no other change.

Add a test that cancels the context during a rebuild backoff. The test must assert that the call returns in much less time than the backoff period.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.