Azure / Azure/azure-sdk-for-rust

Event Hubs: InMemoryCheckpointStore accepts an ownership claim with an ETag for a record that does not exist

Open
#4,868 0 comments 1 reaction 1 assignee Claimed by @j7nw4r View on GitHub
Client Event Hubs
Dominant language
Rust
Stars
884
Forks
365
Avg merge
2d 19h
Merged PRs (30d)
109

Description

## Summary

`InMemoryCheckpointStore::update_ownership` accepts an ownership claim that carries an ETag when no ownership record exists for the key. The production `BlobCheckpointStore` rejects the same claim. The two stores give different results for the same input. The in-memory store is not a correct test double for this case.

The work on #4594 and PR #4703 showed this gap. That PR covers only the ETag rotation and the timestamp refresh on renewal. This gap is out of scope for that PR.

## Motivation

`update_ownership` in `sdk/eventhubs/azure_messaging_eventhubs/src/in_memory_checkpoint_store.rs` looks up the key first. When the store holds a record, the code compares the supplied ETag to the stored ETag, and it returns an error on a mismatch. When the store holds no record, the code never reads the supplied ETag. The code makes a new ETag and it inserts the record. A caller that supplies an ETag makes a statement about a record version that it holds. When no record exists, that statement is false. The store must then reject the claim.

PR #4703 collapses the renewal path and the first-claim path into one shared insert path. The defect stays the same after that change, but the code shape changes. The fix goes next to the existing ETag guard, not in a separate branch.

`BlobCheckpointStore` rejects the claim. `set_ownership_metadata_on_blob` in `sdk/eventhubs/azure_messaging_eventhubs_checkpointstore_blob/src/checkpoint_store.rs` sends `set_metadata` with `if_match` set to the supplied ETag. The function uploads a new blob only when the caller supplies no ETag. A `set_metadata` request against a blob that does not exist does not succeed, so the blob store does not grant the claim.

The exact status code needs a test against the live service. The Azure documentation states that an unmet `If-Match` condition on a write operation gives 412 (Precondition Failed). The documentation does not state the result for a blob that does not exist, where the service usually gives 404 (BlobNotFound). The difference changes the outcome. `BlobCheckpointStore::claim_ownership` maps 412 to a skipped partition and it continues the loop, but it returns any other error to the caller and it abandons the batch. Make sure of the real status code before you choose the fix. See also #4867, which covers the batch-abort behavior.

The practical case is a lost or deleted ownership record. A stale consumer then claims the partition with its old ETag. The in-memory store grants the claim, and the blob store denies it. A test that uses the in-memory store passes. The same code loses the claim against the blob store.

## Proposal

Reject the claim in `update_ownership` when the caller supplies an ETag and the store holds no record for the key. Return the same error kind and the same message shape as the ETag mismatch path. Both stores then deny the claim for the same reason. Keep the current behavior for a caller that supplies no ETag, because that caller asks for a first claim. Add a unit test that supplies an ETag for an absent key. The test must expect an error.

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.