Skip to content

Commit f2661f4

Browse files
Ali Poursamadiclaude
andcommitted
[fast-client] De-flake testRequestBasedMetadataOnDemandRefresh
The test dispatched start() onto the shared ForkJoinPool.commonPool() via CompletableFuture.runAsync. start() blocks indefinitely on isReadyLatch.await() in this all-failures scenario, and under CI load the common-pool task could sit queued past the 3s verification timeout, intermittently failing the first verify(updateCache(false)). Run start() on a dedicated daemon thread so it begins promptly regardless of pool load, and deterministically tear it down (interrupt + join + assert it exited) instead of leaking a blocked thread until JVM teardown. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent a9deece commit f2661f4

1 file changed

Lines changed: 19 additions & 8 deletions

File tree

clients/venice-client/src/test/java/com/linkedin/venice/fastclient/meta/RequestBasedMetadataTest.java

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -424,15 +424,26 @@ public void testRequestBasedMetadataOnDemandRefresh() throws IOException, Interr
424424
try (RequestBasedMetadata requestBasedMetadata = new RequestBasedMetadata(clientConfig, d2TransportClient)) {
425425
RequestBasedMetadata spy = spy(requestBasedMetadata);
426426
spy.setD2ServiceDiscovery(d2ServiceDiscovery);
427-
// let child thread handling the start logic otherwise the main thread cannot verify the invocation times.
428-
CompletableFuture.runAsync(spy::start);
427+
// Run start() on a dedicated thread (not the shared ForkJoinPool.commonPool() that runAsync uses): start()
428+
// blocks indefinitely on isReadyLatch.await() in this all-failures scenario, and dispatching it onto the common
429+
// pool makes the test flaky under load (the task can sit queued past the verification timeout). A dedicated
430+
// daemon thread guarantees start() begins promptly so the main thread can verify the invocation counts.
431+
Thread starter = new Thread(spy::start, "on-demand-refresh-start");
432+
starter.setDaemon(true);
433+
starter.start();
429434

430-
// refresh would happen multiple times
431-
// the first one w/o onDemond refresh and would fail due to d2 client exception
432-
verify(spy, timeout(3000).atLeast(1)).updateCache(false);
433-
434-
// the first failed refresh triggers a onDemand refresh
435-
verify(spy, timeout(3000).atLeast(1)).updateCache(true);
435+
try {
436+
// refresh would happen multiple times
437+
// the first one w/o onDemond refresh and would fail due to d2 client exception
438+
verify(spy, timeout(3000).atLeast(1)).updateCache(false);
439+
440+
// the first failed refresh triggers a onDemand refresh
441+
verify(spy, timeout(3000).atLeast(1)).updateCache(true);
442+
} finally {
443+
starter.interrupt();
444+
starter.join(TimeUnit.SECONDS.toMillis(5));
445+
assertFalse(starter.isAlive(), "starter thread should exit after interruption");
446+
}
436447
}
437448
}
438449

0 commit comments

Comments
 (0)