Skip to content

pin the reservation to the lease try_reserve actually charged - #123

Merged
bpapillon merged 1 commit into
mainfrom
bpapillon/sch-7487-sdk-reservation-can-pin-the-wrong-lease-id-when-the-slots
Sep 17, 2026
Merged

bpapillon merged 1 commit into
mainfrom
bpapillon/sch-7487-sdk-reservation-can-pin-the-wrong-lease-id-when-the-slots

Conversation

@bpapillon

Copy link
Copy Markdown
Collaborator

The client-mode check took the lease ID from acquire_if_needed and then called try_reserve, which debits whichever lease the slot holds at that moment and is not keyed by lease ID. Between the two calls the slot's lease can be replaced: the window spans the awaited extend, and the sweeper or a sibling pod can install a successor. The credits then came out of the new lease while the reservation pinned the old one, so the settle and sweep refunds were dropped and the track event billed a released lease.

Ports the shape from schematic-node #186. try_reserve now returns the post-debit balance and the lease it charged as a ReserveResult, read under the same lock as the debit in memory and inside the same script on Redis. The Lua body is byte-identical to Node's, and the key layout and hash fields are unchanged, so mixed fleets keep sharing one Redis. The reservation record, the failed-persist undo, and through the record the track event all pin the returned ID.

SPEC.md and the lease-lifecycle vectors carry Node's additions verbatim. Tests cover the swap window on both backends, the settling track event carrying the successor's ID, and the real extend window on a shared fakeredis.

Shares a test helper with #122; whichever merges second needs a small rebase.

@bpapillon
bpapillon requested a review from a team as a code owner September 17, 2026 00:25
@bpapillon
bpapillon merged commit b56cdf9 into main Sep 17, 2026
6 checks passed
@bpapillon
bpapillon deleted the bpapillon/sch-7487-sdk-reservation-can-pin-the-wrong-lease-id-when-the-slots branch September 17, 2026 00:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants