Repository navigation
feat(kvdb): expose read-only lmdb transactions - #25631
Draft
spalladino wants to merge 4 commits into
Draft
spalladino wants to merge 4 commits into
spalladino wants to merge 4 commits into
Conversation
Adds START_READ_TX / CLOSE_READ_TX to the kvdb addon protocol and an optional txId on GET and START_CURSOR, so a client can hold a consistent LMDB snapshot open across many reads without blocking writers. The wrapper keeps a registry of open read transactions, each with a mutex that serializes its use across the libuv worker pool; cursors opened against a shared transaction (including cursor creation) take the same mutex. Closing a read transaction only unregisters it: cursors opened against it keep it alive until they are closed. CLOSE releases every registered read transaction. GET / START_CURSOR with an unknown txId fail; CLOSE_READ_TX on an unknown id returns ok=false. lmdblib::LMDBStore::get gains an overload that reads through a caller-provided read transaction. The new message types are appended to the enum and txId is optional, so existing clients keep working unchanged.
Each transaction factory took its reader or writer permit before constructing the transaction, and only the transaction's destructor gave it back. If mdb_txn_begin threw (e.g. MDB_READERS_FULL because another process holds the reader table), the permit leaked, and repeated failures eventually left every later reader or writer waiting forever. The factories now release the permit when construction throws. Also adds try_create_shared_read_transaction, which takes a reader permit without waiting and can be told to leave some permits free.
START_READ_TX waited for a reader permit on a libuv worker while holding the dispatcher's shared lock. With every slot held by open read transactions, a few such requests parked every worker, so the CLOSE_READ_TX that would free a slot never ran and CLOSE could never take the dispatcher's exclusive lock. START_READ_TX now fails immediately when no permit is available, and it never takes the last one, so requests that open their own short-lived read transaction (GET/HAS/STATS and cursors without txId) can always make progress while long-lived read transactions are held. CLOSE_READ_TX now rejects a request without a tx id instead of reading an uninitialized integer.
Adds node:test coverage that loads the built addon and talks to it over msgpack the way the kv-store client does: requests without txId, snapshot gets and cursor scans, concurrent use of one read transaction, cursors outliving CLOSE_READ_TX, unknown and missing ids, reader slot exhaustion and recovery, and CLOSE with live read transactions. Wired into CI as kvdb-tests.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Part of A-1817. Redoes the native half of #25280, which was closed after the repo split. The TypeScript client lives in aztec-node, where
readOnlyTransactioncurrently delegates to the write transaction; A-2377 wires it to this once released.LMDB supports many concurrent readers that never block the writer, but the kvdb addon only gave the client per-message throwaway read transactions, so two consecutive reads could straddle a commit, and consistent reads had to go through the single writer queue.
Approach
START_READ_TX = 111(no body, returns{ tx }) andCLOSE_READ_TX = 112({ tx }returns{ ok }, false for unknown ids), plus an optionaltxIdonGETandSTART_CURSORthat reads through the registered transaction's snapshot. Existing message numbers are unchanged and requests withouttxIdbehave as before.CLOSE_READ_TXuntil they close.CLOSEreleases all of them.START_READ_TXnever waits for a reader slot: it rejects immediately when opening would take the last free slot, so clients can't deadlock the store by holding every slot, and plainGET/HAS/cursors always have one left.lmdblib::LMDBStore::getgains an overload that reads through a caller-provided read transaction.mdb_txn_begin(e.g.MDB_READERS_FULL) leaked the permit forever.Testing
mdb_txn_beginfails with a realMDB_READERS_FULLfrom a forked child holding the reader table.node:testsuite (ts/test/read_tx.test.mjs, wired askvdb-tests) that drives the built addon over msgpack: requests withouttxId, snapshot gets and cursors, ~600 concurrent operations on one transaction with interleaved writes, cursors outlivingCLOSE_READ_TX, unknown/missing ids, slot exhaustion rejecting and recovering, andCLOSEwith live transactions.Wire notes for the client
txIdasnullor omit it; neverundefined(msgpackr encodes it as a fixext the native side can't decode).START_READ_TXrejection as "back off or fall back to plain reads".