Azure / Azure/azure-sdk-for-cpp
WS4: Fix the processor error path and the blob checkpoint store
- Dominant language
- C++
- Stars
- 205
- Forks
- 172
- Avg merge
- 1d 15m
- Merged PRs (30d)
- 33
Description
## Summary
The C++ processor stops consuming without telling the application. Any exception from the dispatch path exits the load balancing loop with only a warning log, and no error callback exists. The blob checkpoint store also parses sequence numbers with `std::stol`, which is 32 bits on Windows, so any sequence number above 2^31 makes the checkpoint unreadable. The load balancing interval carries no jitter, so processors that start together stay in lockstep.
Severity tags follow the key in #7252.
## Motivation
The .NET `EventProcessorClient` requires a `ProcessErrorAsync` handler. Errors flow to that handler and the processor keeps running.
The C++ processor has no such callback. An exception from `Dispatch`, for example a checkpoint store outage or a failed properties call, exits `RunInternal` with a warning log (`src/processor.cpp:112-115`). The thread that `Start` launches swallows exceptions the same way (`src/processor.cpp:59-62`). The result is a processor that stopped consuming with no signal to the application. The load balancing loop also sleeps with `std::this_thread::sleep_for` and ignores the context (`src/processor.cpp:109`), so `Stop` can wait a full update interval before it returns.
The blob checkpoint store parses the sequence number with `std::stol` (`azure-messaging-eventhubs-checkpointstore-blob/src/blob_checkpoint_store.cpp:24`). `long` is 32 bits on MSVC, so a sequence number above 2^31 throws `std::out_of_range` and the checkpoint cannot be read. The store also requires both the `sequencenumber` and the `offset` metadata keys and throws when either is absent (`blob_checkpoint_store.cpp:19-32`), which risks interop failures with checkpoints written by other SDKs. `ClaimOwnership` swallows every exception with `catch (...)`, including cancellation (`blob_checkpoint_store.cpp:91-97`). Its ETag conditional write matches the .NET design and needs no change (`blob_checkpoint_store.cpp:80-89`).
The C++ processor sleeps exactly `m_ownershipUpdateInterval` between claim cycles (`src/processor.cpp:113`). Go jitters that interval into the range 0.8x to 1.3x (`sdk/messaging/azeventhubs/processor.go:320-324`), and Python adds up to 20 percent (`azure/eventhub/_eventprocessor/event_processor.py:258`). Without jitter, processors that start in the same window run every claim cycle in step and contend on the same blobs. The load balancer does seed `std::rand` from `std::time(nullptr)` (`src/private/processor_load_balancer.hpp:131`), which has one second resolution, so processors that start in the same second also draw the same ownership order.
## Proposal
- [ ] Keep the load balancing loop alive across transient dispatch failures, and surface errors to the application through an error callback, the equivalent of the .NET `ProcessErrorAsync` handler (`src/processor.cpp:59-62,112-115`). **[GA blocker]**
- [ ] Replace the `std::stol` sequence number parse with a 64-bit parse (`blob_checkpoint_store.cpp:24`). **[GA blocker]**
- [ ] Add jitter to the load balancing interval, matching the Go range of 0.8x to 1.3x (`src/processor.cpp:113`; reference `processor.go:320-324`). **[GA quality bar]**
- [ ] Make the load balancing loop sleep cancellable, so `Stop` returns promptly instead of waiting a full update interval (`src/processor.cpp:109`). **[GA quality bar]**
- [ ] Tolerate checkpoints that carry only an offset or only a sequence number (`blob_checkpoint_store.cpp:19-32`). **[GA quality bar]**
- [ ] Add cross-SDK checkpoint interop tests against blobs written by the .NET and Go processors. **[GA quality bar]**
- [ ] Replace the `catch (...) { continue; }` in `ClaimOwnership` with handling that lets cancellation escape (`blob_checkpoint_store.cpp:91-97`). **[GA quality bar]**
- [ ] Document the processor defaults and their origin, and document the ownership theft behavior: every processor uses owner level 0, so theft surfaces as an epoch detach from `ReceiveEvents` on the previous owner (`src/processor.cpp:165-170,191`). **[GA quality bar]**
## Resolution of the earlier open question
This issue previously asked whether the processor defaults should align with .NET or be documented as a divergence. The five language libraries were then compared directly:
| SDK | Strategy | Update interval | Ownership expiration | Interval jitter |
|---|---|---|---|---|
| .NET | Greedy | 30 s | 2 min | none found |
| Java | Greedy | 30 s | 2 min | none found |
| Python | Greedy | 30 s | 180 s | up to 20 percent |
| Go | Balanced | 10 s | 60 s | 0.8x to 1.3x |
| C++ | Balanced | 10 s | 60 s | none |
There is no cross-language parity to align to. The libraries sit in two camps, and C++ matches Go exactly, which is the library it was ported from. The .NET documentation states that the ownership expiration should exceed the update interval by at least a factor of two; C++ is at a factor of six, so it already satisfies that constraint.
The resolution is to document, not to align. The defaults item is now tagged **GA quality bar** as a documentation task, and it is no longer a candidate for promotion to blocker.
The comparison did surface one real gap, which the table makes visible: C++ is the only library in the Balanced camp with no interval jitter. That is now its own item above.
Anchors: `inc/azure/messaging/eventhubs/processor.hpp:32-43`, `EventProcessorOptions.cs:39,42,299`, `EventProcessorClientBuilder.java:133,138,157`, `_consumer_client.py:171`, `event_processor.py:75,104`, `processor.go:148,154,174`.
## Validation
- [ ] A processor survives an induced checkpoint store outage, keeps consuming, and reports the error through its error callback.
- [ ] `Stop` returns promptly rather than after a full update interval.
- [ ] Two processors started in the same second do not run their claim cycles in step.
- [ ] A checkpoint whose sequence number exceeds 2^31 is read correctly on Windows and on Linux.
- [ ] A checkpoint written by the .NET processor is read correctly by the C++ processor.
Contributor guide
Assessment
This issue has not been assessed yet.