Azure / Azure/azure-sdk-for-cpp

WS4: Fix the processor error path and the blob checkpoint store

Open
#7,256 0 comments 0 reactions 1 assignee Claimed by @j7nw4r View on GitHub
Event Hubs
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

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.