* Remove dead client-side storage cache code left behind by #12486
LocationInfo::hasCaches, addCaches(), updateLocationCacheWithCaches(),
DatabaseContext::{cacheListMonitor, updateCache}, and the dead hasCaches branch in loadBalance() - all unreachable since #12486.
Follow-up sweep in the same spirit as #13119.
* Inline trivial loadBalance wrapper into its callers
Addresses review feedback on #13326. With the hasCaches branch gone, the anonymous-namespace loadBalance(DatabaseContext*, Reference<LocationInfo>, ...) wrapper is a pure forwarder. Delete it and update its five callers to call loadBalance(alternatives->locations(), ...) directly. No behavior change.
Building on #13188 (Peer LSAN suppressions + gRPC use-after-return fix) and
#13255 (client-side shutdown leak suppressions), this commit addresses the root
causes rather than suppressing symptoms.
Problem: CI ASAN nightly builds reported 128K+ bytes leaked in 2000+
allocations across 27 tests. The leaks came from two sources:
1. TaskQueue::clear() in Net2::stopImmediately() swapped out the timer and
ready queues but never deleted the PromiseTask* pointers inside them.
Each leaked PromiseTask held a Promise<Void> whose destruction would have
freed waiting actors coroutine frames.
2. DatabaseContext::~DatabaseContext() cancelled some background actors but
missed four others: logger (databaseLogger + tssLogger),
clientStatusUpdater.actor, throttleExpirer, and statusLeaderMon.
Fix:
- TaskQueue::clear() now swaps queues into locals then iterates and deletes
all Task* pointers. Deleting a PromiseTask fires broken_promise to waiting
futures, which cancels the associated actors and frees their coroutine
frames. The swap-first approach prevents infinite loops from actors that
catch broken_promise and retry with a new delay().
- DatabaseContext destructor now cancels all four leaked background actors,
following the same pattern already used for clientDBInfoMonitor et al.
- LSAN suppressions trimmed from 30+ entries to 8. The remaining suppressions
cover genuinely unfixable cases: Peer objects (no safe destructor path),
fdbcli transaction references not released before stopNetwork(), external
client DatabaseContext cleanup deferred via onMainThreadVoid after network
stop, and monitorProtocolVersion cross-thread dispatch.
Result: Zero LSAN reports, 91% ctest pass rate (remaining 4 failures are
ASAN slowness timeouts and the pre-existing makecontext/Severity=40 issue).
Attempting to reduce max source file size to a more reasonable 4000 lines, to start with.
These methods are mostly DatabaseContext member functions so this seems like a no-brainer.
The rationale for this is mostly self-evident but rdar://160903696 has some additional notes.
Testing in progress:
20250919-205905-gglass-1364d752d341f4e4 compressed=True data_size=41575837 duration=6130944 ended=100000 fail=1 fail_fast=10 max_runs=100000 pass=99999 priority=100 remaining=0 runtime=0:57:39 sanity=False started=100000 stopped=20250919-215644 submitted=20250919-205905 timeout=5400 username=gglass
The one failure was this: fdbserver -r simulation -s 424077837 -b on -f tests/slow/BackupCorrectnessPartitioned.toml
reproduces on main without this PR. Filed rdar://160972142 for that.
* move initial batch of DatabaseContext methods to DatabaseContext.actor.cpp
* another batch of functions NativeAPI.actor.cpp to DatabaseContext.actor.cpp
* Split out printable() methods to a separate file, and ongoing migration of DatabaseContext methods to its own file
* add Printable.cpp, methods split out from NativeAPI
* add a header to eliminate ugly random prototypes in source files for getSourceVersion
* move more DatabaseContext methods
* move more code from NativeAPI.actor.cpp to DatabaseContext.actor.cpp
* formatting
In NativeAPI.actor.cpp, ::watch ACTOR will call cx->clearWatchMetadata()
when the connectionFileChanged event is triggered. After that, it will
create a new watch metadata for itself.
If there are multiple watches, each of them that receives the
cnnectionFileChanged will clear *all* watch data and create *one* watch
for itself. This does not makes sense. A watch should only clear the
metadata, and then create one, for itself.
cx->clearWatchMetadata() is only used there, thus removed.
In the following scenario a watch might be deleted by mistake:
Two watches over key A
Watch 1 Watch 2 Reference Count
| []
| Version 100, Value 10
T | Add reference count [100]
I | Metadata added
M | ...
E | Version 200, Value 20
| Add reference count [100, 200]
| Delete the old metadata
| *together with the reference count* <----- [1]
| []
| Trigger watch 1
| Update metadata
| Triggered
| Delete the reference count []
| ...
L | Version 200, Value 20
I | Add reference count [200]
N | Same metadata used
E | ...
| Watch 2 cancelled
| Reduce reference count []
| Delete the metadata
| (watchPromise removed, send broken_promise to listeners)
| broken_promise!
V
By *not* clear the reference count in [1], just remove 100, this problem
will be fixed, since the watchPromise will still have one reference and
not being removed until watch 1 get triggered/cancelled again.
Tested by running 100k correctness on branch 7.1. One irrelevant
failure occured which will be tracked differently.
In the case
1. A watch to key A is set, the watchValueMap ACTOR, noted as X, starts waiting.
2. All watches are cleared due to connection string change.
3. The watch to key A is restarted with watchValueMap ACTOR Y.
4. X receives the cancel exception, and tries to dereference the counter. This causes Y gets cancelled.
the reference count will cause watch prematurely terminate. Recording
the versions of each watch would help preventing this issue
Trevor Clinkenbeard