Free Path Hash bulk deletions on the lazy free thread

[madolson]

PHDELPREFIX key "" and PHDEL key path release the deleted memory on the main thread, unlike every other collection type in Valkey. Deleting 1M paths with PHDELPREFIX key "" blocks the event loop for 40 ms, and deleting a single path holding 2M fields with PHDEL blocks it for 121 ms, while UNLINK of the identical object takes 13 us. Both sites already unlink the memory from the keyspace before freeing it, so the detached rax or the detached payload can be handed to the bio lazy free thread instead of being freed inline. Gated on lazyfree-lazy-user-del and on the existing free effort threshold, so small trees and listpack payloads still free inline.

AI generated details

Problem

phdelprefixCommand's empty-prefix branch replaced the index with a fresh rax and then freed the old one inline (src/t_path_hash.c:572-575 on agents/unstable):

rax *empty = raxNew();
raxFreeWithCallback(path_hash->index, freePathHashPayload);
path_hash->index = empty;
path_hash->num_fields = 0;

phdelCommand did the same for a whole path, freeing the unlinked payload with a bare decrRefCount (src/t_path_hash.c:389 on agents/unstable).

lazyfreeGetFreeEffort already understands the type (src/lazyfree.c:189-192), but that only serves whole-object frees through UNLINK, eviction and FLUSHALL. Neither command reached it.

The Path Hash design document names this as deferred work: "If truly nonblocking deletion of a large subtree is needed in the future, rax can gain a subtree-detach capability and hand detached nodes to the lazy-free thread. This is not required for v1."

Fix

freePathHashIndex(rax *index) in src/t_path_hash.c:54-58 wraps the raxFreeWithCallback(index, freePathHashPayload) call. The free function has to live in t_path_hash.c because freePathHashPayload is static there and is the only thing that knows a rax value is a refcounted payload. freePathHashObject now calls it, so there is one definition of how to free an index.

freePathHashIndexAsync(rax *index, uint64_t num_fields) in src/lazyfree.c:224-233 applies the threshold and queues lazyFreePathHashIndex (src/lazyfree.c:98-104) via bioCreateLazyFreeJob. The effort calculation moved into pathHashFreeEffort (src/lazyfree.c:131-135) so the async decision and lazyfreeGetFreeEffort cannot drift apart.

phdelprefixCommand reassigns path_hash->index before handing off (src/t_path_hash.c:579-590). phdelCommand reuses freeObjAsync for the unlinked payload (src/t_path_hash.c:395-397), matching the existing freeObjAsync(NULL, obj, -1) pattern at src/t_set.c:1665 and src/t_zset.c:2277.

Accounting

lazyFreePathHashIndex decrements lazyfree_objects by 1 and increments lazyfreed_objects by 1, exactly like lazyfreeFreeObject (src/lazyfree.c:15-20). A detached index is one pending object, the same as the whole object would be under UNLINK, so lazyfree_pending_objects keeps its existing meaning. The PHDEL path goes through freeObjAsync, which already does its own accounting.

Config decision

Gated on lazyfree-lazy-user-del. PHDELPREFIX and PHDEL are a user explicitly asking to delete, which is the DEL/UNLINK shape that config governs, and its default is already yes (src/config.c:3436). Unconditional-when-effort-is-high was rejected: an operator who set lazyfree-lazy-user-del no did so to keep user deletions synchronous and accounted to the calling command, and silently ignoring that for one type would be surprising. No new config.

Why the handoff cannot race

Payloads in a Path Hash index are referenced only by the rax. t_path_hash.c never calls incrRefCount on a payload, so no payload pointer outlives a single command invocation, and every reader outside the file reaches payloads through path_hash->index (src/rdb.c:1208, src/rdb.c:2596, src/debug.c:1269, src/module.c:4470, src/valkey-check-rdb.c:395).

path_hash->index is reassigned to a fresh rax before the job is created, so by the time the bio thread can see the detached rax it is reachable from nothing else. A subsequent command on the same key, FLUSHALL, DEBUG RELOAD, or DEL of the key all operate on the pathHashObject and therefore on the new empty index. Freeing the object later frees only that new index. A snapshot child gets a copy-on-write image and is unaffected by a job queued in the parent.

freeObjAsync additionally re-checks refcount == 1 before handing an object off (src/lazyfree.c:216), so the PHDEL path is safe even if a future caller does retain a payload.

Measurements

1M paths, 2 fields each, 87.6 MB. Main thread time is usec_per_call from INFO commandstats, which is the time the command holds the event loop.

PHDELPREFIX key "":

build	run 1	run 2
before	40595 us	40764 us
after, lazyfree-lazy-user-del no	40155 us
after, default	22 us	19 us

UNLINK of the identical object, for reference: 13 us.

PHDEL key path on one path holding 2M fields, 98.2 MB: 121109 us before, 12 us after. lazyfree_pending_objects:1 was observable after that one.

DEBUG DIGEST-VALUE of the resulting empty tree is 892cc1e58fdd61b3796fec8aaa1cb849d2de3ebf in every run above, sync and async. EXISTS 1, TYPE pathhash, PHCARD 0.

Not done: the non-empty-prefix path

PHDELPREFIX key <prefix> still deletes synchronously. The design document suggests batching the payload frees, but measured on 500k paths with a 1-byte prefix, the field count barely moves the cost:

fields per path	memory	main thread
1	36.8 MB	87602 us
2	44.4 MB	87203 us
8	110.1 MB	92570 us

Tripling the memory adds 5.7% to the duration, so payload teardown is not the dominant cost and moving it to bio would buy almost nothing. The cost is in the per-path raxRemove plus the raxSeek restart per 256-path chunk (src/t_path_hash.c:601-626). Making that nonblocking needs the real rax subtree-detach capability the design document names, which touches path-compression invariants and needs its own differential testing. Left out deliberately.

Testing

Four tests in tests/unit/type/path_hash.tcl. lazyfree_pending_objects draining to zero plus lazyfreed_objects becoming 1 is the assertion, rather than catching lazyfree_pending_objects > 0. Catching the rise is inherently a race against the bio thread on the small trees a test can build, and there is no hook to pause the lazy free worker, only DEBUG BIO-DRAIN to wait for it. lazyfreed_objects == 1 proves the free went through the thread deterministically, which is what the flaky check was trying to establish.

Coverage: async on a 200-path tree, async on a payload with hash-max-listpack-entries + 1 fields, inline with lazyfree-lazy-user-del no, and inline for a 5-path tree under the effort threshold. The inline cases matter because a policy bug that always went async would otherwise pass every other test.

Existing assertions were not modified, including the empty-object round-trip at tests/unit/type/path_hash.tcl:658.

ASAN plus LeakSanitizer, HAVE_IFUNC 0, over unit/type/path_hash and unit/lazyfree: clean. Also clean on a manual run that interleaves PHDELPREFIX with a 1-byte prefix, PHDELPREFIX "", PHDEL of a large payload, DEBUG RELOAD immediately after an async PHDELPREFIX, and FLUSHALL with a free still pending.

Could not verify formatting: this host has clang-format 11, which rejects src/.clang-format (unknown key 'InsertNewlineAtEOF'), and clang-format-18 is not installed. Added lines are under 120 columns and follow the surrounding style. Also did not test under a replica or with AOF enabled beyond the existing suite, since propagation is unchanged: both commands still propagate verbatim and server.dirty and the keyspace notifications are untouched.

This was generated by AI but verified, with love, by a human.