Skip to content

hash_migrate: server-side DB writes on the read-only handle; failures logged as collisions; ghost duplicates share a hash #215

Closed

Description

@dborup

Relates to #202, #209

Problem

cmd/server/hash_migrate.go (migrateContentHashesAsync) issues UPDATE/DELETE statements from the server process. The server opens SQLite with mode=ro, so this breaks the read/write separation invariant (Kpa-clawbot#1283). The current readonly_invariant_test.go does not catch it, because it only asserts three method names.

Found during the #209 review, using reviewer-only probes against master and the #209 merged tree:

  1. Every write fails on the production handle. With OpenDB (mode=ro), two stale-hash duplicates, and the migration run, every UPDATE fails.
    • Each failure is logged as collides — merging duplicate, which is misleading.
    • The DB is left unchanged, with both rows and the old hash, so the migration repeats on every server start.
  2. Ghost duplicates in memory. Both transmissions take the new content hash in memory and both stay in s.packets. byHash points to only one of them, and QueryPackets returns both.
  3. Shared nodeHashes entry. nodeHashes is keyed by hash. Evicting one ghost removes the shared nodeHashes[pk][hash] entry that the surviving transmission still relies on. This is inferred from the code, not probed.
  4. Stale nodeHashes keys. Keys created before a migration are never removed. After both duplicates are evicted, byNode is empty, but nodeHashes still holds the relay keys under the old hashes. This applies to every byNode source, and it is bounded by migrated transmissions × relays.
  5. Stale chargedBytes. After migration, chargedBytes is off by the hash length difference. It is credited consistently, so there is no drift.
  6. Untested constant. perFallbackRelayBytes (added in fix(store): charge the path_json fallback and pin the hash-migration accounting (#202) #209) is not pinned by any default-run test. The mutant perFallbackRelayBytes = 0 survives every _202/_113 test, and the heap comparison that would catch it is env-gated (CORESCOPE_PERF_202).

Proposed direction

  • Move the DB part of the content-hash migration (rehash, collision merge, duplicate delete) to cmd/ingestor, like every other write.
  • The server should only rehash in memory. On a collision, it should merge in memory: move the duplicate's observations to the survivor, then remove the duplicate from s.packets, byTxID, byHash and the node indexes, crediting each charge exactly once.
  • Re-key or remove the nodeHashes entries under the old hash during migration.
  • Extend readonly_invariant_test.go, or add a test, so that a write statement from cmd/server fails the build or a test. One option is a probe that runs the migration on a mode=ro handle and asserts that no write is attempted.
  • Add a default-run bound for perFallbackRelayBytes.

Acceptance

  • No UPDATE/DELETE/INSERT from cmd/server for the hash migration, and a test enforces it.
  • Migration on a DB with stale-hash duplicates converges once (in the ingestor) and is not repeated on the next start.
  • In memory, migrated duplicates leave exactly one transmission per hash, with all observations, and QueryPackets cardinality is correct.
  • After migration and eviction, byNode, nodeHashes and trackedBytes all return to the baseline exactly, without relying on the clamp.
  • StoreTx stays at 320 bytes. No new map[string]interface{}.
  • A test pins perFallbackRelayBytes, so the = 0 mutant is killed.

Activity

  1. dborup commented on Oct 4, 2026

    @dborup
    OwnerAuthor

    Fixed by #222 (merged as 97542417).

    • The server no longer writes to the DB for the hash migration. A recording-driver probe on the mode=ro handle, a source guard and the write-SQL ratchet now enforce this.

    • The ingestor runs a one-time migration, content_hash_formula_v1. It rehashes in id-ordered batches under writerMu. When two rows collide:

      • the lowest id survives;
      • observations move to it, and dedup-identical ones are dropped;
      • first_seen takes the MIN and last_seen the MAX;
      • the route_mask values are unioned;
      • allow-listed nullable columns are filled with COALESCE;
      • ping_triggers follows the survivor.

      A cancelled run resumes to the same state and is logged as cancelled (will resume).

    • The server merges duplicates in memory over the whole store after the startup load. Each charge is credited once, and nodeHashes is re-keyed through the transmission. The run logs its max write-lock hold per batch.

    • perFallbackRelayBytes is now pinned by a default-run test.

    The deploy plan is in the PR description:

    1. Take a snapshot.
    2. Wait for _async_migrations content_hash_formula_v1 = done.
    3. Restart the server program. This step is required: receptions on rows that were merged away are only picked up after a reload.
    4. Run the verification SQL.

    Remaining, documented in #222:

    • The survivor can lie outside the server's retention window. This is rare.
    • Scope fill differs slightly between memory and the DB until the restart.
    • The write-lock hold of a merging batch grows with the store. It will be measured on staging, and a follow-up issue will be opened only if that shows a problem.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions