mirror of
https://github.com/tiennm99/miti99bot.git
synced 2026-09-08 08:20:22 +00:00
docs(plan): native mongo value documents + version CAS plan
Brainstorm decision record (Option A) + validated 2-phase TDD plan for storing mongo values as native BSON and switching CAS to a version field.
This commit is contained in:
+142
@@ -0,0 +1,142 @@
|
||||
---
|
||||
phase: 1
|
||||
title: Version-Field Optimistic Locking
|
||||
status: completed
|
||||
priority: P1
|
||||
dependencies: []
|
||||
---
|
||||
|
||||
# Phase 1: Version-Field Optimistic Locking
|
||||
|
||||
## Overview
|
||||
|
||||
Replace the value-bytes `CompareAndSwap` with a version-field optimistic lock,
|
||||
across all backends + the 3 callers, while values are still stored as strings.
|
||||
Representation-neutral and independently shippable; unblocks Phase 2 (native
|
||||
storage can't use byte-exact CAS).
|
||||
|
||||
## Requirements
|
||||
|
||||
- Functional: a versioned read+swap contract — read returns value + a version
|
||||
token; write succeeds only if the stored version is unchanged, else
|
||||
`ErrConflict`. Absent key = version 0; first write must be create-only.
|
||||
- Functional: concurrent writers → exactly one winner, losers `ErrConflict`
|
||||
(same guarantee as today's CAS).
|
||||
- Non-functional: `KVStore` bulk interface (Get/Put/PutJSON/GetJSON/Delete/List)
|
||||
unchanged. <!-- Updated: Validation Session 1 - versioned CAS only on memory + mongodb; firestore backend dropped; dynamodb keeps base KVStore for the migrator (no CAS). -->
|
||||
Only memory + mongodb implement the versioned lock (the live backends).
|
||||
dynamodb keeps only the base `KVStore` (migrator uses Scan/Put, never CAS).
|
||||
The firestore backend is **removed entirely** (legacy, unused since self-host).
|
||||
|
||||
## Architecture
|
||||
|
||||
Replace `CompareAndSwapStore` with a versioned contract in
|
||||
`internal/storage/kv_store.go`:
|
||||
|
||||
```go
|
||||
type VersionedStore interface {
|
||||
// GetVersioned returns the value and its current version; ErrNotFound if absent.
|
||||
GetVersioned(ctx context.Context, key string) (val []byte, version int64, err error)
|
||||
// PutVersioned writes val iff the stored version still equals expectedVersion.
|
||||
// expectedVersion == 0 means "must not exist yet". ErrConflict on mismatch.
|
||||
PutVersioned(ctx context.Context, key string, expectedVersion int64, val []byte) error
|
||||
}
|
||||
```
|
||||
|
||||
Per-backend version source:
|
||||
- **memory** (`memory_kv.go`): per-key `version int64` alongside the bytes;
|
||||
`PutVersioned` checks+increments under the existing mutex. `prefixedStore`
|
||||
(`prefix.go`) forwards both methods with the key prefix.
|
||||
- **mongodb** (`mongodb_kv.go`): add a `version` int field to the doc.
|
||||
`GetVersioned` reads value+version; **a doc with no `version` field (written
|
||||
by the pre-refactor build) reports version 0 but still "exists"**.
|
||||
`PutVersioned` with `expectedVersion==0` → `UpdateOne(filter={_id, version:
|
||||
{$in:[0,null]} OR $exists:false}, {$set:{value,version:1,updatedAt}}, upsert:true)`
|
||||
so it upserts a new key AND adopts a legacy version-less doc without a
|
||||
spurious duplicate-key conflict; `expectedVersion>0` →
|
||||
`UpdateOne(filter={_id, version:expectedVersion}, {$set:{value,updatedAt}, $inc:{version:1}})`,
|
||||
`MatchedCount==0` → `ErrConflict`. Keeps the linearizable single-winner
|
||||
property. <!-- Updated: Validation Session 1 - legacy version-less docs treated as v0 + match missing-or-equal so existing users' updates don't fail during rollout. -->
|
||||
- **dynamodb** (`dynamodb_kv.go`): **drop `CompareAndSwap`** — the migrator only
|
||||
Scans/Puts, never locks. Keeps the base `KVStore` only.
|
||||
- **firestore**: **delete the backend** (`firestore_client.go`,
|
||||
`firestore_provider.go`, `firestore_kv.go` + tests), the `firestore` case in
|
||||
`buildProvider`, and the firestore go.mod deps. Legacy, unused since self-host.
|
||||
Keep the shared helpers it currently hosts (`validateKey`/`validatePrefix`/
|
||||
`prefixSuccessor`/`collectionNameRe`) by relocating them to a backend-neutral
|
||||
storage file so memory/mongodb/dynamodb keep compiling.
|
||||
|
||||
Callers (load → mutate → swap-by-version, retry on conflict):
|
||||
- `coin/portfolio.go` `UpdatePortfolio` + `loadPortfolioForUpdate`: capture
|
||||
`version` instead of `expected []byte`; call `PutVersioned`.
|
||||
- `gold/portfolio.go`: same shape.
|
||||
- `lolschedule/cron.go` `claimDailyPush`: `GetVersioned` the date key, claim via
|
||||
`PutVersioned(key, version, today)`; conflict → already claimed.
|
||||
|
||||
## Related Code Files
|
||||
|
||||
- Modify: `internal/storage/kv_store.go` — replace `CompareAndSwapStore` with `VersionedStore`.
|
||||
- Modify: `internal/storage/memory_kv.go`, `prefix.go`, `mongodb_kv.go` —
|
||||
implement versioned methods; drop old CAS.
|
||||
- Modify: `internal/storage/dynamodb_kv.go` — drop `CompareAndSwap` (migrate-only).
|
||||
- Create: `internal/storage/keys.go` (or similar) — relocate shared helpers
|
||||
`validateKey`/`validatePrefix` (from `firestore_kv.go`), `prefixSuccessor`
|
||||
(from `firestore_kv.go`), `collectionNameRe` (from `firestore_provider.go`)
|
||||
to a backend-neutral file so they survive the firestore deletion.
|
||||
- Delete: `internal/storage/firestore_client.go`, `firestore_provider.go`,
|
||||
`firestore_kv.go`, `firestore_kv_test.go`, `firestore_provider_test.go`.
|
||||
- Modify: `cmd/server/main.go` — remove the `firestore` case + `FirestoreProject`/
|
||||
`FirestoreEmulatorHost` config and the firestore init-timeout const.
|
||||
- Modify: `go.mod`/`go.sum` — `go mod tidy` drops `cloud.google.com/go/firestore`
|
||||
+ now-unused google.golang.org/api deps.
|
||||
- Modify: `internal/modules/coin/portfolio.go`, `internal/modules/gold/portfolio.go`,
|
||||
`internal/modules/lolschedule/cron.go` — switch to version flow.
|
||||
- Modify tests: `memory_kv_test.go`, `mongodb_kv_test.go`, `dynamodb_kv_test.go`,
|
||||
`coin/portfolio_test.go`, `gold/portfolio_test.go`, `lolschedule/cron_test.go`.
|
||||
- Check: `Makefile` (drop `firestore-emulator`/`test-emulator` targets),
|
||||
`.github/workflows/ci.yml` (firestore-emulator note), `README.md` storage list.
|
||||
|
||||
## Implementation Steps (TDD)
|
||||
|
||||
1. **Tests first — lock current behavior, expressed in the new contract.** Before
|
||||
changing impls, write/convert backend tests asserting: create-only on
|
||||
version 0, conflict on stale version, success on current version, and the
|
||||
N-goroutine concurrent single-winner test (memory + mongodb). Convert the
|
||||
coin/gold concurrent-update tests to assert the same observable outcome
|
||||
(exactly one mutation wins; balances never double-applied). These fail to
|
||||
compile/pass until the impl lands — that's the red state.
|
||||
2. Relocate shared helpers (`validateKey`/`validatePrefix`/`prefixSuccessor`/
|
||||
`collectionNameRe`) to a backend-neutral file; delete the firestore backend +
|
||||
its `buildProvider` case + config; `go mod tidy`. Confirm build still green.
|
||||
3. Define `VersionedStore`; implement in memory + `prefixedStore`.
|
||||
4. Implement in mongodb (`version` field; legacy version-less doc = v0 via
|
||||
match-missing-or-equal + upsert).
|
||||
5. Drop `CompareAndSwap` from dynamodb (keeps base `KVStore`).
|
||||
6. Migrate the 3 callers to load-version → mutate → `PutVersioned`.
|
||||
7. Make red tests green: `make test`; `make test-mongo`; `make test-dynamodb`.
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [ ] `VersionedStore` implemented + tested on memory + mongodb.
|
||||
- [ ] Concurrent version-CAS test: exactly one winner, losers `ErrConflict` (memory + real Mongo).
|
||||
- [ ] A legacy doc with no `version` field is updated successfully (treated as v0); test seeds one and asserts no spurious conflict.
|
||||
- [ ] dynamodb no longer implements CAS; migrator (Scan/Put) + its e2e test still pass.
|
||||
- [ ] firestore backend deleted; shared helpers relocated; `go mod tidy` drops firestore deps; build green.
|
||||
- [ ] coin/gold portfolio updates + lolschedule daily-push claim use version flow; their tests pass.
|
||||
- [ ] Values still stored as strings (representation unchanged this phase).
|
||||
- [ ] `make vet` + full `go test ./...` green; 12 non-CAS consumers untouched.
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
- **Legacy version-less docs (HIGH for live rollout)** — docs from the deployed
|
||||
build have no `version` field; naive create-only CAS would conflict and fail
|
||||
existing users' coin/gold updates. Mitigation: treat absent version as v0 and
|
||||
match missing-or-equal (upsert) — explicit seeded test.
|
||||
- **Helper relocation regression** — `validateKey`/`prefixSuccessor`/
|
||||
`collectionNameRe` currently live in firestore files; deleting firestore
|
||||
without relocating them breaks memory/mongodb/dynamodb. Mitigation: relocate
|
||||
first (step 2), build before proceeding.
|
||||
- **Contract ripple** — interface + 3 callers + memory/mongodb. Mitigation:
|
||||
tests-first per backend; phase is representation-neutral, ships independently.
|
||||
- **Conflict-loop regression** — a bug in version compare could exhaust the
|
||||
bounded retry. Mitigation: explicit conflict + success unit tests before wiring callers.
|
||||
+108
@@ -0,0 +1,108 @@
|
||||
---
|
||||
phase: 2
|
||||
title: Native BSON Value Representation
|
||||
status: completed
|
||||
priority: P1
|
||||
dependencies:
|
||||
- 1
|
||||
---
|
||||
|
||||
# Phase 2: Native BSON Value Representation
|
||||
|
||||
## Overview
|
||||
|
||||
Store the Mongo `value` field as a native BSON document (object/array) instead
|
||||
of a stringified-JSON blob, with a string fallback for non-JSON values, so
|
||||
values are expandable and queryable in Atlas/Compass. Depends on Phase 1's
|
||||
version CAS (byte-exact compare is gone, so native storage is now safe).
|
||||
|
||||
## Requirements
|
||||
|
||||
- Functional: JSON object → native BSON object; JSON array → native array;
|
||||
non-JSON / bare scalar (e.g. lolschedule `daily_push:last_date`) → BSON string.
|
||||
- Functional: `Get`/`GetJSON` reconstruct the caller's `[]byte`/struct from the
|
||||
native form with **no numeric type/precision loss**.
|
||||
- Functional: dual-read — documents written by the prior string/binary build
|
||||
still read correctly.
|
||||
- Non-functional: `KVStore` + `VersionedStore` signatures unchanged; the 12
|
||||
PutJSON/GetJSON/List consumers untouched. Conversion is confined to
|
||||
`mongodb_kv.go`.
|
||||
|
||||
## Architecture
|
||||
|
||||
### JSON ↔ BSON conversion (the fidelity-critical part)
|
||||
- **Write** (`Put`/`PutVersioned` receive JSON `[]byte`): if `json.Valid` and
|
||||
the top level is `{` or `[`, decode with `json.Decoder` + `UseNumber()` into a
|
||||
generic value, then a recursive `jsonToBSON` converts `json.Number` →
|
||||
**int64 when integral and representable, else float64** (so int64 stays int64,
|
||||
not coerced to double). Store the result under `value`. Otherwise store
|
||||
`string(val)`.
|
||||
- **Read** (`decodeValue`): inspect the decoded `value`:
|
||||
- `bson.M` / `bson.D` / `bson.A` (object/array) → recursive `bsonToJSON` →
|
||||
`json.Marshal`-compatible bytes (int64 → integer, double → number).
|
||||
- `string` → `[]byte(v)`.
|
||||
- `bson.Binary` / `[]byte` → legacy fallback (pre-refactor docs).
|
||||
- Map-key ordering is NOT preserved on object re-serialization; acceptable —
|
||||
no byte-exact consumer remains (Phase 1 removed value-bytes CAS; all readers
|
||||
are `GetJSON`/unmarshal or the bare-string `last_date`).
|
||||
|
||||
### Codec decision (RESOLVED — Validation Session 1)
|
||||
<!-- Updated: Validation Session 1 - int64-preserving json.Number codec chosen. -->
|
||||
**Use the int64-preserving `json.Number` codec.** Verification confirmed no
|
||||
persisted integer exceeds 2^53 today (all timestamps are `UnixMilli` ~1.7e12;
|
||||
`CreatedAt int64` = UnixMilli; lolschedule cache `Ts` = UnixMilli), so float64
|
||||
coercion would be safe *now* — but the `json.Number` codec is chosen anyway to
|
||||
future-proof against any later large-int field, for trivial extra cost. Decode
|
||||
with `json.Decoder.UseNumber()`; `jsonToBSON` maps `json.Number` → BSON int64
|
||||
when integral and representable, else double.
|
||||
|
||||
## Related Code Files
|
||||
|
||||
- Modify: `internal/storage/mongodb_kv.go` — `jsonToBSON`/`bsonToJSON` helpers;
|
||||
`doc()` stores native-or-string; `decodeValue` handles object/array/string/
|
||||
binary; `GetVersioned` re-serializes value.
|
||||
- Modify: `internal/storage/mongodb_kv_test.go` — fidelity + native-shape tests.
|
||||
- Modify: `cmd/migrate-dynamo-to-mongo/` — no code change (writes through `Put`,
|
||||
now native); update `main_test.go` assertions if they inspect raw value type.
|
||||
- Modify: `docs/deploy-coolify-selfhosted.md` + the self-host plan's
|
||||
`phase-01` note — value now native BSON, re-migration note.
|
||||
|
||||
## Implementation Steps (TDD)
|
||||
|
||||
1. Codec already chosen (int64-preserving `json.Number`; audit done in
|
||||
validation — no field >2^53). Proceed straight to tests.
|
||||
2. **Tests first** — per-struct round-trip fidelity tests through
|
||||
`PutJSON`→Mongo→`GetJSON` for: coin/gold/stock Portfolio (balances, int qty,
|
||||
timestamps), wordle/loldle/twentyq state, stats counters, lolschedule
|
||||
subscribers (array) + `last_date` (bare string). Assert struct equality AND
|
||||
that the raw stored `value` is a native object/array (not a string) for the
|
||||
JSON cases, and a string for `last_date`. Add a dual-read test seeding a
|
||||
legacy string-value doc. These are red until the impl lands.
|
||||
3. Implement `jsonToBSON`/`bsonToJSON` + wire into `doc()`/`decodeValue`/`GetVersioned`.
|
||||
4. Make red tests green: `make test-mongo`; full `go test ./...`.
|
||||
5. Re-run migrator e2e (DynamoDB Local → Mongo) — values land native, `--verify`
|
||||
counts match, byte round-trip via `Get` still decodes.
|
||||
6. Docs: native representation + re-migration note.
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [ ] Raw stored `value` is a native object/array for JSON values, a string for
|
||||
`last_date`; verified by reading the raw doc in a test.
|
||||
- [ ] Every persisted struct round-trips through `PutJSON`/`GetJSON` with no
|
||||
type/precision loss (fidelity tests green).
|
||||
- [ ] Dual-read test: a seeded legacy string-value doc still decodes.
|
||||
- [ ] Migrator e2e green; values native; `--verify` matches.
|
||||
- [ ] `make vet` + full `go test ./...` (incl. Mongo + DynamoDB) green; 12
|
||||
non-CAS consumers untouched.
|
||||
- [ ] In Atlas, a portfolio document is visibly expandable (manual confirm).
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
- **Int/double fidelity (HIGH)** — see codec decision. Mitigation: int64-
|
||||
preserving `json.Number` codec + per-struct fidelity tests as the gate.
|
||||
- **Map-key reordering on read** — harmless for unmarshal consumers; called out
|
||||
so no future code assumes byte-stable `Get` on objects.
|
||||
- **Legacy docs** — dual-read fallback covers string/binary docs from the prior
|
||||
build; explicit test.
|
||||
- **Re-migration** — if prod data was already migrated as strings, re-run the
|
||||
migrator (idempotent; overwrites to native). Atlas currently fresh → low.
|
||||
@@ -0,0 +1,128 @@
|
||||
---
|
||||
title: Mongo-native value documents + version-field CAS
|
||||
description: >-
|
||||
Store Mongo KV values as native BSON documents (object/array, string fallback)
|
||||
and switch CompareAndSwap to a version-field optimistic lock, keeping the
|
||||
KVStore interface.
|
||||
status: completed
|
||||
priority: P2
|
||||
branch: feature/selfhosted
|
||||
tags:
|
||||
- storage
|
||||
- mongodb
|
||||
- refactor
|
||||
- cas
|
||||
- tdd
|
||||
blockedBy:
|
||||
- 260627-1849-selfhost-coolify-mongodb
|
||||
blocks: []
|
||||
created: '2026-06-28T04:51:16.659Z'
|
||||
createdBy: 'ck:plan'
|
||||
source: skill
|
||||
---
|
||||
|
||||
# Mongo-native value documents + version-field CAS
|
||||
|
||||
## Overview
|
||||
|
||||
Make MongoDB store each KV value as a **native BSON document** (object/array
|
||||
natively; string fallback for non-JSON like the lolschedule date guard) instead
|
||||
of a stringified-JSON blob, so values are expandable and queryable in
|
||||
Atlas/Compass. This requires replacing the value-bytes `CompareAndSwap` with a
|
||||
**version-field optimistic lock** first — byte-exact compare is the only thing
|
||||
that makes native storage impossible. The generic `KVStore` interface
|
||||
(Get/Put/PutJSON/GetJSON/Delete/List) is preserved, so the 12 PutJSON/GetJSON/
|
||||
List consumers stay unchanged; only the 3 CAS callers (coin, gold, lolschedule)
|
||||
and the storage layer change.
|
||||
|
||||
Approved design: `plans/reports/brainstorm-260628-1113-mongo-native-documents-report.md` (Option A).
|
||||
|
||||
Sequencing rationale: Phase 1 (version CAS) is representation-neutral and
|
||||
independently shippable — it de-risks the concurrency change while values are
|
||||
still strings. Phase 2 flips the representation to native on top of the
|
||||
already-safe version lock.
|
||||
|
||||
## Phases
|
||||
|
||||
| Phase | Name | Status |
|
||||
|-------|------|--------|
|
||||
| 1 | [Version-Field Optimistic Locking](./phase-01-version-field-optimistic-locking.md) | Completed |
|
||||
| 2 | [Native BSON Value Representation](./phase-02-native-bson-value-representation.md) | Completed |
|
||||
|
||||
## Dependencies
|
||||
|
||||
- **blockedBy `260627-1849-selfhost-coolify-mongodb`** — that plan created the
|
||||
`mongodb` KVProvider, the migrator, and the deployed self-host runtime this
|
||||
refactor modifies. Its code is already committed on `feature/selfhosted`.
|
||||
- Phase 2 depends on Phase 1 (native storage requires version CAS, not
|
||||
value-bytes CAS).
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- [ ] In Atlas/Compass, a coin/gold/stock portfolio and a game-state value
|
||||
render as **native, expandable BSON documents** (not a quoted JSON string).
|
||||
- [ ] Non-JSON values (lolschedule `daily_push:last_date`) stored as a plain
|
||||
BSON string; round-trip byte-exact.
|
||||
- [ ] CAS is version-based: concurrent updates yield exactly one winner, losers
|
||||
get `ErrConflict` (proven by the existing-style concurrent test against real
|
||||
Mongo, plus memory).
|
||||
- [ ] coin/gold/stock portfolios + each game-state struct round-trip through
|
||||
`PutJSON`/`GetJSON` with **no numeric precision/type loss** (TDD fidelity
|
||||
tests; explicit check that no int field exceeds 2^53 or the codec preserves
|
||||
int64).
|
||||
- [ ] The 12 PutJSON/GetJSON/List-only consumers compile and pass unchanged.
|
||||
- [ ] memory backend + hermetic `go test ./...` (no DB) still pass; full suite
|
||||
(incl. Mongo + DynamoDB integration) green.
|
||||
- [ ] Migrator writes native documents; `--verify` still matches per-module
|
||||
counts; re-migration path documented (Atlas fresh → low impact now).
|
||||
- [ ] Dual-read fallback: documents written by the previous string/binary build
|
||||
still read correctly.
|
||||
|
||||
## Risks (carried into phases)
|
||||
|
||||
1. **JSON↔BSON int/double fidelity (HIGH)** — `json.Unmarshal` to a generic
|
||||
coerces all numbers to float64 → BSON double; large int64 (>2^53) would lose
|
||||
precision. Phase 2 step 1 audits struct numeric fields and picks the codec
|
||||
(accept float64 if all safe, else a `json.Number`-preserving decode). Gated
|
||||
by TDD fidelity tests.
|
||||
2. **CAS contract change ripple** — `CompareAndSwapStore` → `VersionedStore`
|
||||
(memory + mongodb implement; dynamodb drops CAS; firestore backend deleted)
|
||||
+ 3 callers + tests. Phase 1 isolates this.
|
||||
3. **Re-migration** — migrator output representation changes; document re-run
|
||||
(Atlas currently fresh).
|
||||
4. **Legacy version-less docs (HIGH, live rollout)** — docs from the deployed
|
||||
build have no `version` field; Phase 1 treats absent version as v0 +
|
||||
match-missing-or-equal so existing users' updates don't fail.
|
||||
|
||||
## Open Questions
|
||||
|
||||
None — all resolved in the Validation Log below.
|
||||
|
||||
## Validation Log
|
||||
|
||||
### Session 1 — 2026-06-28
|
||||
|
||||
**Verification pass (Light tier, Fact Checker, 2 phases):** Claims checked
|
||||
against code. Key result — **no persisted integer exceeds 2^53**: all timestamps
|
||||
are `UnixMilli` (~1.7e12) / `Unix()` seconds; `coin|gold.Portfolio.Meta.CreatedAt int64`
|
||||
= UnixMilli; lolschedule cache `Ts` = UnixMilli. CAS interface
|
||||
(`CompareAndSwapStore`), its 3 callers, and the backend files confirmed present.
|
||||
Failures: 0.
|
||||
|
||||
| # | Question | Decision | Affects |
|
||||
|---|----------|----------|---------|
|
||||
| 1 | JSON↔BSON codec | **int64-preserving `json.Number`.** float64 is safe today (no int >2^53) but json.Number future-proofs cheaply. | Completed |
|
||||
| 2 | Versioned CAS backend scope | **memory + mongodb only.** Drop the **firestore backend entirely** (legacy/unused); **dynamodb** keeps base `KVStore` only (migrate-only; drop its CAS). | Completed |
|
||||
| 3 | Pre-existing version-less docs | **Treat absent `version` as v0, match missing-or-equal (upsert).** No backfill; live docs keep working through the Phase 1 deploy. | Phase 1 |
|
||||
|
||||
Consequence surfaced: dropping firestore requires relocating the shared helpers
|
||||
it hosts (`validateKey`/`validatePrefix`/`prefixSuccessor`/`collectionNameRe`)
|
||||
to a backend-neutral file — added to Phase 1.
|
||||
|
||||
### Whole-Plan Consistency Sweep
|
||||
Re-read `plan.md` + both phase files after propagation. Reconciled: backend
|
||||
scope is now "memory + mongodb implement versioned CAS; dynamodb base-only;
|
||||
firestore deleted" consistently in plan risks, Phase 1 architecture/files/steps/
|
||||
criteria. Codec is "int64-preserving json.Number" in plan + Phase 2. Version-less
|
||||
doc handling consistent (Phase 1 architecture + risk + criteria). No unresolved
|
||||
contradictions.
|
||||
@@ -0,0 +1,89 @@
|
||||
# Brainstorm: Mongo-native documents + version-field CAS
|
||||
|
||||
**Date:** 2026-06-28
|
||||
**Branch:** feature/selfhosted
|
||||
**Modes:** (none)
|
||||
**Outcome:** Approved — Option A (native value documents, keep KVStore interface, switch CAS to version field).
|
||||
|
||||
## Problem statement
|
||||
|
||||
Current Mongo storage wraps each value as a stringified-JSON blob under `value`
|
||||
(`{ _id, value: "<json>", updatedAt }`). Readable but not native — can't expand
|
||||
or query inside values in Atlas/Compass. User wants real, native, viewable
|
||||
documents. Initial ask ("use Mongo directly, drop the KV interface") was
|
||||
inverted (problem-first): the underlying goal is the native-document
|
||||
*representation*, not removing the abstraction.
|
||||
|
||||
## Approaches evaluated
|
||||
|
||||
| Option | Summary | Verdict |
|
||||
|---|---|---|
|
||||
| **A. Native value + version CAS, keep interface** | Store value as native BSON object/array (string fallback for non-JSON). Switch CAS from value-bytes compare to a `version` field. `PutJSON`/`GetJSON`/`List` unchanged → 12 consumers untouched. | **Chosen** — ~80% of the benefit, contained cost |
|
||||
| B. Typed per-module Mongo repositories | Real typed collections per module + thin interface seam. Idiomatic, fully queryable. | Rejected — big effort (~10 modules), not justified by current needs |
|
||||
| C. Full Mongo-direct, no abstraction | Handlers call `*mongo.Collection`. | Rejected — loses memory/firestore/dynamodb, kills hermetic tests + no-DB local dev, needs Mongo in CI, largest rewrite |
|
||||
| Keep current KV | Leave string-blob design as-is. | Rejected by user (wants native docs) |
|
||||
|
||||
Why not B/C: the KVStore seam is load-bearing — it gives hermetic millisecond
|
||||
tests (memory backend, no DB) and the portability that made the AWS→Mongo
|
||||
migration cheap. "No interface" is partly a mirage in Go: testable persistence
|
||||
needs *some* seam, so removal just relocates it while sacrificing dev ergonomics.
|
||||
|
||||
## Chosen design (Option A)
|
||||
|
||||
- **Value representation (Mongo):** JSON object → native BSON object; JSON array
|
||||
→ native array; bare/non-JSON (e.g. lolschedule `last_date` string) → BSON
|
||||
string fallback. `Get`/`GetJSON` reconstruct `[]byte` from the native form.
|
||||
Keep dual-read fallback for existing string/binary docs.
|
||||
- **CAS → version field.** Replace value-bytes compare with load-value+version,
|
||||
swap-if-version-unchanged (standard optimistic lock). This removes the
|
||||
byte-exactness coupling that previously blocked native storage.
|
||||
- **Interface:** bulk `KVStore` (Get/Put/PutJSON/GetJSON/Delete/List) signatures
|
||||
unchanged. Only the `CompareAndSwapStore` contract changes shape.
|
||||
|
||||
## Blast radius (verified against consumers)
|
||||
|
||||
- **Unchanged (12):** all `PutJSON`/`GetJSON`/`List`-only modules — conversion
|
||||
hides inside `Put`/`Get`.
|
||||
- **Changed (CAS, 3):** coin/portfolio.go, gold/portfolio.go, lolschedule/cron.go
|
||||
(the last-push claim).
|
||||
- **Storage layer:** `internal/storage/mongodb_kv.go` (native encode/decode +
|
||||
version), `CompareAndSwapStore` contract + 4 backend impls (memory + mongo
|
||||
live; dynamodb/firestore for parity), memory CAS.
|
||||
- **Migrator:** writes native now; re-run needed if data already migrated (Atlas
|
||||
currently fresh → fine).
|
||||
|
||||
## Risks
|
||||
|
||||
1. **JSON↔BSON type fidelity (HIGH).** Native round-trip can drift int vs double
|
||||
(struct `int64` field returning as double). App data (float balances,
|
||||
in-range int timestamps < 2^53) is likely safe, but the plan MUST prove
|
||||
per-struct round-trip with tests or use a type-preserving codec. This is the
|
||||
real cost of leaving byte-blobs.
|
||||
2. **CAS contract change** ripples to 4 backends + 3 callers + tests.
|
||||
3. **Re-migration** of any already-migrated data (none yet → low now).
|
||||
4. **Get-raw byte fidelity:** validated no consumer needs byte-identical `Get`
|
||||
of a JSON object (CAS path is being refactored; only bare-string `last_date`
|
||||
uses raw Get, and strings round-trip exact).
|
||||
|
||||
## Success criteria
|
||||
|
||||
- Values appear as native, expandable documents in Atlas/Compass.
|
||||
- coin/gold/stock portfolios + game state round-trip through typed structs with
|
||||
no precision/type loss (tested).
|
||||
- Concurrent CAS still single-winner (version-based); coin/gold concurrency tests
|
||||
pass.
|
||||
- memory backend + hermetic `go test ./...` (no DB) still work.
|
||||
- 12 PutJSON/GetJSON/List consumers compile + pass unchanged.
|
||||
|
||||
## Recommended next step
|
||||
|
||||
`/ck:plan --tdd` — this refactors critical, tested money-path code (portfolios +
|
||||
concurrency). Lock current behavior with tests first, then change representation
|
||||
underneath.
|
||||
|
||||
## Unresolved questions
|
||||
|
||||
- Exact JSON↔BSON codec choice (driver extJSON vs custom typed decode) — decide
|
||||
in plan after a fidelity spike on the real structs.
|
||||
- Whether to keep CAS on dynamodb/firestore (parity) or drop it there (only
|
||||
memory + mongo are live) — plan decision.
|
||||
Reference in New Issue
Block a user