cockroachdb / cockroachdb/cockroach
kvnemesis: let operations carry their lock timeout instead of threading it through the applier
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
**Summary**
kvnemesis sets a `LockTimeout` on the batches issued by a fraction of transactions, so that operations blocking on a conflicting lock fail with a `WriteIntentError`. The timeout is chosen once per closure transaction and stored on `ClosureTxnOperation.LockTimeoutNanos`. To apply it, the applier extracts the duration and threads it as an explicit `lockTimeout time.Duration` parameter into `applyClientOp`, which sets it on every batch header it builds.
It would read more naturally for the individual `Operation` to carry its own lock timeout, so that `applyClientOp` reads `op.LockTimeoutNanos` off the operation rather than receiving it as a side parameter.
**Why this isn't a trivial field addition**
`Operation` is a `gogoproto.onlyone` union (see the generated `GetValue()`), so `onlyone` codegen assumes every field is a nil-comparable message pointer. Adding a scalar `int64 lock_timeout_nanos` to `Operation` produces uncompilable generated code:
```
operations.pb.go: invalid operation: this.LockTimeoutNanos != nil (mismatched types int64 and untyped nil)
```
because `onlyone` emits `if this.LockTimeoutNanos != nil { return this.LockTimeoutNanos }`. Every other `onlyone` message in the tree is message-only, so a scalar has never had to work there.
**Options**
- Drop `gogoproto.onlyone` from `Operation` and hand-roll `GetValue`/`SetValue`. This touches the core op-dispatch used throughout kvnemesis — largest churn.
- Add the timeout to each leaf op message (`GetOperation`, `PutOperation`, …). Removes the parameter but duplicates the field ~15 ways and pushes per-type handling into the applier.
- Bundle the applier's growing argument list (`inTxn`, `spIDToToken`, `lockTimeout`) into a small config struct. Removes the bare `time.Duration` parameter without proto changes, though it doesn't literally move the value onto the leaf operation.
- Leave as-is. The value already lives on the proto at its correct (transaction) level — the validator and formatter also read it from `ClosureTxnOperation` — and the parameter is just how a per-batch-header value chosen once per txn reaches the batch builder.
**Code references**
- `pkg/kv/kvnemesis/applier.go` — the `lockTimeout` parameter on `applyClientOp`; the `ClosureTxnOperation` case reads `o.LockTimeoutNanos` and threads it down.
- `pkg/kv/kvnemesis/operations.proto` — `ClosureTxnOperation.lock_timeout_nanos`; `Operation` with `option (gogoproto.onlyone) = true`.
- `pkg/kv/kvnemesis/validator.go`, `operations.go` — read the timeout from `ClosureTxnOperation` for validation and formatting.
Jira issue: CRDB-65666
Contributor guide
Research direction
Start with pkg/kv/kvnemesis/applier.go and trace how ClosureTxnOperation.LockTimeoutNanos reaches applyClientOp. Read the Operation union and onlyone setting in pkg/kv/kvnemesis/operations.proto, then compare timeout handling in validator.go and operations.go. Done means the chosen design removes or relocates the side parameter without breaking generated code or the existing lock-timeout behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- databases, testing
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100