fix(eventhubs): treat a lost ownership claim as a normal outcome - #5112
Draft
Johnathan W (j7nw4r) wants to merge 2 commits into
Draft
fix(eventhubs): treat a lost ownership claim as a normal outcome#5112Johnathan W (j7nw4r) wants to merge 2 commits into
Johnathan W (j7nw4r) wants to merge 2 commits into
Conversation
|
Azure Pipelines: Successfully started running 1 pipeline(s). 3 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
InMemoryCheckpointStore::claim_ownership returns an error when a competing instance rotates the ETag first. That error travels up through the load balancer to EventProcessor::run, which stops every partition. BlobCheckpointStore already treats the same race as a normal outcome and returns an empty result. Add four tests for the target contract: - a lost claim returns Ok with no ownership and does not change the winner's record - a lost claim in the middle of a batch does not cancel the partitions behind it, and the winners' rotated ETags reach the caller - a validation failure still returns an error, which stops a fix that swallows every error in the loop - the load balancer survives a lost claim and owns no partition Three of the four fail against the current store. The validation test passes today and guards the behavior that must not change.
`InMemoryCheckpointStore::claim_ownership` returned an error when the caller presented a stale ETag. One lost partition therefore cancelled the whole batch: the error reached `EventProcessor::run`, which stopped every partition the instance owned and discarded the new ETag for each partition that the same batch had already renewed. A private `try_update_ownership` helper now reports a stale ETag as `Ok(None)` and logs the conflict at the debug level with the `claim-conflict` event field that `BlobCheckpointStore` uses. `claim_ownership` skips that partition and keeps the claims it made. A store failure still leaves the batch as an error. The public `update_ownership` keeps its error contract for a stale ETag, and the `CheckpointStore` trait doc now states that a lost claim is not an error.
Johnathan W (j7nw4r)
force-pushed
the
j7nw4r/fix-eventhubs-lost-claim-ends-run
branch
from
August 25, 2026 18:13
514e545 to
32d6860
Compare
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
InMemoryCheckpointStore::claim_ownershipreturned an error when a competing instance rotated the ETag first. That error reachedEventProcessor::run, which stopped every partition the instance owned. A lost claim is now a normal outcome, and the cycle continues without that partition.Motivation
A lost claim is an ordinary result of load balancing under
ProcessorStrategy::Balanced, and two instances that share one checkpoint store are the ordinary deployment shape.BlobCheckpointStorealready treats the race as normal and omits the partition from its result.InMemoryCheckpointStorebroke that contract, so one stale ETag cancelled the whole batch and the error travelled throughload_balanceanddispatchintorun(). The same abort also discarded the rotated ETag of every partition that the batch had already renewed, which left the caller with records it could never renew again.Changes
InMemoryCheckpointStore::claim_ownershipnow skips a partition with a stale ETag, keeps the claims the batch already made, and returns the remaining partitions.try_update_ownershiphelper reports a lost claim asOk(None)and reservesErrfor a store failure, so no code path matches on a rendered error message.claim-conflictevent field thatBlobCheckpointStoreuses.InMemoryCheckpointStore::update_ownershipkeeps its public signature and still returns an error for a stale ETag.CheckpointStore::claim_ownershipdoc now states that a lost claim is not an error, and that an implementation must omit that partition from the returned vector.### Bugs Fixedentry to the crate CHANGELOG.The production code in
load_balancer.rsandprocessor.rsdid not change, and there is no public API change.Test plan
Four tests were added. Three of them failed before the fix.
test_claim_ownership_lost_claim_is_not_an_error: a stale ETag returnsOkwith no ownership, and the winner's record stays unchanged. Failed before the fix.test_claim_ownership_continues_past_a_lost_claim: a lost claim in the middle of a three-partition batch does not cancel the partitions around it, and the rotated ETags of the winners reach the caller. Failed before the fix.test_claim_ownership_invalid_ownership_still_errors: a validation failure still returns an error. This test passes before and after the fix. It guards against a fix that swallows every error in the loop, and it fails whenclaim_ownershipdiscards errors instead of classifying them.load_balance_survives_a_lost_claim:load_balancereturnsOkand owns no partition after a competing instance claims it first. Failed before the fix.Every test runs offline. No live test was added, and no live test was run.
Results with
CARGO_BUILD_JOBS=1and--test-threads=1: 196 passed before the change, and 200 passed with 0 failed after it.cargo test --no-run --package azure_messaging_eventhubswithRUSTFLAGS=-Dwarningsexits 0.cargo fmt --check,cargo clippy --all-features --all-targets --no-deps, andcargo doc --no-depseach exit 0. Theazure_messaging_eventhubs_checkpointstore_blobcrate still builds.Follow-up
No structured discriminator for a claim conflict is shared across
CheckpointStoreimplementations.BlobCheckpointStoreclassifies the conflict withazure_core::Error::http_status()againstStatusCode::PreconditionFailedandStatusCode::Conflict.InMemoryCheckpointStoreproducedErrorKind::Otherwith a formatted message, which only a string match could classify. This change removes the need for a match, because the conflict never becomes anError. The gap remains for a third-partyCheckpointStore, which can still return an error for a lost claim and endrun(). A proposed remedy is anErrorKind::OwnershipLostvariant on the crate's#[non_exhaustive]error enum, whichload_balancecan detect through the same source-chain downcast thatfind_link_stolenalready uses.The two stores now log the same event at different levels.
BlobCheckpointStoreusesinfo!andInMemoryCheckpointStoreusesdebug!, which is the level this issue asked for. The .NET SDK logs its equivalentOwnershipNotClaimableevent at the Informational level. Aligning the two Rust stores on one level is a small follow-up if that is preferred.Closes #5095