fix(eventhubs): rotate ETag and refresh timestamp on InMemoryCheckpointStore renewal#4703
Draft
j7nw4r wants to merge 1 commit into
Draft
fix(eventhubs): rotate ETag and refresh timestamp on InMemoryCheckpointStore renewal#4703j7nw4r wants to merge 1 commit into
j7nw4r wants to merge 1 commit into
Conversation
…ntStore renewal The renewal branch of `InMemoryCheckpointStore::update_ownership` reinserted the caller's ownership record verbatim, leaving the ETag and `last_modified_time` unchanged. The create branch, and the production `BlobCheckpointStore`, both refresh these values on every successful claim. The divergence let the in-memory test double preserve a stale ETag across renewals, which can mask bugs in code that relies on ETag rotation for optimistic concurrency. Align the renewal branch with the create branch: rotate the ETag to a new UUID and set `last_modified_time` to the current time, returning the refreshed record. The `expire_ownership` test helper previously depended on the stale behavior to simulate an expired partition (it set a past timestamp and round-tripped through `claim_ownership`, relying on the renewal path to preserve it). Add a `#[cfg(test)]` `set_last_modified_time_for_test` seam on `InMemoryCheckpointStore` that mutates the stored timestamp directly under the mutex, and rework `expire_ownership` to use it. Fixes Azure#4594
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::update_ownershiprefreshes the ETag andlast_modified_timewhen creating a fresh ownership claim but not when renewing an existing one, where it reinserts the caller's record verbatim. This aligns the renewal path with the create path so both rotate the ETag and set the current timestamp, matching the productionBlobCheckpointStore. Fixes #4594.Motivation
InMemoryCheckpointStoreis a public, exported type that consumers can use as a test double forEventProcessor-based code. Its renewal path left the ETag andlast_modified_timeat whatever the caller passed, while the create path generated a new UUID ETag and setnow_utc(), and the productionBlobCheckpointStorederives both from the storage service response on every successful claim. The stale-ETag renewal behavior is a fidelity gap: it can mask bugs in code that relies on ETag rotation for optimistic concurrency, since a renewal that should rotate the ETag silently keeps the old one.Changes
InMemoryCheckpointStore::update_ownershiprenewal branch now clones the incoming ownership, rotatesetagto a newUuid::new_v4(), setslast_modified_timetoOffsetDateTime::now_utc(), and returns the refreshed record, mirroring the create branch.#[cfg(test)]pub(crate) fn set_last_modified_time_for_testseam onInMemoryCheckpointStorethat locks the ownership map and sets an entry'slast_modified_timedirectly. The priorexpire_ownershiptest helper depended on the buggy renewal path preserving a stale timestamp round-tripped throughclaim_ownership; that mechanism no longer works once renewal refreshes the timestamp.expire_ownershipload-balancer test helper to take&InMemoryCheckpointStoreand call the new seam instead of round-tripping throughclaim_ownership, and updated its single call site to pass the concrete store.Bugs FixedCHANGELOG entry under the unreleasedazure_messaging_eventhubsversion.Validation
cargo build -p azure_messaging_eventhubs: clean.cargo test -p azure_messaging_eventhubs --lib load_balancer: 11 passed (incl.processor_load_balancers_any_strategy_grab_expired_partitionand the relinquish test).cargo test -p azure_messaging_eventhubs --test eventhubs_checkpoint_store: 5 passed.cargo clippy -p azure_messaging_eventhubs --tests --all-features: no warnings.cargo fmt: clean.