From b5c78ba1a9815239a464dbd385ccf7566a993746 Mon Sep 17 00:00:00 2001 From: Igor Melnichenko Date: Sat, 10 Oct 2026 20:57:08 +0300 Subject: [PATCH 1/2] Ignore unreachable nodes when detecting the local DC A failed TCP probe returns Long.MAX_VALUE and detectLocalDC summed it with the other probes of the datacenter. The sum overflowed into a negative value, so a datacenter with an unreachable node - or with two of them - got the lowest average ping and was picked as the local one: e.g. probes of [1ms, unreachable] average to -4611686018426887904. Average only the successful probes and skip a datacenter whose probed nodes are all unreachable. Co-Authored-By: Claude Opus 5.5 --- .../ydb/core/impl/pool/PriorityPicker.java | 15 ++++++-- .../core/impl/pool/PriorityPickerTest.java | 34 +++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/tech/ydb/core/impl/pool/PriorityPicker.java b/core/src/main/java/tech/ydb/core/impl/pool/PriorityPicker.java index 185557261..d59f16419 100644 --- a/core/src/main/java/tech/ydb/core/impl/pool/PriorityPicker.java +++ b/core/src/main/java/tech/ydb/core/impl/pool/PriorityPicker.java @@ -90,14 +90,25 @@ static String detectLocalDC(List endpoints, Ticker ticker) { int nodeSize = Math.min(nodes.size(), DETECT_DC_NODE_SIZE); long tcpPing = 0; + int reachableCount = 0; for (EndpointRecord node : nodes.subList(0, nodeSize)) { long currentPing = tcpPing(new InetSocketAddress(node.getHost(), node.getPort()), ticker); logger.debug("Address: {}, port: {}, nanos ping: {}", node.getHost(), node.getPort(), currentPing); - tcpPing += currentPing; + + // an unreachable node tells nothing about the distance and its Long.MAX_VALUE would overflow the sum + if (currentPing != Long.MAX_VALUE) { + tcpPing += currentPing; + reachableCount++; + } + } + + if (reachableCount == 0) { + logger.debug("No reachable nodes in {}, skip it", dc); + continue; } - tcpPing /= nodeSize; + tcpPing /= reachableCount; if (minPing > tcpPing) { minPing = tcpPing; diff --git a/core/src/test/java/tech/ydb/core/impl/pool/PriorityPickerTest.java b/core/src/test/java/tech/ydb/core/impl/pool/PriorityPickerTest.java index b2abd3cbf..c11cfdb8c 100644 --- a/core/src/test/java/tech/ydb/core/impl/pool/PriorityPickerTest.java +++ b/core/src/test/java/tech/ydb/core/impl/pool/PriorityPickerTest.java @@ -2,6 +2,7 @@ import java.io.IOException; import java.net.ServerSocket; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.stream.Collectors; @@ -108,4 +109,37 @@ public void detectLocalDCTest() { throw new RuntimeException(e); } } + + @Test + public void detectLocalDCWithUnreachableNodesTest() throws IOException { + int closedPort; + + try (ServerSocket socket = ServerSocketFactory.getDefault().createServerSocket(0)) { + closedPort = socket.getLocalPort(); + } + + // every successful probe takes 10 nanos + Ticker ticker = new Ticker() { + private long now = 0; + + @Override + public long read() { + now += 10; + return now; + } + }; + + try (ServerSocket serverSocket = ServerSocketFactory.getDefault().createServerSocket(0)) { + int openPort = serverSocket.getLocalPort(); + + List records = Arrays.asList( + new EndpointRecord("localhost", closedPort, 1, "DOWN", null), + new EndpointRecord("localhost", closedPort, 2, "DOWN", null), + new EndpointRecord("localhost", openPort, 3, "UP", null) + ); + + // failed probes of DOWN must not overflow into a negative, i.e. the best, ping + Assert.assertEquals("UP", PriorityPicker.detectLocalDC(records, ticker)); + } + } } From a82809313ef202eaed5472bd3140c663c84ea3a2 Mon Sep 17 00:00:00 2001 From: Alexandr Gorshenin Date: Fri, 16 Oct 2026 15:05:14 +0100 Subject: [PATCH 2/2] Style fixes --- .../ydb/core/impl/pool/PriorityPicker.java | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/tech/ydb/core/impl/pool/PriorityPicker.java b/core/src/main/java/tech/ydb/core/impl/pool/PriorityPicker.java index d59f16419..b2d576f50 100644 --- a/core/src/main/java/tech/ydb/core/impl/pool/PriorityPicker.java +++ b/core/src/main/java/tech/ydb/core/impl/pool/PriorityPicker.java @@ -93,13 +93,13 @@ static String detectLocalDC(List endpoints, Ticker ticker) { int reachableCount = 0; for (EndpointRecord node : nodes.subList(0, nodeSize)) { - long currentPing = tcpPing(new InetSocketAddress(node.getHost(), node.getPort()), ticker); - logger.debug("Address: {}, port: {}, nanos ping: {}", node.getHost(), node.getPort(), currentPing); - - // an unreachable node tells nothing about the distance and its Long.MAX_VALUE would overflow the sum - if (currentPing != Long.MAX_VALUE) { + try { + long currentPing = tcpPing(new InetSocketAddress(node.getHost(), node.getPort()), ticker); + logger.debug("{}, nanos ping: {}", node, currentPing); tcpPing += currentPing; reachableCount++; + } catch (IOException ex) { + logger.warn("{} is not reachable with error {}", node, ex.getMessage()); } } @@ -119,14 +119,11 @@ static String detectLocalDC(List endpoints, Ticker ticker) { return localDC; } - private static long tcpPing(InetSocketAddress socketAddress, Ticker ticker) { + private static long tcpPing(InetSocketAddress socketAddress, Ticker ticker) throws IOException { try (Socket socket = SocketFactory.getDefault().createSocket()) { - final long startConnection = ticker.read(); + long started = ticker.read(); socket.connect(socketAddress, DETECT_DC_TCP_PING_TIMEOUT_MS); - final long stopConnection = ticker.read(); - return stopConnection - startConnection; - } catch (IOException e) { - return Long.MAX_VALUE; + return ticker.read() - started; } } }