Remove PeerFinder request timeout (#134365)

There's really no need to time out so enthusiastically here, we can wait for as long as it takes to receive a list of other discovery targets from the remote peer. This commit removes the timeout, deferring failure detection down to the TCP level (e.g. keepalives) as managed by the OS. Relates #132713, #123568

Author: DaveCTurner

docs/changelog/134365.yaml

@@ -0,0 +1,13 @@
+pr: 134365
+summary: Remove `PeerFinder` request timeout
+area: Cluster Coordination
+type: deprecation
+issues: []
+deprecation:
+ title: Remove `PeerFinder` request timeout
+ area: Cluster and node setting
+ details: >-
+ There is no need to time out requests sent by the `PeerFinder` during discovery and cluster formation, and this
+ timeout may sometimes cause spurious failures. With this change the `PeerFinder` requests will wait indefinitely for
+ responses. The `discovery.request_peers_timeout` setting no longer has any effect.
+ impact: Discontinue use of the `discovery.request_peers_timeout` setting.

docs/reference/elasticsearch/configuration-reference/discovery-cluster-formation-settings.md

@@ -50,7 +50,7 @@ If you adjust these settings then your cluster may not form correctly or may bec
 : ([Static](docs-content://deploy-manage/stack-settings.md#static-cluster-setting)) Sets how long to wait when attempting to identify the remote node via a handshake. Defaults to `30s`.
 
 `discovery.request_peers_timeout`
-: ([Static](docs-content://deploy-manage/stack-settings.md#static-cluster-setting)) Sets how long a node will wait after asking its peers again before considering the request to have failed. Defaults to `3s`.
+: ([Static](docs-content://deploy-manage/stack-settings.md#static-cluster-setting)) In 9.1.x and earlier versions, sets how long a node will wait after asking its peers again before considering the request to have failed. Has no effect from version 9.2.0 onwards. Defaults to `3s`.
 
 `discovery.find_peers_warning_timeout`
 : ([Static](docs-content://deploy-manage/stack-settings.md#static-cluster-setting)) Sets how long a node will attempt to discover its peers before it starts to log verbose messages describing why the connection attempts are failing. Defaults to `3m`.

server/src/main/java/org/elasticsearch/discovery/PeerFinder.java

@@ -66,7 +66,8 @@ public abstract class PeerFinder {
  "discovery.request_peers_timeout",
  TimeValue.timeValueMillis(3000),
  TimeValue.timeValueMillis(1),
- Setting.Property.NodeScope
+ Setting.Property.NodeScope,
+ Setting.Property.Deprecated
  );
 
  // We do not log connection failures immediately: some failures are expected, especially if the hosts list isn't perfectly up-to-date
@@ -81,7 +82,6 @@ public abstract class PeerFinder {
  );
 
  private final TimeValue findPeersInterval;
- private final TimeValue requestPeersTimeout;
  private final TimeValue verbosityIncreaseTimeout;
 
  private final Object mutex = new Object();
@@ -106,7 +106,6 @@ public PeerFinder(
  ConfiguredHostsResolver configuredHostsResolver
  ) {
  findPeersInterval = DISCOVERY_FIND_PEERS_INTERVAL_SETTING.get(settings);
- requestPeersTimeout = DISCOVERY_REQUEST_PEERS_TIMEOUT_SETTING.get(settings);
  verbosityIncreaseTimeout = VERBOSITY_INCREASE_TIMEOUT_SETTING.get(settings);
  this.transportService = transportService;
  this.clusterCoordinationExecutor = transportService.getThreadPool().executor(Names.CLUSTER_COORDINATION);
@@ -571,7 +570,7 @@ public Executor executor() {
  discoveryNode,
  REQUEST_PEERS_ACTION_NAME,
  new PeersRequest(getLocalNode(), knownNodes),
- TransportRequestOptions.timeout(requestPeersTimeout),
+ TransportRequestOptions.EMPTY,
  peersResponseHandler
  );
  }

server/src/test/java/org/elasticsearch/discovery/PeerFinderTests.java

@@ -750,52 +750,6 @@ public void testDoesNotMakeMultipleConcurrentConnectionAttemptsToOneAddress() {
  assertFoundPeers(otherNode);
  }
 
- public void testTimesOutAndRetriesConnectionsToBlackholedNodes() {
- final DiscoveryNode otherNode = newDiscoveryNode("node-from-hosts-list");
- final DiscoveryNode nodeToFind = newDiscoveryNode("node-to-find");
-
- providedAddresses.add(otherNode.getAddress());
- transportAddressConnector.addReachableNode(otherNode);
- transportAddressConnector.addReachableNode(nodeToFind);
-
- peerFinder.activate(lastAcceptedNodes);
-
- while (true) {
- deterministicTaskQueue.advanceTime();
- runAllRunnableTasks(); // MockTransportAddressConnector verifies no multiple connection attempts
- if (capturingTransport.getCapturedRequestsAndClear().length > 0) {
- break;
- }
- }
-
- final long timeoutAtMillis = deterministicTaskQueue.getCurrentTimeMillis() + PeerFinder.DISCOVERY_REQUEST_PEERS_TIMEOUT_SETTING.get(
- Settings.EMPTY
- ).millis();
- while (deterministicTaskQueue.getCurrentTimeMillis() < timeoutAtMillis) {
- assertFoundPeers(otherNode);
- deterministicTaskQueue.advanceTime();
- runAllRunnableTasks();
- }
-
- // need to wait for the connection to timeout, then for another wakeup, before discovering the peer
- final long expectedTime = timeoutAtMillis + PeerFinder.DISCOVERY_FIND_PEERS_INTERVAL_SETTING.get(Settings.EMPTY).millis();
-
- while (deterministicTaskQueue.getCurrentTimeMillis() < expectedTime) {
- deterministicTaskQueue.advanceTime();
- runAllRunnableTasks();
- }
-
- respondToRequests(node -> {
- assertThat(node, is(otherNode));
- return new PeersResponse(Optional.empty(), singletonList(nodeToFind), randomNonNegativeLong());
- });
-
- deterministicTaskQueue.advanceTime();
- runAllRunnableTasks();
-
- assertFoundPeers(nodeToFind, otherNode);
- }
-
  @TestLogging(reason = "testing logging at WARN level", value = "org.elasticsearch.discovery:WARN")
  public void testLogsWarningsIfActiveForLongEnough() throws IllegalAccessException {
  final DiscoveryNode otherNode = newDiscoveryNode("node-from-hosts-list");