Skip to content

Correctness: callback completeness, ttl validation, RefreshTimeout, refresh-lock CAS - #13

Open
dcbickfo wants to merge 2 commits into
mainfrom
fix/correctness-bundle
Open

Correctness: callback completeness, ttl validation, RefreshTimeout, refresh-lock CAS#13
dcbickfo wants to merge 2 commits into
mainfrom
fix/correctness-bundle

Conversation

@dcbickfo

Copy link
Copy Markdown
Owner

Follow-up to a thorough review of the v0.2.0 cycle. Fixes the one confirmed-new correctness sharp edge (the map-callback contract) plus the tracked RC-1/RC-5/RC-6 correctness items. No hot-path change; all tests green under -race, lint clean.

Fixed

  • GetMulti no longer hangs (re-locking + re-invoking the callback until the ctx deadline) when the callback omits a requested key — it fails fast with an error naming the missing keys. The contract is now: the callback must return a value for every key it is given (empty string for a known-absent entity).
  • SetMulti no longer silently reverts + returns nil for callback-omitted keys — they're surfaced in the returned BatchError.
  • Refresh-ahead callbacks are no longer capped at LockTTL (RC-1). A callback slower than LockTTL was cancelled mid-flight and mis-reported as an error, silently disabling refresh for slow-to-recompute values. The successful back-write now runs under a fresh context so a callback that consumed most of its budget still persists its value.
  • Refresh lock released via ownership CAS (RC-6): holds a unique token (not a fixed value) and CAS-deletes, so an overran refresh can't delete a concurrent refresher's lock.
  • tryGetMulti length guard (RC-5): fails loud on a truncated DoMultiCache response instead of silently dropping keys (mirrors waitForReadLocks).

Added

  • RefreshTimeout option — refresh-callback compute budget, decoupled from LockTTL; defaults to the value's per-call ttl.
  • Per-call ttl validation across Get/GetMulti/Set/SetMulti/Touch/TouchMulti/ForceSet/ForceSetMulti — rejects non-positive ttl up front instead of an opaque Redis error; notably Touch/TouchMulti no longer delete the key via PEXPIRE 0.

Changed

  • ForceSetMulti wraps its first error with key context (matches DelMulti).
  • Removed dead mapsx.Values; doc fixes (lock-value wording, refresh best-effort note, README Go version / Close() / Metrics list).

Tests

  • New callback_contract_test.go and refresh_timeout_test.go (TDD: watched-fail then pass).
  • RC-5/RC-6 ship as defensive/consistency guards whose bug conditions can't be triggered through the public API; existing refresh tests confirm no regression.

Verification

  • go test -race ./... — green; golangci-lint run — 0 issues; go vet — clean.

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.

1 participant