Skip to content

Commit

Permalink
xds: Avoid nonexistent DNS resolution in XdsClientImplV3Test
Browse files Browse the repository at this point in the history
The DNS lookups are taking considerable time on the Windows CI (~11s),
which causes the test to time out:

```
Wanted but not invoked:
ldsResourceWatcher.onError(<any>);
-> at io.grpc.xds.XdsClientImplTestBase.sendToNonexistentHost(XdsClientImplTestBase.java:3733)
Actually, there were zero interactions with this mock.

	at io.grpc.xds.XdsClientImplTestBase.sendToNonexistentHost(XdsClientImplTestBase.java:3733)
```

The ARM build, which uses an emulator, has had this test succeed, so the
failure seems unrelated to CPU usage. We want to avoid external I/O
anyway during tests, so removing the DNS lookup is good.

The TSAN comment referenced XdsClientImplTestBase.sendToNonexistentHost,
but the test no longer calls fakeClock.forwardTime so the comment was
out-of-date. Change the comment to make clear the race involved.
  • Loading branch information
ejona86 committed Feb 23, 2024
1 parent d7628a3 commit bfc0f95
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 3 deletions.
4 changes: 3 additions & 1 deletion xds/src/main/java/io/grpc/xds/ControlPlaneClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,9 @@ private void handleRpcStreamClosed(Status error) {
// has never been initialized.
retryBackoffPolicy = backoffPolicyProvider.get();
}
// Need this here to avoid tsan race condition in XdsClientImplTestBase.sendToNonexistentHost
// FakeClock in tests isn't thread-safe. Schedule the retry timer before notifying callbacks
// to avoid TSAN races, since tests may wait until callbacks are called but then would run
// concurrently with the stopwatch and schedule.
long elapsed = stopwatch.elapsed(TimeUnit.NANOSECONDS);
long delayNanos = Math.max(0, retryBackoffPolicy.nextBackoffNanos() - elapsed);
rpcRetryTimer = syncContext.schedule(
Expand Down
7 changes: 5 additions & 2 deletions xds/src/test/java/io/grpc/xds/XdsClientImplTestBase.java
Original file line number Diff line number Diff line change
Expand Up @@ -3726,9 +3726,12 @@ public void sendToBadUrl() throws Exception {
}

@Test
public void sendToNonexistentHost() throws Exception {
public void sendToNonexistentServer() throws Exception {
// Setup xdsClient to fail on stream creation
XdsClientImpl client = createXdsClient("some.garbage");
// The Windows CI takes ~11 seconds to resolve "doesnotexist.invalid.". That seems broken, since
// it should take no I/O, but let's limit ourselves to IP literals and hostnames in the hosts
// file. Assume localhost doesn't speak HTTP/2 on the finger port
XdsClientImpl client = createXdsClient("localhost:79");
client.watchXdsResource(XdsListenerResource.getInstance(), LDS_RESOURCE, ldsResourceWatcher);
verify(ldsResourceWatcher, Mockito.timeout(5000).times(1)).onError(ArgumentMatchers.any());
assertThat(fakeClock.numPendingTasks()).isEqualTo(1); //retry
Expand Down

0 comments on commit bfc0f95

Please sign in to comment.