Disable snapshots for get_analytics queries

[cap10morgan]

Disables read snapshots for get_analytics queries so long-running analytics scans are easier on the rest of the system — a scan no longer pins a consistent read snapshot, which would otherwise block storage compaction for its duration.

Note: reimplemented on top of current main. The original one-liner conflicted (the file was refactored) and, as Kris noted, was a no-op because snapshot: false was never plumbed past the request object. See the discussion below for the design alignment.

What snapshot: false means

A search request can now set snapshot: false to read against the latest committed data without holding a consistent read snapshot open for the whole iteration. The trade-off is consistency: rows written while the query iterates may be observed. Default remains true (a stable snapshot is held).

How it's wired

The backends expose snapshot control differently, so the flag is honored at the layer that actually supports it:

• RocksDB (the engine backing hdb_analytics): the read transaction is created with { disableSnapshot: true } in DatabaseTransaction.getReadTxn. Passing that transaction down as the normal read txn is sufficient — no getRange option is needed, which sidesteps the lmdb "can't disable snapshot on a dupSort store" throw entirely (every secondary index is dupSort).
• LMDB: has no per-transaction snapshot toggle (its only lever is getRange({ snapshot: false }), which throws on dupSort stores). The flag is accepted for parity but is a documented no-op; LMDB-backed reads keep a snapshot for now.

Changes

• RequestTarget — declares snapshot?: boolean (default true).
• Table.search — txn.useReadTxn(target.snapshot === false).
• DatabaseTransaction.getReadTxn / useReadTxn (RocksDB) — build the read txn with { disableSnapshot: true } when requested.
• LMDBTransaction.useReadTxn — accepts the flag, no-ops it (commented).
• resources/analytics/read.ts get() — requests snapshot: false.
• unitTests/resources/snapshotFalse.test.js — regression test (primary-key, dupSort secondary-index, and combined scans).
• resources/DESIGN.md — documents the read-snapshot behavior.

Testing

• Clean typecheck, prettier, oxlint.
• 84 analytics unit tests pass.
• New regression test green on both RocksDB and LMDB.

Docs

Companion docs PR: HarperFast/documentation#557 (adds a "Read behavior" note to the get_analytics reference).

[kriszyp]

I guess I didn't explain this well: this isn't implemented for Table.search (yet). It is an option available for lmdb-js's getRange() method (and will be in rocksdb-js as well). So we would need to pass it through to there. I suppose it might make the most sense to add this as a property on the context, which is accessible by searchByIndex where the getRange usually takes place. But snapshot: false implies that we aren't keeping the read transaction, and the transaction is also kept open for the duration of the iteration with the useReadTxn method (until doneReadTxn). So we would probably want to turn that off as well in this scenario.

[cap10morgan]

I guess I didn't explain this well: this isn't implemented for Table.search (yet). It is an option available for lmdb-js's getRange() method (and will be in rocksdb-js as well). So we would need to pass it through to there. I suppose it might make the most sense to add this as a property on the context, which is accessible by searchByIndex where the getRange usually takes place. But snapshot: false implies that we aren't keeping the read transaction, and the transaction is also kept open for the duration of the iteration with the useReadTxn method (until doneReadTxn). So we would probably want to turn that off as well in this scenario.

Want to see if I understand. You're saying that we should:

1. Make get_analytics call searchByIndex directly
2. Modify searchByIndex to look for a snapshot: false in the context and, if present:
   1. Pass that down to getRange()'s arg
   2. Close the read txn right away (by just calling doneReadTxn?)

[dawsontoth]

Claude's take on picking this up: The one-liner now conflicts with main (this file was refactored) and, as Kris noted, snapshot: false currently does nothing — so this is a reimplementation. Digging into the engine turned up a wrinkle worth aligning on before I touch core code: the two backends expose snapshot control differently.

Findings

• hdb_analytics is RocksDB-backed. rocksdb-js doesn't accept snapshot on getRange — it's a transaction-level option (new Transaction(store, { disableSnapshot: true })). So passing snapshot:false into searchByIndex→getRange is a silent no-op for analytics. The benefit has to come from how the read txn is created in DatabaseTransaction.getReadTxn.
• lmdb-js does take getRange({ snapshot:false }), but it throws "Can not disable snapshot on a dupSort data store" when values are included (read.js:531), and we open every secondary index dupSort (databases.ts:605). So a raw pass-through breaks secondary-index queries on LMDB tables and needs a guard.
• Not holding the read txn open for the whole iteration (your point about useReadTxn/doneReadTxn) is backend-agnostic and is where most of the "easier on the rest of the system" win comes from for a long scan.

Proposed plan

1. Add snapshot?: boolean to RequestTarget (default true; analytics get passes false).
2. Table.search: when snapshot === false, don't pin the read txn across iteration (skip the useReadTxn refcount hold / doneReadTxn in onDone), and acquire the read txn in snapshot-disabled mode.
3. DatabaseTransaction.getReadTxn (Rocks): create the RocksTransaction with { disableSnapshot: true } for this mode.
4. LMDBTransaction: use a renewing/non-pinned read txn; thread snapshot:false to getRange only where it won't hit the dupSort+values throw (i.e. guard it).
5. executeConditions/searchByIndex: thread the flag down to rangeOptions, dupSort-guarded.

Questions for you, Kris:

• Happy with the request-property approach, or do you still prefer hanging it off the context?
• For the Rocks path, is disableSnapshot: true on the read transaction the right lever, or did you have getRange-level support landing in rocksdb-js soon that I should target instead?
• Any concern with leaving the LMDB getRange path dupSort-guarded (so it silently keeps a snapshot for dupSort indexes rather than throwing)?

[dawsontoth]

• we should document this too

[dawsontoth]

@cb1kenobi mind weighing in on this PR too? Specifically the ideas in my last comment around RocksDB.

[dawsontoth]

Pushed the implementation (force-pushed this branch onto current main, since the original one-liner conflicted and was a no-op).

One deviation from the plan above, in a good way. Plan steps 4–5 had us threading snapshot: false down to getRange with a dupSort guard. While implementing, I found that wasn't necessary: for the RocksDB path, creating the read transaction with { disableSnapshot: true } and passing it down as the normal read transaction is enough — no getRange option required. That sidesteps the lmdb dupSort throw entirely (it never enters the picture), so there's no guard to get wrong.

Net result vs. the three questions:

• Request property vs. context: went with the request property (snapshot?: boolean on RequestTarget), consistent with how allowFullScan / allowConditionsOnDynamicAttributes already flow.
• Rocks lever: disableSnapshot: true on the read transaction, created in DatabaseTransaction.getReadTxn. This is the only place the flag does real work.
• lmdb dupSort: moot — we never pass snapshot: false to getRange. LMDB-backed reads keep a snapshot for now (the flag is a documented no-op there); RocksDB-backed tables (incl. hdb_analytics) get the real behavior.

What changed

• RequestTarget — declares snapshot? (default true).
• Table.search — txn.useReadTxn(target.snapshot === false).
• DatabaseTransaction.getReadTxn (Rocks) — builds the read txn with { disableSnapshot: true } when requested.
• LMDBTransaction.useReadTxn — accepts the flag, no-ops it (commented).
• analytics/read.ts get() — requests snapshot: false.
• Regression test (unitTests/resources/snapshotFalse.test.js) covering primary-key, dupSort secondary-index, and combined scans; passes on both RocksDB and LMDB. Behavior documented in resources/DESIGN.md.

Verification: clean typecheck / prettier / oxlint; 84 analytics unit tests pass; new test green on both engines.

Docs: HarperFast/documentation#557 adds a "Read behavior" note to the get_analytics reference.

[kriszyp]

Approving — nice perf win: dropping the read snapshot stops a long hdb_analytics scan from pinning an MVCC snapshot that blocks RocksDB compaction. Zero default-path cost (opt-in boolean, default takes the identical path), scoped to the single read-only get_analytics site, and torn reads are fine for approximate analytics. Regression test included. Thanks @cap10morgan!

— 🤖 KrAIs (Kris's review assistant)

[dawsontoth]

I'm assuming Studio is the main consumer of get_analytics, and it'll be OK if we like... read a deleted key or something. We can deal with any explosions down the line.

[kriszyp]

Approving — real perf win: dropping the read snapshot stops a long hdb_analytics scan from pinning an MVCC snapshot that blocks RocksDB compaction. Opt-in with zero default-path cost, read-only scope, regression test included; the latest push is a cosmetic collapse to a direct {disableSnapshot} pass-through. Thanks @cap10morgan!

— 🤖 KrAIs (Kris's review assistant)