Repository navigation
Conversation
RFC-first for review item O-D1 (perf rank 1): DB->get/put/del allocate and free a transient cursor per call, and both ends take a cursor-queue partition mutex that dominates the read path (85% of the t=96 profile, test/bench/BATCHED-READS-RESULTS.md). 0021 proposes a process-local, per-thread cache of idle transient cursors keyed by DB handle. Cached cursors stay on the handle's active queue, so __db_walk_cursors still sees every positioned cursor (the empty-partition-skip invariant is preserved and argued in Design S5). No on-disk/log/region/ABI format change; sizeof(DB) and sizeof(DBC) unchanged. DB_THREAD_INFO is a shared-region struct and cannot hold process-local pointers, so the cache uses the existing DB_LO_TLS thread-local pattern.
Reuse DB->get/put transient cursors from process-local thread-local storage so the common path skips the cursor-queue partition mutex at both ends (__db_cursor_int alloc and __dbc_close free), the 85% of the t=96 read profile in test/bench/BATCHED-READS-RESULTS.md. Mechanism: - A 4-slot per-thread TLS cache in db_cam.c keyed by (DB handle, DBTYPE), guarded by the same TLS portability macro as src/mutex/mut_order.c; with no TLS the cache is a no-op and behaviour matches master. - An eligible transient cursor (DBC_TRANSIENT, no opd, own default locker, no txn, threaded non-partitioned non-secondary handle) is left ON the handle's active queue across __dbc_close instead of moving to the free queue. While idle its cp->pgno is PGNO_INVALID and it holds no lock, so __db_walk_cursors adjustment callbacks never match it; a reused cursor is already on the queue before it positions, preserving the empty-partition-skip invariant in db.c. DB->del never sets DB_CURSOR_TRANSIENT, so cached cursors never carry a pending delete and am_close does no cursor adjustment. - __db_cursor_int serves a cache hit without any queue mutex and skips the final active-queue insert; a refresh failure returns the cursor to the free queue. - __db_refresh bumps a process-global epoch (so other threads' stale TLS slots miss without dereferencing a freed DB) and brackets its active-queue walk with refresh-enter/leave so re-entrant __dbc_close does not re-cache cursors being destroyed. - DB_NO_CURSOR_CACHE disables the cache (A/B + kill switch), matching the fork's DB_NO_OPTREAD / DB_NO_FNAME_CACHE pattern. No on-disk/log/region/ABI format change; sizeof(DB), sizeof(DBC) and DB_THREAD_INFO unchanged. dbinc_auto regenerated via dist/s_include.
test/db/cursorcache_adjust.c hammers one shared DB_THREAD btree handle with concurrent transient DB->put/DB->get on crowded leaves and verifies every value round-trips, so a cached cursor that missed a concurrent adjustment (and so addresses the wrong record) is caught. run_cursorcache_adjust.sh runs two arms: the real library (must PASS) and a library whose db_cam.o is rebuilt with -DDBC_CACHE_SABOTAGE (must FAIL). The sabotage positions a reused cached cursor while OFF the active queue, breaking the Design S5 invariant, so a missed adjustment corrupts a value. MANIFEST gets the db-tier entry; the runner is executable.
The first sabotage (positioning a reused cached cursor while off the active queue) was not observable: a transient get/put is page-latch-atomic, so an invisible positioned cursor produced the same result. Switch the sabotage to the RESET invariant, which is observable single-threaded: -DDBC_CACHE_SABOTAGE now skips the reuse reset in db_am.c, so a reused cached cursor keeps its prior position and returns the wrong record. The test gains a deterministic Part A (interleaved far-apart gets) that catches exactly that, plus the concurrent Part B storm. The runner rebuilds db_am.o for the sabotage arm.
Reusing a cached transient cursor is safe by construction: every DB->get/put re-searches from the root under the page latch, so a stale position or a brief off-queue window cannot corrupt a single transient op (confirmed: both earlier sabotages were no-ops). The one cache invariant whose failure IS observable is the stale-handle epoch guard (RFC 0021 S4a): a closed handle destroys its cached cursor, and the epoch bump must make a reopened handle miss the stale slot instead of dereferencing freed memory. The test's Part A now opens/uses/closes a DB_THREAD handle REOPENS times; the -DDBC_CACHE_SABOTAGE build (in db_cam.c) disables the epoch guard, so Part A faults on a dangling cached cursor under --enable-diagnostic. Part B keeps the concurrent adjustment storm. Runner rebuilds db_cam.o for the sabotage arm.
The disabled epoch guard permits a reopened handle to reuse a freed cached cursor, but freed memory often still looks valid, so the fault was silent. Link the sabotage executable with AddressSanitizer (it intercepts malloc/free process-wide, covering libdb's allocations) and run it under MALLOC_PERTURB_, so the use-after-free aborts deterministically. The correct arm is unaffected.
The probe compiled /dev/null, which has no main, so the link always failed and ASan was never used. Pipe a trivial main() instead.
Measured on c6id.metal (128 vCPU, XFS NVMe) against the non-transactional per-key DB->get path (batch_bench indiv), the workload behind the 85%-cursor-lifecycle profile. 6 interleaved reps, all CV <= 5.2%. Branch vs base (origin/master): 1.08x @t=1 (no single-thread regression), 1.53x @t=64, 2.81x @t=96; p99 262us -> 33us at t=96. Base scales to t=32 then declines to 997k/s (the cursor-queue-mutex wall); branch keeps climbing to 2.80M/s. The t=96 perf profile shows the machine idle 67% (base) vs 47% (branch): base leaves two-thirds of the cores blocked on the per-handle cursor mutex; the branch recovers that time. Adds the driver (cc_ab.sh), raw data, profile summaries and the results doc.
ABI diff vs
|
Coccinelle convention checksNo new violations. ✅ Resolved since baseline (3) -- update dist/cocci/baseline.txt to lock these in. |
Independent adversarial review — #238 perf/cursor-cache (RFC 0021)Verdict: MERGE (one documentation nit, no correctness defect found). Reviewer wrote none of this code and is read-only on the branch. Build + tests on the shared dev box (Debian 12, 32 vCPU, What I checked that passed
The one thing to fix (documentation, not code)RFC 0021 §5 and the This is still safe, but for a different reason than the RFC gives, and the RFC should say so:
Please correct the RFC/comment to say "reuse resets Risks from the brief and where they landedcursor-adjustment visibility (A1 ✅), stale handle after close/reopen (sabotage arm + A4 ✅), fork/multi-process (A2 ✅), thread exit (A4 ✅), DB_THREAD gating + non-threaded off (eligibility code + A6 ✅), failchk (process-local cache dies with the process; §4 reasoning sound), secondary indices (A3 ✅), cached cursor holding a locker/lock — CDB / RU/RC / non-txn under DB_INIT_LOCK (A3 lock-count + eligibility gate ✅), stale page pin ( Nothing blocks the merge. |
|
Lead review notes for the independent reviewer and the author. These come from reading the code; nothing here is measured yet.
|
Follow-up (lead-directed): two concerns tested — revising to FIX FIRSTI built the branch under ASan and TSan with high iteration counts and ran the two scenarios the lead raised. Both are real. One is benign-on-x86; the other is a measurable regression. I am revising my earlier MERGE verdict to FIX FIRST. Reproducers in Concern 1 — epoch data race: REAL, benign on x86-TSO, not portable
Severity: low on x86, but fix it. My ARM-2 probe (readers hammer Concern 2 — parked cursors degrade inserts on a long-lived handle: REAL, scales, merge-blockerOn thread exit, a thread's up-to-
Inserts on a handle churned by 256 exited threads run 2.3× slower, and it grows with the pool. Same binary with the cache disabled, and master, are flat — so this is the parked cursors, not noise. It persists for the life of the handle (parked cursors are reclaimed only at handle close). The RFC calls the thread-exit leak "bounded, correct, and no worse than an application that opened and leaked that many cursors" — but the measured cost on the write path is real and unbounded in pool-churn, and the RFC's own §4 flags the proactive Recommended fix: implement the Everything else from the earlier review still standsBuilds, own |
|
Lead: BLOCKER, worse than the two FIX FIRST items. A cached cursor is orphaned on every handle close anywhere in the process. No thread exit is needed.
Reproducer
This happens to any application that opens and closes a short-lived handle (a sub-database, a temp table, a The fix has to handle three things together:
Until then this PR must not merge. |
The #238 transient-cursor cache had three defects found in review: BLOCKER (orphan): the process-global epoch was used as a validity stamp. A close of ANY handle bumped it, so a thread's next cache get/put treated EVERY slot as stale and NULLed them all -- including slots whose handle was still open. That dropped a live handle's cached cursor from its slot while the cursor stayed DBC_ACTIVE on the handle's active queue forever. Every later insert/delete walked the orphan via __db_walk_cursors, unbounded: one thread doing get-on-H + open/close-of-T for 20k cycles made inserts on H 237x slower and added 32.7 MB. Fix: the epoch is now only a CHANGE SIGNAL. When a thread sees it moved, it RECONCILES its own slots -- keeping those whose handle is still live, dropping only those whose handle closed. Liveness is decided by walking env->dblist under mtx_dblist and matching a new per-handle, never-reused serial (DB->cursor_cache_gen, assigned at open from a process-global atomic counter), never by dereferencing a possibly-dangling dbp (ABA-safe). A live handle's slot is therefore never orphaned; a closed handle's cursor was already destroyed by __db_refresh's active-queue walk, so the slot is just dropped. The closing thread also clears its own slots for the handle at close, so no slot ever outlives its env and reconcile's env->dblist deref is always safe. Thread-exit flush: a pthread-key destructor (where pthreads back the mutexes) returns an exiting thread's live cached cursors to their handles' free queues, so a churning thread pool cannot pile inert cursors on the active queue (review concern 2: inserts 2.3x slower after 256 pool threads exit). Atomic epoch: __dbc_cache_global_epoch and the serial counter are now db_atomic_t with atomic_read/atomic_inc, fixing the data race on the plain u_int32_t (review concern 1, TSan-confirmed; UB on weakly ordered CPUs). DB grows by one u_int32_t (cursor_cache_gen); it is a process-local handle, not a region struct, so this is ABI-only, recorded via handle_sizes under the #234 soname bump. No on-disk, log or region format change. ABI-BREAK-INTENDED (sizeof(DB) +8 with padding; coordinated with #234 soname).
…0021) The orphan fix added a second stale-slot guard: the closing thread now clears its own cache slots at handle close, so the single-thread reopen loop in cursorcache_adjust part A is safe by self-clear even without the reconcile sweep. The sabotage arm must therefore disable BOTH guards to recreate the use-after-free, so -DDBC_CACHE_SABOTAGE now also skips the self-clear in __db_cursor_cache_discard. Sabotage arm FAILs (ASan UAF in __dbc_cache_free_cursor), correct arm PASSes.
…e (RFC 0021) Two CI-safe test/db runners that reproduce review #238's defects and assert they stay fixed: run_cursorcache_orphan -- a long-lived handle H, a loop of DB->get on H plus open/close of an unrelated handle T. Pre-fix: inserts >20x slower, RSS +tens of MB (the orphaned cursors on H's active queue). Fixed: insert ratio ~1.0, RSS flat. FAIL if ratio > 2 or RSS grows > 2 MB. run_cursorcache_parkcost -- single-thread insert time on a shared handle before and after 256 worker threads fill their cache slots and EXIT. Pre-fix: ratio ~2.1 at 256 threads (parked cursors walked by every insert). Fixed: ~1.0. FAIL if ratio > 1.5. Dev-box teeth: orphan FAIL 23.98 (pre-fix) -> PASS 1.02 (fix); parkcost FAIL 2.14 (pre-fix) -> PASS 1.03 (fix). Both are bounded/5k-cycle/256-thread and finish in well under their timeouts. MANIFEST rows + +x set.
Update RFC 0021 to match the review-#238 fixes: the epoch is a change signal (not a validity stamp), a per-handle never-reused serial + env->dblist walk tells a live slot from a stale one (ABA-safe), the closing thread clears its own slots so no slot outlives its env, the thread-exit pthread-key destructor is now implemented, and epoch/serial are db_atomic_t. Also records that DB->cursor_cache_gen costs no bytes (fits existing padding, sizeof(DB) unchanged) and corrects the \u00a75 claim that am_close zeroes cp->pgno -- it does not; the reuse-time reset is what makes an idle cached cursor safe.
c6id.metal, 6 interleaved reps, fixed branch vs the fork-point base vs the same binary with the cache disabled. The read-path win survives the orphan + thread-exit + atomic-epoch fixes unchanged: 2.81x at t=96, 1.39x at t=64; the cache-off arm tracks base at every point, so the delta is the cache. Profile: base burns ~90% of CPU in __db_tas_mutex_lock_int (the cursor-queue mutex), the branch's top symbol is __bam_cmp -- the contention is gone and the time is now the real B-tree get. sizeof(DB) unchanged (1744), so no ABI size change.
P1b: the three #238 defects are fixed (BLOCKER + thread-exit + atomic epoch)Branch DesignThe root cause of the BLOCKER was using the process-global epoch as a validity stamp: a close of any handle bumped it, so a thread's next transient get/put NULLed every slot — including slots whose handle was still open — leaving those live cursors The epoch is now only a change signal. When a thread sees it moved, it reconciles its own slots individually:
Plus:
Reproducer gates (dev box
|
| reproducer | pre-fix head (6bed835) | fixed |
|---|---|---|
rev238_a9_orphan, 20k cycles |
ratio 113.9×, +33.5 MB | 1.01×, +0 KB |
rev238_a8_parkcost, 256 threads |
ratio 2.05× | 0.999× (16/128 also flat) |
Each fails on the current head and passes with the fix. Added as bounded CI runners:
run_cursorcache_orphan(FAIL if ratio > 2 or RSS > 2 MB): pre-fix FAIL 23.98×/+7.9 MB, fix PASS 1.02×/+0 KB.run_cursorcache_parkcost(POOL=256, FAIL if ratio > 1.5): pre-fix FAIL 2.14×, fix PASS 1.03×.
Both have MANIFEST rows and +x.
Sanitizers
- A1–A4, A6 (
rev238_run.sh+ extras) under ASan: clean, branch==master. - A7 epoch race under TSan: PASS, zero races on
__dbc_cache_global_epochorcursor_cache_gen(the prototype's race is gone). The remaining TSan warnings are all pre-existing BDB mutex/lock/pid patterns.
Functional gates
run_cursorcache_adjust: correct PASS / sabotage FAIL (ASan use-after-free in__dbc_cache_free_cursor). The sabotage now disables both stale-slot guards (self-clear + reconcile), since the orphan fix added a second layer.run_handle_sizesPASS (sizeof(DB)/sizeof(DBC)unchanged).- Diagnostic
test/dbtierrun_allrc=0, 0 FAIL. - TCL btree+hash test001–010: 20/20 PASS.
The read-path win survives (c6id.metal, 6 interleaved reps, CV ≤ 6.4%)
| t | base (k/s) | branch (k/s) | cache-off (k/s) | br/base | br/off |
|---|---|---|---|---|---|
| 1 | 521,420 | 548,624 | 510,240 | 1.05× | 1.08× |
| 16 | 1,280,632 | 1,470,616 | 1,319,645 | 1.15× | 1.11× |
| 32 | 1,663,194 | 1,896,430 | 1,654,966 | 1.14× | 1.15× |
| 64 | 1,603,846 | 2,224,867 | 1,607,494 | 1.39× | 1.38× |
| 96 | 991,014 | 2,780,778 | 980,008 | 2.81× | 2.84× |
cache-off (same binary, DB_NO_CURSOR_CACHE=1) tracks base at every point, so the delta is entirely the cache. 2.81× at t=96 is identical to the prototype. Profile at t=96: base burns ~90% of CPU in __db_tas_mutex_lock_int (the cursor-queue mutex); the branch's top symbol is __bam_cmp — the contention is gone and the time is the real B-tree get.
(The t=64 ratio reads 1.39× vs the prototype's 1.53× only because this run's base t=64 median was higher/noisier; the same-binary branch/cache-off = 1.38× is the apples-to-apples figure.)
RFC 0021 §4 (thread exit) and the stale-slot design are updated. Master-only; no LTS backport. EC2 p1b torn down.
The prototype's cursor-cache A/B script shipped mode 100644; the exec-bit gate (dist/s_execbits) fails on it, which blocks every CI job early. One-line permission fix, no content change.
… gate)
CQ_LOCK/CQ_UNLOCK expand to MUTEX_LOCK/MUTEX_UNLOCK, which `return
(DB_RUNRECOVERY)` on a lock failure under --enable-debug. A void function
using them tripped clang's -Wreturn-type ("void function should not return a
value"), which failed the #140 ASan lock-mode CI build (clang). Give the
helper an int return and ignore it at the two void-context call sites (a
partition-mutex failure there is an env panic that surfaces on the next op;
the slot is dropped either way).
The TLS guard keys on HAVE_MUTEX_TAS, but the x86_64 gcc-assembly mutex backend defines HAVE_MUTEX_X86_64_GCC_ASSEMBLY instead, so DBC_CACHE_TLS is undefined there and the cache is compiled out -- yet __dbc_close referenced __dbc_cache_eligible/__dbc_cache_put unconditionally, giving an implicit declaration + link error (the mutex tas-x86_64 CI job). Add #else stubs that make every cursor ineligible and never cache, so __dbc_close behaves exactly like master on backends without the cache. Pre-existing in the prototype; surfaced by the full CI matrix.
Under AddressSanitizer+UBSan the process RSS is dominated by shadow memory and redzones that grow with the 5000 temp-handle cycles regardless of any orphan, so the +2 MB RSS bound tripped on a correct build (ratio 1.01, RSS +41 MB) in the --enable-typed asan+ubsan CI job. Skip the RSS assertion when a sanitizer is active; the insert-time ratio is the primary orphan signal and keeps its teeth (pre-fix FAILs 10.98x under ASan, fixed PASSes 1.01x).
|
Follow-up: four CI-hardening fixes after the first push (all build/portability, no behaviour change to the shipped default):
Locally reproduced + green on every CI config: gcc |
|
Lead re-review of the fix (f73b8e0): one blocker left, a NEW use-after-free introduced by the redesign. The orphan fix, the thread-exit flush, the atomic epoch and the metal numbers all check out. But the redesign rests on one claim (db_cam.c,
That is false when the thread that closes the handle is not the thread that cached the cursor. That is the normal case: a pool worker caches, and the main thread closes the DB and the env.
Reproducer
(freed by Fix direction:
|
…RFC 0021) Review of f73b8e0 found a use-after-free the redesign introduced (rev238_a10): the stale-slot validator __dbc_cache_handle_live walked slot->env->dblist, dereferencing the slot's ENV. The claim that "no slot outlives its env" holds only when the thread that CLOSES the handle is the one that CACHED the cursor. In the normal case -- a pool worker caches a cursor on handle H in env E and stays alive while the main thread closes H and E -- the worker's slot keeps a pointer to the freed ENV, and its next reconcile (thread-exit destructor, or check_epoch on a get against any other env) did MUTEX_LOCK on freed memory. ASan heap-use-after-free at db_cam.c:228 on both the exit and reuse arms; pre-fix head 6bed835 clean. Fix: a process-global live-handle registry, keyed by (dbp, cursor_cache_gen), under one process mutex (__dbc_reg_mtx) that lives in no env. DB->open adds (__db_cursor_cache_register); __db_refresh removes (in __db_cursor_cache_discard) BEFORE the handle's cursors are destroyed. Reconcile and the eviction path validate a slot against the registry ONLY -- they never dereference the slot's dbp or env. A "live" result means the handle is open, so holding the registry mutex across the check and the free_cursor keeps both the handle and its env valid for the duration. Lock order registry -> CQ_LOCK; the registry mutex is a leaf taken in open/close/reconcile with no other lock held first, and nothing takes CQ_LOCK then the registry mutex, so no inversion. The registry is only consulted on an epoch change (a handle closed), never per get, so the hot path stays mutex-free. The cache now requires a pthreads or hybrid mutex backend (it needs __thread, a pthread_key destructor, and a pthread_mutex for the registry together); the default autoconf build is hybrid, so the mainstream build keeps the cache, and niche backends (pure TAS, UI-threads, Solaris LWP, WIN32) compile it out and behave like master. sizeof(DB) unchanged (cursor_cache_gen already fit the existing padding). No region/log/on-disk/ABI-size change.
…0021) run_cursorcache_envuaf reproduces review #238's cross-thread env use-after-free: a worker caches a cursor on H in env E and stays alive, the main thread closes H and E, and the worker then either exits (pthread-key destructor reconciles) or opens a new env and gets (check_epoch reconciles). Built with ASan when available (the fault is a read of freed heap). Pre-fix head FAILs both arms (heap-use-after-free in __dbc_cache_handle_live); the live-handle-registry fix PASSes. MANIFEST row + +x.
… db.h) COMPQUIET is an internal libdb macro not visible to a test that includes only db.h, so the ASan link failed with an undefined reference. Use a plain (void)a; cast instead.
The orphan/UAF guards are now the live-handle registry, so the sabotage arm must defeat the registry too: -DDBC_CACHE_SABOTAGE makes __dbc_cache_handle_live always report "live", which (with the existing gen-ignore in get and the skipped self-clear in discard) lets a reopened handle reuse a destroyed cached cursor. Sabotage arm FAILs (ASan use-after-free), correct arm PASSes.
… flake) Part B runs 8 writer + 4 reader threads on one shared DB_THREAD handle, but the env was opened with DB_INIT_MPOOL only -- no DB_INIT_LOCK -- so the concurrent writers were unsynchronised, an unsupported configuration. Under a tight loop that raced ~20% of the time with "writer get: Operation not permitted" (EPERM from the latch layer), independent of the cursor cache: the prototype head and the cache-off path fail at the same rate. Add DB_INIT_LOCK so the concurrent access is page-locked and supported; the transient cursors still use their own default locker, so they stay cache-eligible. 0/40 fails after the change (was ~6/30); sabotage arm still FAILs, correct arm PASSes.
__db_refresh read dbp's active_queue head unlocked (the resync probe) BEFORE __db_cursor_cache_discard removed the handle from the live-handle registry. That read raced a concurrent reconcile on another thread, which could still be moving a cached cursor on the same queue under CQ_LOCK (TSan: db.c:925 read vs __dbc_cache_free_cursor TAILQ write). Move discard ahead of the probe: once discard has removed the handle from the registry (under the registry mutex), no reconcile touches this handle's queues -- an in-flight one has already finished (the mutex serialises it) and a new one sees the handle closed and skips it -- so the unlocked probe and the teardown walk are race-free again. A7 under TSan: no cache/registry/epoch race remains (644 pre-existing BDB mutex/lock races).
Update RFC 0021 to the registry design: a process-global live-handle registry keyed (dbp, cursor_cache_gen) under one process mutex replaces the env->dblist liveness check that dereferenced a slot's possibly-freed ENV (review #238 rework, rev238_a10). Document the registry, the lock order (registry -> CQ), that validation runs only on an epoch change (never per get), that the cache now requires a pthreads/hybrid backend, and the new run_cursorcache_envuaf gate. Add the metal re-measure after the registry fix: 2.88x at t=96 / 1.51x at t=64, unchanged, cache-off tracks base (the registry adds no hot-path lock).
P1b rework: the cross-thread ENV use-after-free (rev238_a10) is fixedYou were right — the "no slot outlives its env" claim held only when the closing thread was also the caching thread. The fix replaces the Design
Gates (dev box + c6id.metal)
Read win survives the rework (c6id.metal, 6 interleaved reps)
2.88× at t=96 / 1.51× at t=64 — unchanged from before the registry. RFC 0021 §4/§4a updated to the registry design. Master-only; no LTS backport. EC2 torn down. |
Two portability fixes so the macOS CI jobs stop failing on measurement noise, without losing the teeth where they matter (Linux CI, metal, dev box): - cursorcache_orphan: skip the RSS bound on macOS/BSD too, not just under ASan. getrusage(2) reports ru_maxrss in BYTES on those platforms (kilobytes on Linux), so a +32 KB growth read as "32768 KB" tripped the 2 MB bound on a correct build (ratio 0.90, no orphan). The insert-time ratio still asserts everywhere and is the real orphan signal (the orphan was >20x). - cursorcache_parkcost: the ratio is a single-thread timing comparison that needs a quiet machine; GitHub's shared macOS runners perturb a 256-thread churn into the same ~2x range as the pre-fix regression (observed 2.21). On __APPLE__ run both phases (exercising the thread-exit flush) but only report the ratio; assert it everywhere else, where it is stable (Linux CI passed at 1.03, metal 0.998).
|
CI is green on the reworked branch ( Two small test-portability fixes after the first push surfaced the macOS failures (engine untouched): |
|
Lead re-verification of dacd6a4: merge-ready from the engine side.
#238 can merge after #234 (soname); it is master-only, with no LTS backport. |
Per-thread transient-cursor cache (RFC 0021, review item O-D1)
DB->get/DB->putwith no application cursor allocate a transient cursor percall (
__db_cursor_int) and free it per call (__dbc_close); both ends take aper-handle cursor-queue partition mutex. On a shared
DB_THREADhandle this isthe dominant read-path cost at scale — 85% of the t=96 profile in
test/bench/BATCHED-READS-RESULTS.md, and the named cause of RFC 0007's t=96per-key regression.
This PR adds a small process-local, per-thread cache of idle transient
cursors keyed by DB handle, so the common path reuses one without touching a
queue mutex.
Design (RFC 0021, committed first)
db_cam.c, keyed on(DB *, DBTYPE),behind the same TLS portability guard as
src/mutex/mut_order.c. With no TLSthe cache compiles to a no-op and behaviour matches master.
DBC_TRANSIENT, no off-page-dup cursor, owndefault locker, no txn, threaded non-partitioned non-secondary handle) is left
on the handle's active queue across
__dbc_closeinstead of moving to thefree queue. While idle its
cp->pgnoisPGNO_INVALIDand it holds no lock,so
__db_walk_cursors's adjustment callbacks never match it; a reused cursoris already on the queue before it positions, preserving the empty-partition
skip's happens-before invariant in
db.c.DB->delnever setsDB_CURSOR_TRANSIENT, so cached cursors never carry a pending delete.__db_refreshbumps a process-global epoch (so other threads' stale TLS slotsmiss without dereferencing a freed
DB) and brackets its active-queue walk sore-entrant
__dbc_closedoes not re-cache cursors being destroyed.DB_NO_CURSOR_CACHEdisables the cache (A/B + kill switch), matching thefork's
DB_NO_OPTREAD/DB_NO_FNAME_CACHEpattern.No on-disk/log/region/ABI format change.
sizeof(DB),sizeof(DBC)andDB_THREAD_INFOare unchanged (proven byrun_handle_sizes.sh);env_sig.cand the region signature are byte-identical to master.
dbinc_autoregeneratedvia
dist/s_include.Scope note
The cache engages only for non-transactional
DB->get/DB->put. Atxn-associated cursor cannot be cached (its locker belongs to a txn that
commits/aborts between ops), so
xrel_kv(all-transactional) shows no delta byconstruction; the win is on the non-txn path (the motivating profile's
workload).
Tests (fail without the change, pass with it)
test/db/run_cursorcache_adjust.sh(+ driver): Part A opens/uses/closes aDB_THREADhandle 400 times (stale-handle stress); Part B is a concurrentput/get storm on crowded shared leaves with verification. The sabotage arm
rebuilds
db_cam.owith-DDBC_CACHE_SABOTAGE(stale-handle epoch guarddisabled) and must FAIL — it does, deterministically, with an ASan
use-after-free at
__dbc_cache_put; the correct arm PASSES.--enable-diagnostictest/dbtier: 30 PASS / 4 SKIP (env) / 0 FAIL.run_method -btree test001..010and-hash test001..010: 20/20 PASS.Performance (c6id.metal, 128 vCPU, XFS NVMe;
batch_bench indiv, 6 reps)All CV ≤ 5.2%. Base declines past t=32 (the cursor-mutex wall); the branch keeps
climbing. The t=96 perf profile shows the machine idle 67% (base) vs 47%
(branch) — base leaves two-thirds of the cores blocked on the cursor-queue
mutex. Full data and profile in
test/bench/CURSOR-CACHE-RESULTS.md.Draft: opened for the lead to review and merge.