Remove races in ProxyConnectionStrategyTests (#50620)

Currently, we use delayed address resolution in the proxy strategy tests
to allow tests to connect to different addresses. Unfortunately, this
has the potential to introduce races as the address is resolved each
connection attempt. The number of connection attempts can vary based on
when connections are opening and closing. This commit modifies the test
be allowing them to specifically control which address is used.

Related to #50618
This commit is contained in:
Tim Brooks 2020-01-06 11:12:00 -06:00
parent 074866256b
commit fa57813c6d
No known key found for this signature in database
GPG Key ID: C2AA3BB91A889E77
1 changed files with 11 additions and 10 deletions

View File

@ -122,9 +122,11 @@ public class ProxyConnectionStrategyTests extends ESTestCase {
ConnectionManager connectionManager = new ConnectionManager(profile, localService.transport); ConnectionManager connectionManager = new ConnectionManager(profile, localService.transport);
int numOfConnections = randomIntBetween(4, 8); int numOfConnections = randomIntBetween(4, 8);
AtomicBoolean useAddress1 = new AtomicBoolean(true);
try (RemoteConnectionManager remoteConnectionManager = new RemoteConnectionManager(clusterAlias, connectionManager); try (RemoteConnectionManager remoteConnectionManager = new RemoteConnectionManager(clusterAlias, connectionManager);
ProxyConnectionStrategy strategy = new ProxyConnectionStrategy(clusterAlias, localService, remoteConnectionManager, ProxyConnectionStrategy strategy = new ProxyConnectionStrategy(clusterAlias, localService, remoteConnectionManager,
numOfConnections, address1.toString(), alternatingResolver(address1, address2), false)) { numOfConnections, address1.toString(), alternatingResolver(address1, address2, useAddress1), false)) {
assertFalse(connectionManager.getAllConnectedNodes().stream().anyMatch(n -> n.getAddress().equals(address1))); assertFalse(connectionManager.getAllConnectedNodes().stream().anyMatch(n -> n.getAddress().equals(address1)));
assertFalse(connectionManager.getAllConnectedNodes().stream().anyMatch(n -> n.getAddress().equals(address2))); assertFalse(connectionManager.getAllConnectedNodes().stream().anyMatch(n -> n.getAddress().equals(address2)));
@ -139,6 +141,7 @@ public class ProxyConnectionStrategyTests extends ESTestCase {
assertEquals(0, initialConnectionsToTransport2); assertEquals(0, initialConnectionsToTransport2);
assertEquals(numOfConnections, connectionManager.size()); assertEquals(numOfConnections, connectionManager.size());
assertTrue(strategy.assertNoRunningConnections()); assertTrue(strategy.assertNoRunningConnections());
useAddress1.set(false);
transport1.close(); transport1.close();
@ -199,10 +202,11 @@ public class ProxyConnectionStrategyTests extends ESTestCase {
ConnectionManager connectionManager = new ConnectionManager(profile, localService.transport); ConnectionManager connectionManager = new ConnectionManager(profile, localService.transport);
int numOfConnections = randomIntBetween(4, 8); int numOfConnections = randomIntBetween(4, 8);
Supplier<TransportAddress> resolver = alternatingResolver(address1, address2); AtomicBoolean useAddress1 = new AtomicBoolean(true);
try (RemoteConnectionManager remoteConnectionManager = new RemoteConnectionManager(clusterAlias, connectionManager); try (RemoteConnectionManager remoteConnectionManager = new RemoteConnectionManager(clusterAlias, connectionManager);
ProxyConnectionStrategy strategy = new ProxyConnectionStrategy(clusterAlias, localService, remoteConnectionManager, ProxyConnectionStrategy strategy = new ProxyConnectionStrategy(clusterAlias, localService, remoteConnectionManager,
numOfConnections, address1.toString(), resolver, false)) { numOfConnections, address1.toString(), alternatingResolver(address1, address2, useAddress1), false)) {
assertFalse(connectionManager.getAllConnectedNodes().stream().anyMatch(n -> n.getAddress().equals(address1))); assertFalse(connectionManager.getAllConnectedNodes().stream().anyMatch(n -> n.getAddress().equals(address1)));
assertFalse(connectionManager.getAllConnectedNodes().stream().anyMatch(n -> n.getAddress().equals(address2))); assertFalse(connectionManager.getAllConnectedNodes().stream().anyMatch(n -> n.getAddress().equals(address2)));
@ -212,6 +216,7 @@ public class ProxyConnectionStrategyTests extends ESTestCase {
assertTrue(connectionManager.getAllConnectedNodes().stream().anyMatch(n -> n.getAddress().equals(address1))); assertTrue(connectionManager.getAllConnectedNodes().stream().anyMatch(n -> n.getAddress().equals(address1)));
assertFalse(connectionManager.getAllConnectedNodes().stream().anyMatch(n -> n.getAddress().equals(address2))); assertFalse(connectionManager.getAllConnectedNodes().stream().anyMatch(n -> n.getAddress().equals(address2)));
useAddress1.set(false);
transport1.close(); transport1.close();
@ -375,16 +380,12 @@ public class ProxyConnectionStrategyTests extends ESTestCase {
} }
} }
private Supplier<TransportAddress> alternatingResolver(TransportAddress address1, TransportAddress address2) { private Supplier<TransportAddress> alternatingResolver(TransportAddress address1, TransportAddress address2,
// On the first connection round, the connections will be routed to transport1. On the second AtomicBoolean useAddress1) {
//connection round, the connections will be routed to transport2
AtomicBoolean transportSwitch = new AtomicBoolean(true);
return () -> { return () -> {
if (transportSwitch.get()) { if (useAddress1.get()) {
transportSwitch.set(false);
return address1; return address1;
} else { } else {
transportSwitch.set(true);
return address2; return address2;
} }
}; };