fix: coalesce forced rediscovery instead of spawning a task per failure

[sshaplygin]

Problem

TimerDiscovery::pessimize spawns a forced rediscovery whenever at least half the nodes are pessimized:

if pessimized_nodes_count > 0 && pessimized_nodes_count >= state.original_nodes.len() / 2 {
    tokio::spawn(async move { ... state.discovery_now().await ... });
}

pessimize is called per failing request. During an outage — exactly when that condition holds — every failure spawns another task. They all queue on the same discovery_lock, so the work is serialised but the tasks are not: the SDK piles up rediscovery tasks against a cluster that is already struggling, which is the opposite of what a backoff should do.

Change

Guard the spawn with an AtomicBool on DiscoverySharedState. The first caller claims the refresh via compare_exchange; further callers observe one already in flight and skip. The flag is released once discovery_now returns, so the next burst can trigger a fresh refresh.

This coalesces the storm without changing when a refresh happens, or the existing discovery_lock serialisation.

Test

forced_discovery_coalesces_requests_until_completion asserts a second claim is refused while one is in flight, and that a claim succeeds again after completion.

Note for reviewers

The flag is cleared after discovery_now().await returns, including on Err. If that spawned task were cancelled — for example at runtime shutdown — the flag would stay set and forced rediscovery would be disabled for the remaining life of the state. That seemed acceptable given the task only runs for as long as the driver does, but say the word if you would rather see it held by a guard that clears on drop.

Verification

cargo fmt --check
cargo clippy --workspace --all-targets --no-deps --exclude=ydb-grpc -- -D warnings
cargo test --workspace     # 215 passed, 88 ignored

🤖 Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 98.06950% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.15%. Comparing base (a6d7911) to head (54d72f3).

Files with missing lines	Patch %	Lines
ydb/src/discovery.rs	98.06%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #592      +/-   ##
==========================================
+ Coverage   86.91%   87.15%   +0.24%     
==========================================
  Files         198      198              
  Lines       19492    19750     +258     
==========================================
+ Hits        16941    17214     +273     
+ Misses       2551     2536      -15     

Flag	Coverage Δ
rust-1.88.0	87.15% <98.06%> (+0.24%)	⬆️
rust-1.96.1	87.47% <99.60%> (+0.31%)	⬆️
tests	87.15% <98.06%> (+0.24%)	⬆️
ubuntu	87.15% <98.06%> (+0.24%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[sshaplygin]

Pushed 54d72f3 raising ydb/src/discovery.rs past the 80% target.

Metric	Before	After
Lines	34.4%	87.3%
Regions	38.8%	88.8%
Functions	31.6%	94.9%

Those are whole-file numbers, so they include the inline test module. Excluding it, the production code alone goes 36.3% → 97.4% of lines.

The module was almost entirely reachable only through the two #[ignore] tests that need a live YDB. Now covered offline:

• DiscoveryState — node exposure, empty default, pessimization and its idempotence, the fallback that restores every node once all are pessimized, with_node_info.
• Endpoint conversion — http/https by the ssl flag, and rejection of a malformed authority.
• StaticDiscovery — state, waiter, no-op pessimization, empty subscription, malformed endpoint.
• DiscoverySharedState — no state before first discovery, state() resolving on publish, wait() surfacing a stored error, subscribe() filtering failures out, pessimization updating the published state.
• background_discovery — exits once the shared state is dropped.

On the coalescing itself

The first version of the storm test spawned the refresh but never waited for it, so lines 252–256 — the spawned task body, the core of this PR — stayed uncovered even though the test passed. It now waits for forced_discovery_in_flight to clear, which only happens after the task has run discovery_now and called complete_forced_discovery. It then asserts a second storm can schedule a fresh refresh, which is the property the flag exists to provide.

Nothing contacts a server: refreshes point at a closed loopback port, refused immediately.

The one production branch still uncovered is the token-waiter error path in background_discovery (lines 392–394), which needs a failing token source to reach.

cargo fmt --check, clippy -D warnings and cargo test --workspace pass (234 passed, 88 ignored).

🤖 Generated with Claude Code