HarperFast / HarperFast/harper
Table.create() existence check not enforced at commit — concurrent creates silently become last-write-wins (and the with-id form is undocumented)
- Dominant language
- JavaScript
- Stars
- 89
- Forks
- 10
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 200
Description
## Summary
`Table.create()` implements insert-if-absent semantics — an explicit-primary-key create throws `ClientError('Record already exists', 409)` — but the existence check is enforced only against the pre-staging snapshot, not at commit time. Under concurrency, two `create()`s of the same key can **both report success**, with the loser silently degrading to a last-write-wins put. Separately, this form of `create()` is undocumented (the docs describe only the auto-generated-id form), so users can't discover the primitive at all.
## Trace (implementation gap)
1. `resources/Table.ts` (`create`, ~line 1783): explicit-id create does `primaryStore.getSync(id)` → throws 409 if a record exists → stages the write via `_writeUpdate(id, record, true)`. The 409 check happens **only here, pre-staging**.
2. The staged write's `commit` closure (`_writeUpdate`, ~line 1896) uses optimistic locking: on storage-level write-write conflict (`ERR_BUSY` from the RocksDB optimistic transaction, or the LMDB `ifVersion` equivalent), `resources/DatabaseTransaction.ts` (~line 364) increments `retries` and re-runs `save()` → `operation.commit(txnTime, freshEntry, retry=true, txn)` with the entry **re-read fresh**.
3. On retry, the commit closure re-stamps timestamps and proceeds as a normal put — nothing re-applies the create-specific "reject if exists" semantic, even though `existingEntry.value` is now populated by the winning create.
Net: concurrent `create(sameKey)` → both pass `getSync` (both absent at read time) → both stage → loser conflicts, retries, overwrites → **both callers resolve successfully**. The 409 only fires when the winner's write is already committed before the loser's `getSync` — i.e. sequential presentations.
The fix appears well-scoped: the conflict-detection, fresh-entry re-read, and `retry` flag machinery all exist — a create-typed write's retry path could throw the same 409 when `existingEntry?.value` is present instead of proceeding.
## Documentation gap
`reference/resources/resource-api.md` documents `create(record)` as only "Creates a new record with an auto-generated primary key … **Do not include a primary key in the `record` argument**." The explicit-id, 409-on-exists form is implemented but undocumented — which reads as "Harper has no insert-if-absent primitive." If the with-id form is intended API, it should be documented (including its concurrency semantics once decided); if not, `create()` should reject explicit ids rather than half-support them.
## Motivating use case
The oauth plugin's RFC 7523 client-assertion replay guard (`jti` single-use enforcement; HarperFast/oauth#165, discussion with @kriszyp) needs per-node atomic insert-if-absent. `create()` + catch-409 is the natural shape and the plugin is adopting it — today it narrows the race window (staging→commit instead of an awaited get→put), and it becomes fully correct per-node automatically if the retry path enforces the 409. Cross-node behavior under async replication is understood and out of scope here.
## Asks
1. Decide whether explicit-id `create()` is blessed API (@kriszyp).
2. If yes: enforce the 409 at commit time (retry path) so `create()` is a true per-node insert-if-absent, and document the with-id form + its concurrency semantics.
3. If no: make `create()` reject explicit primary keys, and document what (if anything) is the supported insert-if-absent primitive.
🤖 Filed by Claude (Fable 5) on Nathan's behalf, from the oauth#165 review discussion.
Contributor guide
Research direction
Start with resources/Table.ts around create and _writeUpdate, then read resources/DatabaseTransaction.ts around the retry and fresh-entry flow. Confirm the intended explicit-id API, trace the existing create and conflict behavior, and inspect reference/resources/resource-api.md. Done means the API decision is recorded, concurrent behavior is covered, and the documentation matches the supported form and semantics.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- nodejs, typescript
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100