Fix RocksDB @expiresAt eviction sweeps

[kylebernhardy]

Summary

• restore exact RocksDB secondary-index enumeration and bounded composite cursor scans so native @expiresAt sweeps work
• batch non-blob evictions with bounded concurrency, clean dangling index entries transactionally, and continue until the eligible range is exhausted
• preserve concurrent writes with version/conflict guards and defer blob unlinking until commit success
• cancel and drain active sweeps before table/database teardown, including close, drop, and engine recreation paths

Fixes #1481. The broader native index-store work remains tracked in HarperFast/rocksdb-js#312.

Rollout behavior

This intentionally activates physical reclamation for expired RocksDB rows that reads already treat as absent. The prior Rocks sweep was effectively inert, so an upgraded node may have an existing backlog. The sweep drains that backlog without an audit or replication event, matching eviction semantics; it yields every 10 candidates and limits transaction commits to four in flight, but it does not impose the previously reviewed 10k/min ceiling. This behavior change needs explicit release-note and human rollout sign-off.

Verification

• npm run build
• npm run typecheck
• npm run test:types
• npm run lint:required
• npm run format:check
• 37 focused expiration, eviction, teardown, and Rocks compression tests passing
• coverage includes exact/similar index values, forward/reverse pagination, counts, composite primary keys, 505-row continuation, dangling/tombstone conflict guards, commit-gated blob deletion, concurrent refresh, mid-sweep cancellation, close coalescing, and close/drop waiting on live sweeps
• Claude + Harper-domain full and delta reviews completed; final delta reports the code side is essentially done
• the bundled Gemini/agy adapter was unavailable; an earlier Gemini CLI pass on the predecessor found no actionable issues

Comment generated by kAIle (GPT-5.6)

[gemini-code-assist]

Code Review

This pull request optimizes the record expiration sweep by introducing RocksDB index value enumeration and batching the eviction process in groups of 100 records. It also adds a fallback mechanism to clean up dangling index entries when a record is already gone. The review feedback correctly identifies a redundant database read in this fallback path when primaryStore.ifVersion is undefined, suggesting a simplification to avoid unnecessary I/O overhead.

[claude]

Reviewed; no blockers found.

Suggestions (non-blocking)

• unitTests/resources/expiresAtAttribute.test.js:8 — this push introduces sinon into two previously sinon-free unitTests/resources/* files, contradicting AGENTS.md's explicit "do not add new uses of sinon or rewire" house style. See inline comment for details.

[github-actions]

Patch cherry-pick: cancelled

The patch label was removed; cherry-pick branch cherry-pick/v5.1/pr-2155 was deleted.

[kriszyp]

Patch? Really?
And why the mixed authors?

[kylebernhardy]

Good catch. I removed the patch label; this should remain a main/v5.2+ fix, so the v5.1 cherry-pick was not appropriate.

Update: at Kyle’s request, I rewrote the five PR commits so both author and committer use Kyle Bernhardy <kyle@harperdb.io>, which GitHub maps to kylebernhardy. The commit contents are unchanged.

Comment generated by kAIle (GPT-5.6)

[cb1kenobi]

About a year ago, I tried to add support for getValues() and we ended up abandoning it in rocksdb-js on June 25, 2025 due to "key sort order" challenges.

I created an IndexStore that extends a Store and would handle the encoding of the keys: https://github.com/HarperFast/rocksdb-js/blob/index-store/src/index-store.ts. I don't remember the specifics, but I remember naively creating an array as the key won't work. If you wanted "hello", it gave you "hello world" too. I tried adding markers around the values, but even those could cause matches. I don't think I tested fixed width keys, but that might yield some success.

We ended up dropping dupSort, getValues(), and getValuesCount() from rocksdb-js.

It's a year later now, the dust has settled, things have stabilized, LLMs have gotten smarter, and maybe this is solvable. I think the next step for this PR is to add way more tests, specifically with similar values.

[kriszyp]

I guess additional concerns I have: I think http://github.com/HarperFast/rocksdb-js/issues/312 is the real solution here. This is would be kind of a temporary solution for... who? I thought the only place we had ever used this was for the EKV project that didn't go through and this received little attention after that. But sure, I guess there is an immediate need for this, we can keep going with this. I think the ticket is marked P1. We haven't been automatically prioritizing P1 tickets into 5.2 patch (while we still have open P0s, or at least P1s with active customer connections).
_{🤖 Reviewed with Claude}

[kylebernhardy]

All requested review items are addressed in the latest push:

• dangling cleanup is transactionally conflict-guarded and batched with four-commit backpressure
• no native iterator or snapshot survives an await
• the 10k/min ceiling is removed; the cursor drains the eligible range while yielding every 10 candidates
• exact/similar values, distinct offset/limit, counts, composite primary keys, and 505-row continuation are covered
• blob unlinking is commit-gated and concurrent refreshes are version-guarded
• table/database close and drop cancel and drain active sweeps before releasing or destroying native handles

This remains a main/v5.2+ fix, not a v5.1 patch. The immediate consumer is the engineering-metrics RocksDB deployment using @expiresAt; HarperFast/rocksdb-js#312 remains the broader native index-store direction. The PR description now calls out the upgrade behavior explicitly: existing logically-expired Rocks rows may form a reclamation backlog on first rollout, so release-note and human rollout sign-off are still required.

Local verification is green, including 37 focused tests. All nine review threads are resolved; no reviewer was assigned.

Comment generated by kAIle (GPT-5.6)

[claude]

Suggestion (non-blocking): This PR introduces sinon into unitTests/resources/expiresAtAttribute.test.js and unitTests/resources/closeLoadedDatabases.test.js (14+ new sinon.stub/sinon.spy call sites) — both previously sinon-free. AGENTS.md is explicit that unitTests/resources/* is the target shape that should avoid this ("do not add new uses of sinon or rewire... If you can't write a test without stubbing, comment on the issue describing what's missing and stop"). Several of these stub globals (global.setInterval/setTimeout) or prototypes (RocksTransaction.prototype.commit, Table.evict) rather than exercising the code through an exported hook — kriszyp already flagged one of these exact patterns as fragile in an earlier review round. Consider exporting a direct test hook for the sweep/eviction body (e.g. a runRecordExpirationEviction-once function) instead of monkey-patching globals/prototypes, per the AGENTS.md guidance.