Repository navigation
feat: make ClaimVM idempotent with request_id - #106
Merged
richardcase merged 4 commits intoSep 29, 2026
Merged
Conversation
store.Open now reads PRAGMA user_version and applies a numbered list of migrations on top of the schema.sql baseline, each in its own transaction with the version bump. Migration 1 adds leases.request_id with a partial unique index. ClaimVMRequest and LeaseRecord gain request_id. CreateLease stores it (empty as NULL) and returns ErrDuplicateRequestID when the index rejects a row, and the Store gains GetLeaseByRequestID. poolmgrctl lease claim gains --request-id, and lease list shows a REQUEST_ID column. The server does not act on request_id yet; replay semantics follow in the next change.
ClaimVM now rejects a request_id over 255 bytes with INVALID_ARGUMENT and, when one is set, looks up an existing lease before claiming. A match on the same pool returns that lease again without extending its expiry; a match on another pool fails with INVALID_ARGUMENT; no match claims as before and records request_id on the new lease. If a concurrent claim with the same request_id commits its lease first, CreateLease's ErrDuplicateRequestID sends the loser's VM back to AVAILABLE and returns the winner's lease. The fresh-claim and replay paths share claimResponse to build the ClaimVMResponse. poolmgr_vm_claims_total gains a replayed="true|false" label. The ADR is now Accepted, and the README and design doc describe retrying claims.
This was referenced Sep 25, 2026
richardcase
reviewed
Sep 29, 2026
richardcase
reviewed
Sep 29, 2026
EnsureVMDeleted marks a VM DELETING before it calls flintlock. A failed delete leaves both the VM and lease rows in place. A replay then returned success for a VM that is being removed. Return ABORTED instead, as we do once the VM row is gone.
The lease column list and Scan arguments were repeated in six places. Move them into leaseColumns, scanLease and queryLeases, so a new column cannot be added to some read paths and missed on others.
richardcase
approved these changes
Sep 29, 2026
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
2 participants
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.
Closes #98. Part 1 of #105.
A client can now retry
ClaimVMwithout losing a VM. The client setsrequest_idon the request. If a lease with that ID still exists, the manager returns that lease again and does not claim a second VM. This follows the idempotent claims ADR.Changes
Store:
store.Opennow applies numbered migrations and records the version inPRAGMA user_version.schema.sqlis the version 0 baseline. Each migration runs in its own transaction.leases.request_idwith a partial unique index. An empty ID is stored as NULL, so claims without an ID never conflict.CreateLeasereturnsErrDuplicateRequestIDwhen the index rejects a row.GetLeaseByRequestIDis new.Openrefuses a database that a newer binary has migrated.API:
ClaimVMRequest.request_idandLeaseRecord.request_idare new fields.INVALID_ARGUMENT.INVALID_ARGUMENT.AVAILABLEand returns the winner's lease.poolmgr_vm_claims_totalhas a newreplayedlabel.CLI:
poolmgrctl lease claim --request-idsets the field.lease listshows aREQUEST_IDcolumn.Two cases the ADR does not cover
Both return
ABORTED, and the client can retry:In both cases a later retry makes a fresh claim.
Testing
go test -race ./...and the e2e tests pass.golangci-lint,buf lintandbuf breakingpass.