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)

Open
#1,745 1 comment 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.