Skip to content

Table.create() existence check not enforced at commit — concurrent creates silently become last-write-wins (and the with-id form is undocumented) #1745

Description

@heskew

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions