Remove PeerFinder request timeout (elastic#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 elastic#132713, elastic#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");