From 810cf2f1ac7912f6a0b445c483f30080d43e70d5 Mon Sep 17 00:00:00 2001 From: Alexandr Gorshenin Date: Fri, 19 Apr 2024 10:50:19 +0100 Subject: [PATCH 01/15] Added location to EndpointRecord --- .../ydb/core/impl/MultiChannelTransport.java | 2 +- .../tech/ydb/core/impl/YdbTransportImpl.java | 2 +- .../tech/ydb/core/impl/pool/EndpointPool.java | 2 +- .../tech/ydb/core/impl/pool/EndpointRecord.java | 16 +++++++++++++--- .../ydb/core/impl/pool/EndpointPoolTest.java | 6 +++--- .../ydb/core/impl/pool/GrpcChannelPoolTest.java | 16 ++++++++-------- .../tech/ydb/core/impl/pool/GrpcChannelTest.java | 12 +++++++----- 7 files changed, 34 insertions(+), 22 deletions(-) diff --git a/core/src/main/java/tech/ydb/core/impl/MultiChannelTransport.java b/core/src/main/java/tech/ydb/core/impl/MultiChannelTransport.java index abc2a6cc5..19ba1d5dd 100644 --- a/core/src/main/java/tech/ydb/core/impl/MultiChannelTransport.java +++ b/core/src/main/java/tech/ydb/core/impl/MultiChannelTransport.java @@ -48,7 +48,7 @@ public MultiChannelTransport(GrpcTransportBuilder builder, List hos List endpoints = new ArrayList<>(); DiscoveryProtos.ListEndpointsResult.Builder discoveryBuilder = DiscoveryProtos.ListEndpointsResult.newBuilder(); hosts.forEach(host -> { - endpoints.add(new EndpointRecord(host.getHost(), host.getPortOrDefault(YdbTransportImpl.DEFAULT_PORT), 0)); + endpoints.add(new EndpointRecord(host.getHost(), host.getPortOrDefault(YdbTransportImpl.DEFAULT_PORT))); discoveryBuilder.addEndpointsBuilder() .setAddress(host.getHost()) .setPort(host.getPortOrDefault(YdbTransportImpl.DEFAULT_PORT)) diff --git a/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java b/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java index dabddbe88..64c00b05b 100644 --- a/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java +++ b/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java @@ -107,7 +107,7 @@ static EndpointRecord getDiscoveryEndpoint(GrpcTransportBuilder builder) { + "endpoint " + builder.getEndpoint() + " and empty host " + builder.getHost()); } - return new EndpointRecord(endpointURI.getHost(), endpointURI.getPort(), 0); + return new EndpointRecord(endpointURI.getHost(), endpointURI.getPort()); } private static BalancingSettings getBalancingSettings(GrpcTransportBuilder builder) { diff --git a/core/src/main/java/tech/ydb/core/impl/pool/EndpointPool.java b/core/src/main/java/tech/ydb/core/impl/pool/EndpointPool.java index 3695c8bd8..73098edc2 100644 --- a/core/src/main/java/tech/ydb/core/impl/pool/EndpointPool.java +++ b/core/src/main/java/tech/ydb/core/impl/pool/EndpointPool.java @@ -182,7 +182,7 @@ static class PriorityEndpoint extends EndpointRecord { private long priority; PriorityEndpoint(DiscoveryProtos.EndpointInfo endpoint, long priority) { - super(endpoint.getAddress(), endpoint.getPort(), endpoint.getNodeId()); + super(endpoint.getAddress(), endpoint.getPort(), endpoint.getNodeId(), endpoint.getLocation()); this.priority = priority; } diff --git a/core/src/main/java/tech/ydb/core/impl/pool/EndpointRecord.java b/core/src/main/java/tech/ydb/core/impl/pool/EndpointRecord.java index 1c8cbbaa0..77e557473 100644 --- a/core/src/main/java/tech/ydb/core/impl/pool/EndpointRecord.java +++ b/core/src/main/java/tech/ydb/core/impl/pool/EndpointRecord.java @@ -7,15 +7,21 @@ */ public class EndpointRecord { private final String host; - private final int port; private final String hostAndPort; + private final String locationDC; + private final int port; private final int nodeId; - public EndpointRecord(String host, int port, int nodeId) { + public EndpointRecord(String host, int port, int nodeId, String locationDC) { this.host = Objects.requireNonNull(host); this.port = port; this.hostAndPort = host + ":" + port; this.nodeId = nodeId; + this.locationDC = locationDC; + } + + public EndpointRecord(String host, int port) { + this(host, port, 0, null); } public String getHost() { @@ -34,8 +40,12 @@ public int getNodeId() { return nodeId; } + public String getLocation() { + return locationDC; + } + @Override public String toString() { - return "Endpoint{host=" + host + ", port=" + port + ", node=" + nodeId + "}"; + return "Endpoint{host=" + host + ", port=" + port + ", node=" + nodeId + ", location=" + locationDC + "}"; } } diff --git a/core/src/test/java/tech/ydb/core/impl/pool/EndpointPoolTest.java b/core/src/test/java/tech/ydb/core/impl/pool/EndpointPoolTest.java index 6be75aede..0a7e3ef26 100644 --- a/core/src/test/java/tech/ydb/core/impl/pool/EndpointPoolTest.java +++ b/core/src/test/java/tech/ydb/core/impl/pool/EndpointPoolTest.java @@ -32,7 +32,7 @@ * @author Kirill Kurdyukov */ public class EndpointPoolTest { - private final EndpointRecord discovery = new EndpointRecord("discovery", 2136, -1); + private final EndpointRecord discovery = new EndpointRecord("discovery", 2136, -1, null); private final AutoCloseable mocks = MockitoAnnotations.openMocks(this); private final MockedStatic threadLocalStaticMock = mockStatic(ThreadLocalRandom.class); @@ -209,8 +209,8 @@ public void nodePessimizationTest() { check(pool.getEndpoint(2)).hostname("n2.ydb.tech").nodeID(2).port(12342); // Pessimize unknown nodes - nothing is changed - pool.pessimizeEndpoint(new EndpointRecord("n2.ydb.tech", 12341, 2)); - pool.pessimizeEndpoint(new EndpointRecord("n2.ydb.tech", 12342, 2)); + pool.pessimizeEndpoint(new EndpointRecord("n2.ydb.tech", 12341, 2, null)); + pool.pessimizeEndpoint(new EndpointRecord("n2.ydb.tech", 12342, 2, null)); pool.pessimizeEndpoint(null); check(pool).records(5).knownNodes(5).needToReDiscovery(false).bestEndpointsCount(4); diff --git a/core/src/test/java/tech/ydb/core/impl/pool/GrpcChannelPoolTest.java b/core/src/test/java/tech/ydb/core/impl/pool/GrpcChannelPoolTest.java index 58150cf68..a5b3fdfbd 100644 --- a/core/src/test/java/tech/ydb/core/impl/pool/GrpcChannelPoolTest.java +++ b/core/src/test/java/tech/ydb/core/impl/pool/GrpcChannelPoolTest.java @@ -34,8 +34,8 @@ public void tearDown() throws Exception { @Test public void simpleTest() { - EndpointRecord e1 = new EndpointRecord("host1", 1234, 10); - EndpointRecord e2 = new EndpointRecord("host1", 1235, 11); + EndpointRecord e1 = new EndpointRecord("host1", 1234, 10, null); + EndpointRecord e2 = new EndpointRecord("host1", 1235, 11, null); GrpcChannelPool pool = new GrpcChannelPool(factoryMock, scheduler); Assert.assertEquals(0, pool.getChannels().size()); @@ -66,9 +66,9 @@ public void simpleTest() { @Test public void removeChannels() { - EndpointRecord e1 = new EndpointRecord("host1", 1234, 10); - EndpointRecord e2 = new EndpointRecord("host1", 1235, 11); - EndpointRecord e3 = new EndpointRecord("host1", 1236, 12); + EndpointRecord e1 = new EndpointRecord("host1", 1234, 10, null); + EndpointRecord e2 = new EndpointRecord("host1", 1235, 11, null); + EndpointRecord e3 = new EndpointRecord("host1", 1236, 12, null); GrpcChannelPool pool = new GrpcChannelPool(factoryMock, scheduler); Assert.assertEquals(0, pool.getChannels().size()); @@ -125,9 +125,9 @@ public void badShutdownTest() { ManagedChannelMock.good(), ManagedChannelMock.good(), ManagedChannelMock.wrongShutdown(), ManagedChannelMock.wrongShutdown()); - EndpointRecord e1 = new EndpointRecord("host1", 1234, 10); - EndpointRecord e2 = new EndpointRecord("host1", 1235, 11); - EndpointRecord e3 = new EndpointRecord("host1", 1236, 12); + EndpointRecord e1 = new EndpointRecord("host1", 1234, 10, null); + EndpointRecord e2 = new EndpointRecord("host1", 1235, 11, null); + EndpointRecord e3 = new EndpointRecord("host1", 1236, 12, null); GrpcChannelPool pool = new GrpcChannelPool(factoryMock, scheduler); Assert.assertEquals(0, pool.getChannels().size()); diff --git a/core/src/test/java/tech/ydb/core/impl/pool/GrpcChannelTest.java b/core/src/test/java/tech/ydb/core/impl/pool/GrpcChannelTest.java index 21cdfd498..3368aa3f9 100644 --- a/core/src/test/java/tech/ydb/core/impl/pool/GrpcChannelTest.java +++ b/core/src/test/java/tech/ydb/core/impl/pool/GrpcChannelTest.java @@ -29,7 +29,7 @@ public void goodChannels() { Mockito.when(factoryMock.newManagedChannel(Mockito.any(), Mockito.anyInt())) .thenReturn(ManagedChannelMock.good(), ManagedChannelMock.good()); - EndpointRecord endpoint = new EndpointRecord("host1", 1234, 0); + EndpointRecord endpoint = new EndpointRecord("host1", 1234); GrpcChannel lazy = new GrpcChannel(endpoint, factoryMock, false); Assert.assertEquals(endpoint, lazy.getEndpoint()); @@ -61,7 +61,7 @@ public void slowChannels() { .thenReturn(new ManagedChannelMock(ConnectivityState.IDLE).nextStates(states)) .thenReturn(new ManagedChannelMock(ConnectivityState.IDLE).nextStates(states)); - EndpointRecord endpoint = new EndpointRecord("host1", 1234, 0); + EndpointRecord endpoint = new EndpointRecord("host1", 1234); GrpcChannel lazy = new GrpcChannel(endpoint, factoryMock, false); Assert.assertEquals(endpoint, lazy.getEndpoint()); @@ -88,13 +88,14 @@ public void badChannels() { .thenReturn(new ManagedChannelMock(ConnectivityState.IDLE).nextStates(states)) .thenReturn(new ManagedChannelMock(ConnectivityState.IDLE).nextStates(states)); - EndpointRecord endpoint = new EndpointRecord("host1", 1234, 0); + EndpointRecord endpoint = new EndpointRecord("host1", 1234); GrpcChannel lazy = new GrpcChannel(endpoint, factoryMock, false); Assert.assertEquals(endpoint, lazy.getEndpoint()); RuntimeException ex1 = Assert.assertThrows(RuntimeException.class, lazy::getReadyChannel); - Assert.assertEquals("Channel Endpoint{host=host1, port=1234, node=0} connecting problem", ex1.getMessage()); + Assert.assertEquals("Channel Endpoint{host=host1, port=1234, node=0, location=null} connecting problem", + ex1.getMessage()); lazy.shutdown(); @@ -102,7 +103,8 @@ public void badChannels() { Assert.assertEquals(endpoint, eager.getEndpoint()); RuntimeException ex2 = Assert.assertThrows(RuntimeException.class, eager::getReadyChannel); - Assert.assertEquals("Channel Endpoint{host=host1, port=1234, node=0} connecting problem", ex2.getMessage()); + Assert.assertEquals("Channel Endpoint{host=host1, port=1234, node=0, location=null} connecting problem", + ex2.getMessage()); eager.shutdown(); } From 69a45e05be4a01aa12ff8bcfd0c5941d2dce2a4d Mon Sep 17 00:00:00 2001 From: Alexandr Gorshenin Date: Fri, 19 Apr 2024 10:24:37 +0100 Subject: [PATCH 02/15] Update implementation of priority picker --- .../tech/ydb/core/impl/pool/EndpointPool.java | 13 +- .../impl/pool/EndpointPriorityFactory.java | 147 ------------------ .../ydb/core/impl/pool/PriorityPicker.java | 117 ++++++++++++++ .../pool/EndpointPriorityFactoryTest.java | 130 ---------------- .../core/impl/pool/PriorityPickerTest.java | 85 ++++++++++ 5 files changed, 211 insertions(+), 281 deletions(-) delete mode 100644 core/src/main/java/tech/ydb/core/impl/pool/EndpointPriorityFactory.java create mode 100644 core/src/main/java/tech/ydb/core/impl/pool/PriorityPicker.java delete mode 100644 core/src/test/java/tech/ydb/core/impl/pool/EndpointPriorityFactoryTest.java create mode 100644 core/src/test/java/tech/ydb/core/impl/pool/PriorityPickerTest.java diff --git a/core/src/main/java/tech/ydb/core/impl/pool/EndpointPool.java b/core/src/main/java/tech/ydb/core/impl/pool/EndpointPool.java index 73098edc2..47397c1ba 100644 --- a/core/src/main/java/tech/ydb/core/impl/pool/EndpointPool.java +++ b/core/src/main/java/tech/ydb/core/impl/pool/EndpointPool.java @@ -11,6 +11,7 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.stream.Collectors; import javax.annotation.Nullable; @@ -71,9 +72,12 @@ public EndpointRecord getEndpoint(@Nullable Integer preferredNodeID) { // Sets new endpoints, returns removed public List setNewState(DiscoveryProtos.ListEndpointsResult result) { - EndpointPriorityFactory priorityFactory = new EndpointPriorityFactory( - balancingSettings, - result + PriorityPicker priorityPicker = PriorityPicker.from(balancingSettings, + result.getSelfLocation(), + result.getEndpointsList() + .stream() + .map(e -> new EndpointRecord(e.getAddress(), e.getPort(), e.getNodeId(), e.getLocation())) + .collect(Collectors.toList()) ); Set newKnownEndpoints = new HashSet<>(); @@ -82,7 +86,8 @@ public List setNewState(DiscoveryProtos.ListEndpointsResult resu logger.debug("init new state with {} endpoints", result.getEndpointsCount()); for (DiscoveryProtos.EndpointInfo info : result.getEndpointsList()) { - PriorityEndpoint entry = priorityFactory.createEndpoint(info); + int priority = priorityPicker.getEndpointPriority(info.getLocation()); + PriorityEndpoint entry = new PriorityEndpoint(info, priority); String endpoint = entry.getHostAndPort(); if (!newKnownEndpoints.contains(endpoint)) { diff --git a/core/src/main/java/tech/ydb/core/impl/pool/EndpointPriorityFactory.java b/core/src/main/java/tech/ydb/core/impl/pool/EndpointPriorityFactory.java deleted file mode 100644 index a823bd3dc..000000000 --- a/core/src/main/java/tech/ydb/core/impl/pool/EndpointPriorityFactory.java +++ /dev/null @@ -1,147 +0,0 @@ -package tech.ydb.core.impl.pool; - -import java.io.IOException; -import java.net.InetSocketAddress; -import java.net.Socket; -import java.util.Collections; -import java.util.List; -import java.util.Map; -import java.util.stream.Collectors; - -import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Ticker; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import tech.ydb.core.grpc.BalancingSettings; -import tech.ydb.proto.discovery.DiscoveryProtos; - -/** - * @author Kirill Kurdyukov - */ -public class EndpointPriorityFactory { - - private static final Logger logger = LoggerFactory - .getLogger(EndpointPriorityFactory.class); - private static final int LOCALITY_SHIFT = 1000; - private static final int NODE_SIZE = 3; - private static final int TCP_PING_TIMEOUT_MS = 5000; - - private final String locationDC; - - public EndpointPriorityFactory( - BalancingSettings settings, - DiscoveryProtos.ListEndpointsResult endpointsResult - ) { - this(settings, endpointsResult, Ticker.systemTicker()); - } - - @VisibleForTesting - EndpointPriorityFactory( - BalancingSettings settings, - DiscoveryProtos.ListEndpointsResult endpointsResult, - Ticker ticker - ) { - switch (settings.getPolicy()) { - case USE_ALL_NODES: - locationDC = null; - break; - case USE_PREFERABLE_LOCATION: - String preferred = settings.getPreferableLocation(); - - if (preferred == null || preferred.isEmpty()) { - preferred = endpointsResult.getSelfLocation(); - } - - locationDC = preferred; - break; - case USE_DETECT_LOCAL_DC: - locationDC = detectLocalDC(endpointsResult, ticker); - break; - default: - throw new RuntimeException("Not implemented balancing policy: " - + settings.getPolicy().name()); - } - } - - public EndpointPool.PriorityEndpoint createEndpoint( - DiscoveryProtos.EndpointInfo endpointInfo - ) { - return new EndpointPool.PriorityEndpoint( - endpointInfo, - locationDC == null - ? 0 : locationDC.equalsIgnoreCase(endpointInfo.getLocation()) - ? 0 : LOCALITY_SHIFT - ); - } - - private static String detectLocalDC( - DiscoveryProtos.ListEndpointsResult endpointsResult, - Ticker ticker - ) { - Map> dcLocationToNodes = endpointsResult - .getEndpointsList() - .stream() - .collect(Collectors - .groupingBy(DiscoveryProtos.EndpointInfo::getLocation) - ); - - if (dcLocationToNodes.size() < 2) { - return null; - } - - long minPing = Long.MAX_VALUE; - String localDC = null; - - for (Map.Entry> entry : dcLocationToNodes.entrySet()) { - String dc = entry.getKey(); - List nodes = entry.getValue(); - - assert !nodes.isEmpty(); - - Collections.shuffle(nodes); - - int nodeSize = Math.min(nodes.size(), NODE_SIZE); - long tcpPing = 0; - - for (DiscoveryProtos.EndpointInfo node : nodes.subList(0, nodeSize)) { - long currentPing = tcpPing( - new InetSocketAddress(node.getAddress(), node.getPort()), - ticker - ); - logger.debug("Address: {}, port: {}, nanos ping: {}", node.getAddress(), node.getPort(), currentPing); - - tcpPing += currentPing; - } - - tcpPing /= nodeSize; - - if (minPing > tcpPing) { - minPing = tcpPing; - localDC = dc; - } - } - - return localDC; - } - - private static long tcpPing( - InetSocketAddress socketAddress, - Ticker ticker - ) { - try (Socket socket = new Socket()) { - final long startConnection = ticker.read(); - - socket.connect( - socketAddress, - TCP_PING_TIMEOUT_MS - ); - - final long stopConnection = ticker.read(); - - return stopConnection - startConnection; - } catch (IOException e) { - return TCP_PING_TIMEOUT_MS * 2_000_000L; - } - } -} 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 new file mode 100644 index 000000000..e7c501059 --- /dev/null +++ b/core/src/main/java/tech/ydb/core/impl/pool/PriorityPicker.java @@ -0,0 +1,117 @@ +package tech.ydb.core.impl.pool; + +import java.io.IOException; +import java.net.InetSocketAddress; +import java.net.Socket; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Ticker; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import tech.ydb.core.grpc.BalancingSettings; + +/** + * @author Kirill Kurdyukov + */ +public class PriorityPicker { + private static final Logger logger = LoggerFactory.getLogger(PriorityPicker.class); + + private static final int LOCALITY_SHIFT = 1000; + private static final int DETECT_DC_NODE_SIZE = 3; + private static final int DETECT_DC_TCP_PING_TIMEOUT_MS = 5000; + + private final String prefferedLocation; + + private PriorityPicker(String location) { + this.prefferedLocation = location; + } + + public int getEndpointPriority(String location) { + if (prefferedLocation == null || prefferedLocation.equalsIgnoreCase(location)) { + return 0; + } + + return LOCALITY_SHIFT; + } + + public static PriorityPicker from(BalancingSettings settings, String selfLocation, List endpoints) { + switch (settings.getPolicy()) { + case USE_ALL_NODES: + return new PriorityPicker(null); + case USE_PREFERABLE_LOCATION: + return new PriorityPicker(getLocationFromConfig(settings.getPreferableLocation(), selfLocation)); + case USE_DETECT_LOCAL_DC: + return new PriorityPicker(detectLocalDC(endpoints, Ticker.systemTicker())); + default: + throw new RuntimeException("Not implemented balancing policy: " + settings.getPolicy().name()); + } + } + + @VisibleForTesting + static String getLocationFromConfig(String prefferable, String selfLocation) { + if (prefferable != null && !prefferable.isEmpty()) { + return prefferable; + } + if (selfLocation != null && !selfLocation.isEmpty()) { + return selfLocation; + } + return null; + } + + @VisibleForTesting + static String detectLocalDC(List endpoints, Ticker ticker) { + Map> dcLocationToNodes = endpoints + .stream() + .collect(Collectors.groupingBy(EndpointRecord::getLocation)); + + if (dcLocationToNodes.size() < 2) { + return null; + } + + long minPing = Long.MAX_VALUE; + String localDC = null; + + for (Map.Entry> entry : dcLocationToNodes.entrySet()) { + String dc = entry.getKey(); + List nodes = entry.getValue(); + + assert !nodes.isEmpty(); + + Collections.shuffle(nodes); + + int nodeSize = Math.min(nodes.size(), DETECT_DC_NODE_SIZE); + long tcpPing = 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; + } + + tcpPing /= nodeSize; + + if (minPing > tcpPing) { + minPing = tcpPing; + localDC = dc; + } + } + + return localDC; + } + + private static long tcpPing(InetSocketAddress socketAddress, Ticker ticker) { + try (Socket socket = new Socket()) { + final long startConnection = 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; + } + } +} diff --git a/core/src/test/java/tech/ydb/core/impl/pool/EndpointPriorityFactoryTest.java b/core/src/test/java/tech/ydb/core/impl/pool/EndpointPriorityFactoryTest.java deleted file mode 100644 index 6c648493c..000000000 --- a/core/src/test/java/tech/ydb/core/impl/pool/EndpointPriorityFactoryTest.java +++ /dev/null @@ -1,130 +0,0 @@ -package tech.ydb.core.impl.pool; - -import java.io.IOException; -import java.net.ServerSocket; -import java.util.Arrays; - -import javax.net.ServerSocketFactory; - -import org.junit.Assert; -import org.junit.Test; - -import tech.ydb.core.grpc.BalancingSettings; -import tech.ydb.core.timer.TestTicker; -import tech.ydb.proto.discovery.DiscoveryProtos; - -/** - * @author Kirill - */ -public class EndpointPriorityFactoryTest { - - @Test - public void randomEvaluatorTest() { - EndpointPriorityFactory priorityFactory = new EndpointPriorityFactory( - BalancingSettings.defaultInstance(), - list(endpoint("DC1")) - ); - - Assert.assertEquals( - 0, - priorityFactory - .createEndpoint(endpoint("DC1")) - .getPriority() - ); - } - - - @Test - public void localDCFixedTest() { - EndpointPriorityFactory priorityFactory = new EndpointPriorityFactory( - BalancingSettings.fromLocation("DC1"), - list( - endpoint("DC1"), - endpoint("DC2") - ) - ); - - Assert.assertEquals( - 0, - priorityFactory - .createEndpoint(endpoint("DC1")) - .getPriority() - ); - - Assert.assertEquals( - 1000, - priorityFactory - .createEndpoint(endpoint("DC2")) - .getPriority() - ); // shift - } - - @Test - public void detectLocalDCTest() { - TestTicker testTicker = new TestTicker( - 9, 15, - 16, 50, - 51, 74, - 75, 77, - 78, 82, - 83, 125 - ); - - try (ServerSocket serverSocket = ServerSocketFactory.getDefault().createServerSocket(8080)) { - Assert.assertFalse(serverSocket.isClosed()); - - EndpointPriorityFactory priorityFactory = new EndpointPriorityFactory( - BalancingSettings.detectLocalDs(), - list( - endpoint("DC1"), - endpoint("DC1"), - endpoint("DC2"), - endpoint("DC2"), - endpoint("DC2"), - endpoint("DC3") - ), - testTicker - ); - - Assert.assertEquals( - 0, - priorityFactory - .createEndpoint(endpoint("DC1")) - .getPriority() - ); - - Assert.assertEquals( - 1000, - priorityFactory - .createEndpoint(endpoint("DC2")) - .getPriority() - ); - - Assert.assertEquals( - 1000, - priorityFactory - .createEndpoint(endpoint("DC3")) - .getPriority() - ); - } catch (IOException e) { - throw new RuntimeException(e); - } - } - - private static DiscoveryProtos.EndpointInfo endpoint(String location) { - return DiscoveryProtos.EndpointInfo.newBuilder() - .setAddress("localhost") - .setPort(8080) - .setNodeId(1) - .setLocation(location) - .build(); - } - - private static DiscoveryProtos.ListEndpointsResult list(DiscoveryProtos.EndpointInfo... endpoints) { - return DiscoveryProtos.ListEndpointsResult.newBuilder() - .setSelfLocation("DC1") - .addAllEndpoints(Arrays.asList(endpoints)) - .build(); - } - -} 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 new file mode 100644 index 000000000..089b10549 --- /dev/null +++ b/core/src/test/java/tech/ydb/core/impl/pool/PriorityPickerTest.java @@ -0,0 +1,85 @@ +package tech.ydb.core.impl.pool; + +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; + +import javax.net.ServerSocketFactory; + +import org.junit.Assert; +import org.junit.Test; + +import tech.ydb.core.grpc.BalancingSettings; +import tech.ydb.core.timer.TestTicker; + +/** + * @author Kirill + */ +public class PriorityPickerTest { + + @Test + public void randomEvaluatorTest() { + PriorityPicker picker = PriorityPicker.from(BalancingSettings.defaultInstance(), null, null); + Assert.assertEquals(0, picker.getEndpointPriority("DC1")); + Assert.assertEquals(0, picker.getEndpointPriority("DC2")); + Assert.assertEquals(0, picker.getEndpointPriority("DC3")); + } + + + @Test + public void fixedLocalDcTest() { + PriorityPicker ignoreSelftLocation = PriorityPicker.from(BalancingSettings.defaultInstance(), "DC1", null); + Assert.assertEquals(0, ignoreSelftLocation.getEndpointPriority("dC1")); + Assert.assertEquals(0, ignoreSelftLocation.getEndpointPriority("Dc2")); + Assert.assertEquals(0, ignoreSelftLocation.getEndpointPriority("Dc3")); + + PriorityPicker useSelfLocation = PriorityPicker.from(BalancingSettings.fromLocation(""), "DC1", null); + Assert.assertEquals(0, useSelfLocation.getEndpointPriority("dC1")); + Assert.assertEquals(1000, useSelfLocation.getEndpointPriority("Dc2")); + Assert.assertEquals(1000, useSelfLocation.getEndpointPriority("Dc3")); + + PriorityPicker useLocalDC = PriorityPicker.from(BalancingSettings.fromLocation("DC2"), "DC1", null); + Assert.assertEquals(1000, useLocalDC.getEndpointPriority("dC1")); + Assert.assertEquals(0, useLocalDC.getEndpointPriority("Dc2")); + Assert.assertEquals(1000, useLocalDC.getEndpointPriority("Dc3")); + } + + @Test + public void detectLocalDCfallbackTest() { + List single = Collections.singletonList(new EndpointRecord("localhost", 8080, 0, "DC1")); + PriorityPicker ignoreSelftLocation = PriorityPicker.from(BalancingSettings.detectLocalDs(), "DC1", single); + + Assert.assertEquals(0, ignoreSelftLocation.getEndpointPriority("DC1")); + Assert.assertEquals(0, ignoreSelftLocation.getEndpointPriority("DC2")); + Assert.assertEquals(0, ignoreSelftLocation.getEndpointPriority("DC3")); + } + + @Test + public void detectLocalDCTest() { + TestTicker testTicker = new TestTicker( + 9, 15, + 16, 50, + 51, 74, + 75, 77, + 78, 82, + 83, 125 + ); + + try (ServerSocket serverSocket = ServerSocketFactory.getDefault().createServerSocket(0)) { + Assert.assertFalse(serverSocket.isClosed()); + final int port = serverSocket.getLocalPort(); + + List records = Arrays.asList("DC1", "DC1", "DC2", "DC2", "DC2", "DC3") + .stream().map(location -> new EndpointRecord("localhost", port, 1, location)) + .collect(Collectors.toList()); + + String localDC = PriorityPicker.detectLocalDC(records, testTicker); + Assert.assertEquals("DC1", localDC); + } catch (IOException e) { + throw new RuntimeException(e); + } + } +} From 27d6f7824c04d885de3aa08f2505ae6b4cc2a675 Mon Sep 17 00:00:00 2001 From: Alexandr Gorshenin Date: Fri, 19 Apr 2024 10:56:29 +0100 Subject: [PATCH 03/15] Added shutdown method to BaseGrpcTransport --- .../tech/ydb/core/impl/BaseGrpcTransport.java | 21 ++++++++++------ .../core/impl/FixedCallOptionsTransport.java | 5 ++-- .../ydb/core/impl/MultiChannelTransport.java | 7 ++---- .../ydb/core/impl/SingleChannelTransport.java | 7 ++---- .../tech/ydb/core/impl/YdbTransportImpl.java | 25 ++++++++----------- 5 files changed, 30 insertions(+), 35 deletions(-) diff --git a/core/src/main/java/tech/ydb/core/impl/BaseGrpcTransport.java b/core/src/main/java/tech/ydb/core/impl/BaseGrpcTransport.java index 2ff42af96..8dd790be4 100644 --- a/core/src/main/java/tech/ydb/core/impl/BaseGrpcTransport.java +++ b/core/src/main/java/tech/ydb/core/impl/BaseGrpcTransport.java @@ -2,6 +2,7 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import io.grpc.CallOptions; import io.grpc.ClientCall; @@ -41,15 +42,21 @@ public abstract class BaseGrpcTransport implements GrpcTransport { .withIssues(Issue.of("Request was not sent: transport is shutting down", Issue.Severity.ERROR) )); - protected volatile boolean shutdown; + private final AtomicBoolean isClosed = new AtomicBoolean(false); - abstract AuthCallOptions getAuthCallOptions(); + protected abstract AuthCallOptions getAuthCallOptions(); protected abstract GrpcChannel getChannel(GrpcRequestSettings settings); - abstract void updateChannelStatus(GrpcChannel channel, io.grpc.Status status); + protected abstract void updateChannelStatus(GrpcChannel channel, io.grpc.Status status); + + protected void shutdown() { + // nothing to shutdown + } @Override public void close() { - this.shutdown = true; + if (isClosed.compareAndSet(false, true)) { + shutdown(); + } } @Override @@ -58,7 +65,7 @@ public CompletableFuture> unaryCall( GrpcRequestSettings settings, ReqT request ) { - if (shutdown) { + if (isClosed.get()) { return CompletableFuture.completedFuture(SHUTDOWN_RESULT.map(null)); } @@ -97,7 +104,7 @@ public GrpcReadStream readStreamCall( GrpcRequestSettings settings, ReqT request ) { - if (shutdown) { + if (isClosed.get()) { return new EmptyStream<>(SHUTDOWN_RESULT.getStatus()); } @@ -137,7 +144,7 @@ public GrpcReadWriteStream readWriteStreamCall( MethodDescriptor method, GrpcRequestSettings settings ) { - if (shutdown) { + if (isClosed.get()) { return new EmptyStream<>(SHUTDOWN_RESULT.getStatus()); } diff --git a/core/src/main/java/tech/ydb/core/impl/FixedCallOptionsTransport.java b/core/src/main/java/tech/ydb/core/impl/FixedCallOptionsTransport.java index 43c832929..b204be802 100644 --- a/core/src/main/java/tech/ydb/core/impl/FixedCallOptionsTransport.java +++ b/core/src/main/java/tech/ydb/core/impl/FixedCallOptionsTransport.java @@ -47,8 +47,7 @@ public String getDatabase() { } @Override - public void close() { - super.close(); + protected void shutdown() { channel.shutdown(); } @@ -63,7 +62,7 @@ protected GrpcChannel getChannel(GrpcRequestSettings settings) { } @Override - void updateChannelStatus(GrpcChannel channel, Status status) { + protected void updateChannelStatus(GrpcChannel channel, Status status) { if (!status.isOk()) { logger.warn("grpc error {}[{}] on fixed channel {}", status.getCode(), diff --git a/core/src/main/java/tech/ydb/core/impl/MultiChannelTransport.java b/core/src/main/java/tech/ydb/core/impl/MultiChannelTransport.java index 19ba1d5dd..8a88335a2 100644 --- a/core/src/main/java/tech/ydb/core/impl/MultiChannelTransport.java +++ b/core/src/main/java/tech/ydb/core/impl/MultiChannelTransport.java @@ -80,12 +80,9 @@ public String getDatabase() { } @Override - public void close() { - super.close(); - + protected void shutdown() { channelPool.shutdown(); callOptions.close(); - YdbSchedulerFactory.shutdownScheduler(scheduler); } @@ -101,7 +98,7 @@ protected GrpcChannel getChannel(GrpcRequestSettings settings) { } @Override - void updateChannelStatus(GrpcChannel channel, Status status) { + protected void updateChannelStatus(GrpcChannel channel, Status status) { if (!status.isOk()) { endpointPool.pessimizeEndpoint(channel.getEndpoint()); diff --git a/core/src/main/java/tech/ydb/core/impl/SingleChannelTransport.java b/core/src/main/java/tech/ydb/core/impl/SingleChannelTransport.java index 039900784..7c60c7da6 100644 --- a/core/src/main/java/tech/ydb/core/impl/SingleChannelTransport.java +++ b/core/src/main/java/tech/ydb/core/impl/SingleChannelTransport.java @@ -55,12 +55,9 @@ public String getDatabase() { } @Override - public void close() { - super.close(); - + protected void shutdown() { channel.shutdown(); callOptions.close(); - YdbSchedulerFactory.shutdownScheduler(scheduler); } @@ -75,7 +72,7 @@ protected GrpcChannel getChannel(GrpcRequestSettings settings) { } @Override - void updateChannelStatus(GrpcChannel channel, Status status) { + protected void updateChannelStatus(GrpcChannel channel, Status status) { if (!status.isOk()) { logger.warn("grpc error {}[{}] on single channel {}", status.getCode(), diff --git a/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java b/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java index 64c00b05b..247df53f4 100644 --- a/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java +++ b/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java @@ -83,6 +83,15 @@ public void initAsync(Runnable readyWatcher) { periodicDiscoveryTask.startAsync(readyWatcher); } + @Override + protected void shutdown() { + periodicDiscoveryTask.stop(); + channelPool.shutdown(); + callOptions.close(); + + YdbSchedulerFactory.shutdownScheduler(scheduler); + } + static EndpointRecord getDiscoveryEndpoint(GrpcTransportBuilder builder) { URI endpointURI = null; try { @@ -134,20 +143,6 @@ public String getDatabase() { return database; } - @Override - public void close() { - if (shutdown) { - return; - } - super.close(); - - periodicDiscoveryTask.stop(); - channelPool.shutdown(); - callOptions.close(); - - YdbSchedulerFactory.shutdownScheduler(scheduler); - } - @Override public AuthCallOptions getAuthCallOptions() { return callOptions; @@ -160,7 +155,7 @@ protected GrpcChannel getChannel(GrpcRequestSettings settings) { } @Override - void updateChannelStatus(GrpcChannel channel, Status status) { + protected void updateChannelStatus(GrpcChannel channel, io.grpc.Status status) { // Usally CANCELLED is received when ClientCall is canceled on client side if (!status.isOk() && status.getCode() != Status.Code.CANCELLED) { endpointPool.pessimizeEndpoint(channel.getEndpoint()); From c546189f367c3847801c4872ea7ebb63745c040b Mon Sep 17 00:00:00 2001 From: Alexandr Gorshenin Date: Fri, 19 Apr 2024 11:16:38 +0100 Subject: [PATCH 04/15] Update implemenation of EndpointPool --- .../ydb/core/impl/MultiChannelTransport.java | 27 +-- .../tech/ydb/core/impl/YdbTransportImpl.java | 8 +- .../tech/ydb/core/impl/pool/EndpointPool.java | 163 ++++++++-------- .../ydb/core/impl/pool/EndpointPoolTest.java | 181 ++++++++---------- 4 files changed, 171 insertions(+), 208 deletions(-) diff --git a/core/src/main/java/tech/ydb/core/impl/MultiChannelTransport.java b/core/src/main/java/tech/ydb/core/impl/MultiChannelTransport.java index 8a88335a2..d1ddbfaba 100644 --- a/core/src/main/java/tech/ydb/core/impl/MultiChannelTransport.java +++ b/core/src/main/java/tech/ydb/core/impl/MultiChannelTransport.java @@ -1,9 +1,9 @@ package tech.ydb.core.impl; -import java.util.ArrayList; import java.util.List; import java.util.Objects; import java.util.concurrent.ScheduledExecutorService; +import java.util.stream.Collectors; import com.google.common.base.Strings; import com.google.common.net.HostAndPort; @@ -20,7 +20,6 @@ import tech.ydb.core.impl.pool.GrpcChannel; import tech.ydb.core.impl.pool.GrpcChannelPool; import tech.ydb.core.impl.pool.ManagedChannelFactory; -import tech.ydb.proto.discovery.DiscoveryProtos; /** * @@ -32,7 +31,7 @@ public class MultiChannelTransport extends BaseGrpcTransport { private final String database; private final AuthCallOptions callOptions; - private final DiscoveryProtos.ListEndpointsResult discoveryResult; + private final List endpoints; private final EndpointPool endpointPool; private final GrpcChannelPool channelPool; private final ScheduledExecutorService scheduler; @@ -45,17 +44,10 @@ public MultiChannelTransport(GrpcTransportBuilder builder, List hos this.database = Strings.nullToEmpty(builder.getDatabase()); this.scheduler = builder.getSchedulerFactory().get(); - List endpoints = new ArrayList<>(); - DiscoveryProtos.ListEndpointsResult.Builder discoveryBuilder = DiscoveryProtos.ListEndpointsResult.newBuilder(); - hosts.forEach(host -> { - endpoints.add(new EndpointRecord(host.getHost(), host.getPortOrDefault(YdbTransportImpl.DEFAULT_PORT))); - discoveryBuilder.addEndpointsBuilder() - .setAddress(host.getHost()) - .setPort(host.getPortOrDefault(YdbTransportImpl.DEFAULT_PORT)) - .build(); - }); - - this.discoveryResult = discoveryBuilder.build(); + this.endpoints = hosts.stream() + .map(host -> new EndpointRecord(host.getHost(), host.getPortOrDefault(YdbTransportImpl.DEFAULT_PORT))) + .collect(Collectors.toList()); + this.callOptions = new AuthCallOptions(scheduler, database, endpoints, @@ -64,9 +56,8 @@ public MultiChannelTransport(GrpcTransportBuilder builder, List hos ); this.channelPool = new GrpcChannelPool(channelFactory, scheduler); - this.endpointPool = new EndpointPool(null, BalancingSettings.defaultInstance()); - - this.endpointPool.setNewState(discoveryResult); + this.endpointPool = new EndpointPool(BalancingSettings.defaultInstance()); + this.endpointPool.setNewState(null, endpoints); } @Override @@ -103,7 +94,7 @@ protected void updateChannelStatus(GrpcChannel channel, Status status) { endpointPool.pessimizeEndpoint(channel.getEndpoint()); if (endpointPool.needToRunDiscovery()) { - endpointPool.setNewState(discoveryResult); + endpointPool.setNewState(null, endpoints); } } } diff --git a/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java b/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java index 247df53f4..fc793ef0f 100644 --- a/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java +++ b/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java @@ -6,6 +6,7 @@ import java.util.Collections; import java.util.List; import java.util.concurrent.ScheduledExecutorService; +import java.util.stream.Collectors; import com.google.common.base.Strings; import com.google.common.net.HostAndPort; @@ -65,7 +66,7 @@ public YdbTransportImpl(GrpcTransportBuilder builder) { Duration.ofMillis(builder.getDiscoveryTimeoutMillis())); this.channelPool = new GrpcChannelPool(channelFactory, scheduler); - this.endpointPool = new EndpointPool(discoveryEndpoint, balancingSettings); + this.endpointPool = new EndpointPool(balancingSettings); this.periodicDiscoveryTask = new PeriodicDiscoveryTask( scheduler, @@ -170,7 +171,10 @@ public boolean useMinDiscoveryPeriod() { @Override public void handleDiscoveryResult(DiscoveryProtos.ListEndpointsResult result) { - List removed = endpointPool.setNewState(result); + List records = result.getEndpointsList().stream() + .map(e -> new EndpointRecord(e.getAddress(), e.getPort(), e.getNodeId(), e.getLocation())) + .collect(Collectors.toList()); + List removed = endpointPool.setNewState(result.getSelfLocation(), records); channelPool.removeChannels(removed); } } diff --git a/core/src/main/java/tech/ydb/core/impl/pool/EndpointPool.java b/core/src/main/java/tech/ydb/core/impl/pool/EndpointPool.java index 47397c1ba..f4aab8342 100644 --- a/core/src/main/java/tech/ydb/core/impl/pool/EndpointPool.java +++ b/core/src/main/java/tech/ydb/core/impl/pool/EndpointPool.java @@ -3,15 +3,11 @@ import java.util.ArrayList; import java.util.Comparator; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.concurrent.ThreadLocalRandom; -import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; -import java.util.stream.Collectors; import javax.annotation.Nullable; @@ -20,7 +16,6 @@ import org.slf4j.LoggerFactory; import tech.ydb.core.grpc.BalancingSettings; -import tech.ydb.proto.discovery.DiscoveryProtos; /** * @author Nikolay Perfilov @@ -34,36 +29,36 @@ public final class EndpointPool { private final BalancingSettings balancingSettings; private final ReadWriteLock recordsLock = new ReentrantReadWriteLock(); - private final AtomicInteger pessimizationRatio = new AtomicInteger(); - private final EndpointRecord discoveryEndpoint; + private List records = new ArrayList<>(); - private Map endpointsByNodeId = new HashMap<>(); + private Map recordsByNodeId = new HashMap<>(); + private Map recordsByEndpoint = new HashMap<>(); + private boolean needToRunDiscovery = false; // Number of endpoints with best load factor (priority) private int bestEndpointsCount = -1; - public EndpointPool(EndpointRecord discoveryEndpoint, BalancingSettings balancingSettings) { + public EndpointPool(BalancingSettings balancingSettings) { logger.debug("Creating endpoint pool with balancing settings policy: {}", balancingSettings.getPolicy()); - - this.discoveryEndpoint = discoveryEndpoint; this.balancingSettings = balancingSettings; } + @Nullable public EndpointRecord getEndpoint(@Nullable Integer preferredNodeID) { recordsLock.readLock().lock(); try { if (preferredNodeID != null) { - PriorityEndpoint knownEndpoint = endpointsByNodeId.get(preferredNodeID); + PriorityEndpoint knownEndpoint = recordsByNodeId.get(preferredNodeID); if (knownEndpoint != null) { - return knownEndpoint; + return knownEndpoint.record; } } if (bestEndpointsCount > 0) { // returns value in range [0, n) int idx = ThreadLocalRandom.current().nextInt(bestEndpointsCount); - return records.get(idx); + return records.get(idx).record; } else { - return discoveryEndpoint; + return null; } } finally { recordsLock.readLock().unlock(); @@ -71,53 +66,57 @@ public EndpointRecord getEndpoint(@Nullable Integer preferredNodeID) { } // Sets new endpoints, returns removed - public List setNewState(DiscoveryProtos.ListEndpointsResult result) { - PriorityPicker priorityPicker = PriorityPicker.from(balancingSettings, - result.getSelfLocation(), - result.getEndpointsList() - .stream() - .map(e -> new EndpointRecord(e.getAddress(), e.getPort(), e.getNodeId(), e.getLocation())) - .collect(Collectors.toList()) - ); - - Set newKnownEndpoints = new HashSet<>(); - Map newKnownEndpointsByNodeId = new HashMap<>(); + public List setNewState(String selfLocation, List endpoints) { + PriorityPicker picker = PriorityPicker.from(balancingSettings, selfLocation, endpoints); + + Map newRecordsByEndpoint = new HashMap<>(); + Map newRecordsByNodeId = new HashMap<>(); List newRecords = new ArrayList<>(); + int newBestEndpointsCount = 0; + int bestPriority = Integer.MAX_VALUE; + + logger.debug("init new state with {} endpoints", endpoints.size()); + for (EndpointRecord endpoint : endpoints) { + int priority = picker.getEndpointPriority(endpoint.getLocation()); - logger.debug("init new state with {} endpoints", result.getEndpointsCount()); - for (DiscoveryProtos.EndpointInfo info : result.getEndpointsList()) { - int priority = priorityPicker.getEndpointPriority(info.getLocation()); - PriorityEndpoint entry = new PriorityEndpoint(info, priority); - String endpoint = entry.getHostAndPort(); + PriorityEndpoint entry = new PriorityEndpoint(endpoint, priority); + String hostAndPort = endpoint.getHostAndPort(); - if (!newKnownEndpoints.contains(endpoint)) { + if (!newRecordsByEndpoint.containsKey(hostAndPort)) { logger.debug("added endpoint {}", entry); - newKnownEndpoints.add(endpoint); - if (entry.getNodeId() != 0) { - newKnownEndpointsByNodeId.put(entry.getNodeId(), entry); + newRecordsByEndpoint.put(hostAndPort, entry); + if (endpoint.getNodeId() != 0) { + newRecordsByNodeId.put(endpoint.getNodeId(), entry); } newRecords.add(entry); + + if (priority < bestPriority) { + bestPriority = priority; + newBestEndpointsCount = 0; + } + if (priority == bestPriority) { + newBestEndpointsCount += 1; + } } else { - logger.warn("duplicate endpoint {}", entry.getHostAndPort()); + logger.warn("duplicate endpoint {}", endpoint.getHostAndPort()); } } newRecords.sort(PriorityEndpoint.COMPARATOR); - int newBestEndpointsCount = getBestEndpointsCount(newRecords); - List removed = new ArrayList<>(); for (PriorityEndpoint entry : records) { - if (!newKnownEndpoints.contains(entry.getHostAndPort())) { - removed.add(entry); + if (!newRecordsByEndpoint.containsKey(entry.record.getHostAndPort())) { + removed.add(entry.record); } } recordsLock.writeLock().lock(); try { records = newRecords; - endpointsByNodeId = newKnownEndpointsByNodeId; + recordsByNodeId = newRecordsByNodeId; + recordsByEndpoint = newRecordsByEndpoint; bestEndpointsCount = newBestEndpointsCount; - pessimizationRatio.set(0); + needToRunDiscovery = false; } finally { recordsLock.writeLock().unlock(); } @@ -125,12 +124,15 @@ public List setNewState(DiscoveryProtos.ListEndpointsResult resu } public void pessimizeEndpoint(EndpointRecord endpoint) { - if (!(endpoint instanceof PriorityEndpoint)) { - logger.trace("Endpoint {} is unknown", endpoint); + if (endpoint == null) { + return; + } + + PriorityEndpoint knownEndpoint = recordsByEndpoint.get(endpoint.getHostAndPort()); + if (knownEndpoint == null) { return; } - PriorityEndpoint knownEndpoint = (PriorityEndpoint) endpoint; if (knownEndpoint.isPessimized()) { logger.trace("Endpoint {} is already pessimized", endpoint); return; @@ -140,54 +142,49 @@ public void pessimizeEndpoint(EndpointRecord endpoint) { try { knownEndpoint.pessimize(); - int newRatio = (pessimizationRatio.get() * records.size() + 100) / records.size(); - pessimizationRatio.set(newRatio); - if (needToRunDiscovery()) { - logger.debug("launching discovery due to pessimization threshold is exceeded: {} is more than {}", - newRatio, DISCOVERY_PESSIMIZATION_THRESHOLD); + records.sort(PriorityEndpoint.COMPARATOR); + + long bestPriority = records.get(0).priority; + int newBestCount = 0; + int newPessimizedCount = 0; + for (PriorityEndpoint record: records) { + if (record.getPriority() == bestPriority) { + newBestCount++; + } + if (record.isPessimized()) { + newPessimizedCount++; + } } - records.sort(PriorityEndpoint.COMPARATOR); - bestEndpointsCount = getBestEndpointsCount(records); + bestEndpointsCount = newBestCount; + needToRunDiscovery = 100 * newPessimizedCount > records.size() * DISCOVERY_PESSIMIZATION_THRESHOLD; + if (needToRunDiscovery) { + logger.debug("launching discovery due to pessimization threshold is exceeded: {}/{} is more than {}", + newPessimizedCount, records.size(), DISCOVERY_PESSIMIZATION_THRESHOLD); + } - logger.info("Endpoint {} was pessimized. New pessimization ratio: {}", endpoint, newRatio); + logger.trace("Endpoint {} was pessimized. New pessimization ratio: {}/{}", + endpoint, newPessimizedCount, records.size()); } finally { recordsLock.writeLock().unlock(); } } public boolean needToRunDiscovery() { - return pessimizationRatio.get() > DISCOVERY_PESSIMIZATION_THRESHOLD; - } - - // returns amount of endpoints with best load factor (priority) - private static int getBestEndpointsCount(List records) { - if (records.isEmpty()) { - return -1; - } - - final long bestPriority = records.get(0).priority; - int pos = 1; - while (pos < records.size()) { - if (records.get(pos).priority != bestPriority) { - break; - } - pos++; - } - return pos; + return needToRunDiscovery; } @VisibleForTesting - static class PriorityEndpoint extends EndpointRecord { + static class PriorityEndpoint { static final Comparator COMPARATOR = Comparator .comparingLong(PriorityEndpoint::getPriority) - .thenComparing(PriorityEndpoint::getHost) - .thenComparing(PriorityEndpoint::getPort); + .thenComparing(e -> e.record.getHostAndPort()); + private final EndpointRecord record; private long priority; - PriorityEndpoint(DiscoveryProtos.EndpointInfo endpoint, long priority) { - super(endpoint.getAddress(), endpoint.getPort(), endpoint.getNodeId(), endpoint.getLocation()); + PriorityEndpoint(EndpointRecord record, long priority) { + this.record = record; this.priority = priority; } @@ -195,6 +192,10 @@ public long getPriority() { return this.priority; } + public EndpointRecord getEndpoint() { + return this.record; + } + public void pessimize() { this.priority = Long.MAX_VALUE; } @@ -202,19 +203,11 @@ public void pessimize() { public boolean isPessimized() { return priority == Long.MAX_VALUE; } - - @Override - public String toString() { - return "PriorityEndpoint{host=" + getHost() + - ", port=" + getPort() + - ", node=" + getNodeId() + - ", priority= " + priority + "}"; - } } @VisibleForTesting Map getEndpointsByNodeId() { - return this.endpointsByNodeId; + return this.recordsByNodeId; } @VisibleForTesting diff --git a/core/src/test/java/tech/ydb/core/impl/pool/EndpointPoolTest.java b/core/src/test/java/tech/ydb/core/impl/pool/EndpointPoolTest.java index 0a7e3ef26..1c0d1b77e 100644 --- a/core/src/test/java/tech/ydb/core/impl/pool/EndpointPoolTest.java +++ b/core/src/test/java/tech/ydb/core/impl/pool/EndpointPoolTest.java @@ -5,7 +5,6 @@ import java.util.Arrays; import java.util.List; import java.util.concurrent.ThreadLocalRandom; -import java.util.stream.Collectors; import javax.net.ServerSocketFactory; @@ -20,7 +19,6 @@ import tech.ydb.core.grpc.BalancingSettings; import tech.ydb.core.timer.TestTicker; -import tech.ydb.proto.discovery.DiscoveryProtos; import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.times; @@ -32,45 +30,46 @@ * @author Kirill Kurdyukov */ public class EndpointPoolTest { - private final EndpointRecord discovery = new EndpointRecord("discovery", 2136, -1, null); - - private final AutoCloseable mocks = MockitoAnnotations.openMocks(this); + private AutoCloseable mocks; private final MockedStatic threadLocalStaticMock = mockStatic(ThreadLocalRandom.class); + private final MockedStatic tickerStaticMock = mockStatic(Ticker.class); private final ThreadLocalRandom random = Mockito.mock(ThreadLocalRandom.class); @Before public void setUp() { + mocks = MockitoAnnotations.openMocks(this); threadLocalStaticMock.when(ThreadLocalRandom::current).thenReturn(random); } @After public void tearDown() throws Exception { + tickerStaticMock.close(); threadLocalStaticMock.close(); mocks.close(); } @Test public void uninitializedTest() { - EndpointPool pool = new EndpointPool(discovery, useAllNodes()); + EndpointPool pool = new EndpointPool(useAllNodes()); check(pool).records(0).knownNodes(0).needToReDiscovery(false).bestEndpointsCount(-1); - check(pool.getEndpoint(null)).hostname("discovery").nodeID(-1).port(2136); - check(pool.getEndpoint(0)).hostname("discovery").nodeID(-1).port(2136); - check(pool.getEndpoint(1)).hostname("discovery").nodeID(-1).port(2136); + check(pool.getEndpoint(null)).isNull(); + check(pool.getEndpoint(0)).isNull(); + check(pool.getEndpoint(1)).isNull(); - pool.setNewState(list("DC1")); + pool.setNewState("DC1", list()); - check(pool.getEndpoint(null)).hostname("discovery").nodeID(-1).port(2136); - check(pool.getEndpoint(0)).hostname("discovery").nodeID(-1).port(2136); - check(pool.getEndpoint(1)).hostname("discovery").nodeID(-1).port(2136); + check(pool.getEndpoint(null)).isNull(); + check(pool.getEndpoint(0)).isNull(); + check(pool.getEndpoint(1)).isNull(); } @Test public void useAllNodesTest() { - EndpointPool pool = new EndpointPool(discovery, useAllNodes()); + EndpointPool pool = new EndpointPool(useAllNodes()); check(pool).records(0).knownNodes(0).needToReDiscovery(false).bestEndpointsCount(-1); - pool.setNewState(list("DC1", + pool.setNewState("DC1", list( endpoint(1, "n1.ydb.tech", 12345, "DC1"), endpoint(2, "n2.ydb.tech", 12345, "DC2"), endpoint(3, "n3.ydb.tech", 12345, "DC3") @@ -93,10 +92,10 @@ public void useAllNodesTest() { @Test public void localDcTest() { - EndpointPool pool = new EndpointPool(discovery, preferredNode(null)); + EndpointPool pool = new EndpointPool(preferredNode(null)); check(pool).records(0).knownNodes(0).needToReDiscovery(false).bestEndpointsCount(-1); - pool.setNewState(list("DC2", + pool.setNewState("DC2", list( endpoint(1, "n1.ydb.tech", 12345, "DC1"), endpoint(2, "n2.ydb.tech", 12345, "DC2"), endpoint(3, "n3.ydb.tech", 12345, "DC3") @@ -118,10 +117,10 @@ public void localDcTest() { @Test public void preferredDcTest() { - EndpointPool pool = new EndpointPool(discovery, preferredNode("DC1")); + EndpointPool pool = new EndpointPool(preferredNode("DC1")); check(pool).records(0).knownNodes(0).needToReDiscovery(false).bestEndpointsCount(-1); - pool.setNewState(list("DC3", + pool.setNewState("DC3", list( endpoint(1, "n1.ydb.tech", 12345, "DC1"), endpoint(2, "n2.ydb.tech", 12345, "DC2"), endpoint(3, "n3.ydb.tech", 12345, "DC3") @@ -143,10 +142,10 @@ public void preferredDcTest() { @Test public void preferredEndpointsTest() { - EndpointPool pool = new EndpointPool(discovery, useAllNodes()); + EndpointPool pool = new EndpointPool(useAllNodes()); check(pool).records(0).knownNodes(0).needToReDiscovery(false).bestEndpointsCount(-1); - pool.setNewState(list("DC3", + pool.setNewState("DC3", list( endpoint(1, "n1.ydb.tech", 12341, "DC1"), endpoint(2, "n2.ydb.tech", 12342, "DC2"), endpoint(3, "n3.ydb.tech", 12343, "DC3") @@ -173,10 +172,10 @@ public void preferredEndpointsTest() { @Test public void nodePessimizationTest() { - EndpointPool pool = new EndpointPool(discovery, useAllNodes()); + EndpointPool pool = new EndpointPool( useAllNodes()); check(pool).records(0).knownNodes(0).needToReDiscovery(false).bestEndpointsCount(-1); - pool.setNewState(list("DC3", + pool.setNewState("DC3", list( endpoint(1, "n1.ydb.tech", 12341, "DC1"), endpoint(2, "n2.ydb.tech", 12342, "DC2"), endpoint(3, "n3.ydb.tech", 12343, "DC3"), @@ -241,10 +240,10 @@ public void nodePessimizationTest() { @Test public void nodePessimizationFallbackTest() { - EndpointPool pool = new EndpointPool(discovery, preferredNode("DC1")); + EndpointPool pool = new EndpointPool(preferredNode("DC1")); check(pool).records(0).knownNodes(0).needToReDiscovery(false); - pool.setNewState(list("DC3", + pool.setNewState("DC3", list( endpoint(1, "n1.ydb.tech", 12341, "DC1"), endpoint(2, "n2.ydb.tech", 12342, "DC1"), endpoint(3, "n3.ydb.tech", 12343, "DC2"), @@ -289,7 +288,7 @@ public void nodePessimizationFallbackTest() { verify(random, times(4)).nextInt(4); // setNewState reset all - pool.setNewState(list("DC3", + pool.setNewState("DC3", list( endpoint(1, "n1.ydb.tech", 12341, "DC1"), endpoint(2, "n2.ydb.tech", 12342, "DC1"), endpoint(3, "n3.ydb.tech", 12343, "DC2"), @@ -300,10 +299,10 @@ public void nodePessimizationFallbackTest() { @Test public void duplicateEndpointsTest() { - EndpointPool pool = new EndpointPool(discovery, useAllNodes()); + EndpointPool pool = new EndpointPool(useAllNodes()); check(pool).records(0).knownNodes(0).needToReDiscovery(false); - pool.setNewState(list("DC", + pool.setNewState("DC", list( endpoint(1, "n1.ydb.tech", 12341, "DC"), endpoint(2, "n2.ydb.tech", 12342, "DC"), endpoint(3, "n3.ydb.tech", 12343, "DC"), @@ -335,10 +334,10 @@ public void duplicateEndpointsTest() { @Test public void duplicateNodesTest() { - EndpointPool pool = new EndpointPool(discovery, useAllNodes()); + EndpointPool pool = new EndpointPool(useAllNodes()); check(pool).records(0).knownNodes(0).needToReDiscovery(false); - pool.setNewState(list("DC", + pool.setNewState("DC", list( endpoint(1, "n1.ydb.tech", 12341, "DC"), endpoint(2, "n2.ydb.tech", 12342, "DC"), endpoint(2, "n3.ydb.tech", 12343, "DC") @@ -363,10 +362,10 @@ public void duplicateNodesTest() { @Test public void removeEndpointsTest() { - EndpointPool pool = new EndpointPool(discovery, useAllNodes()); + EndpointPool pool = new EndpointPool(useAllNodes()); check(pool).records(0).knownNodes(0).needToReDiscovery(false); - pool.setNewState(list("DC", + pool.setNewState("DC", list( endpoint(1, "n1.ydb.tech", 12341, "DC"), endpoint(2, "n2.ydb.tech", 12342, "DC"), endpoint(3, "n3.ydb.tech", 12343, "DC") @@ -389,7 +388,7 @@ public void removeEndpointsTest() { verify(random, times(3)).nextInt(3); - pool.setNewState(list("DC", + pool.setNewState("DC", list( endpoint(2, "n2.ydb.tech", 12342, "DC"), endpoint(4, "n4.ydb.tech", 12344, "DC"), endpoint(5, "n5.ydb.tech", 12345, "DC"), @@ -419,62 +418,50 @@ public void removeEndpointsTest() { @Test - public void detectLocalDCTest() { - try (MockedStatic systemMocked = mockStatic(Ticker.class)) { - TestTicker testTicker = new TestTicker( - 1, 4, - 5, 26, - 83, 125 - ); - - systemMocked.when(Ticker::systemTicker).thenReturn(testTicker); - - List servers = Arrays.stream(new int[]{8081, 8082, 8083}) - .mapToObj( - port -> { - try { - return ServerSocketFactory.getDefault() - .createServerSocket(port); - } catch (IOException e) { - throw new RuntimeException(e); - } - } - ) - .collect(Collectors.toList()); - - EndpointPool pool = new EndpointPool(discovery, detectLocalDC()); + public void detectLocalDCTest() throws IOException { + final TestTicker testTicker = new TestTicker( + 1, 4, + 5, 26, + 83, 125 + ); + + tickerStaticMock.when(Ticker::systemTicker).thenReturn(testTicker); + + try ( + ServerSocket s1 = ServerSocketFactory.getDefault().createServerSocket(0); + ServerSocket s2 = ServerSocketFactory.getDefault().createServerSocket(0); + ServerSocket s3 = ServerSocketFactory.getDefault().createServerSocket(0); + ) { + + EndpointPool pool = new EndpointPool(detectLocalDC()); check(pool).records(0).knownNodes(0).needToReDiscovery(false); - pool.setNewState(list("DC", - endpoint(1, "localhost", 8081, "DC1"), - endpoint(2, "localhost", 8082, "DC2"), - endpoint(3, "localhost", 8083, "DC3") + int p1 = s1.getLocalPort(); + int p2 = s2.getLocalPort(); + int p3 = s3.getLocalPort(); + + pool.setNewState("DC", list( + endpoint(1, "127.0.0.1", p1, "DC1"), + endpoint(2, "127.0.0.2", p2, "DC2"), + endpoint(3, "127.0.0.3", p3, "DC3") )); check(pool).records(3).knownNodes(3).needToReDiscovery(false).bestEndpointsCount(1); - check(pool.getEndpoint(null)).hostname("localhost").nodeID(2).port(8082); // detect local dc - check(pool.getEndpoint(0)).hostname("localhost").nodeID(2).port(8082); // random from local dc - check(pool.getEndpoint(1)).hostname("localhost").nodeID(1).port(8081); - check(pool.getEndpoint(2)).hostname("localhost").nodeID(2).port(8082); // local dc - check(pool.getEndpoint(3)).hostname("localhost").nodeID(3).port(8083); - check(pool.getEndpoint(4)).hostname("localhost").nodeID(2).port(8082); // random from local dc + check(pool.getEndpoint(null)).hostname("127.0.0.2").nodeID(2).port(p2); // detect local dc + check(pool.getEndpoint(0)).hostname("127.0.0.2").nodeID(2).port(p2); // random from local dc + check(pool.getEndpoint(1)).hostname("127.0.0.1").nodeID(1).port(p1); + check(pool.getEndpoint(2)).hostname("127.0.0.2").nodeID(2).port(p2); // local dc + check(pool.getEndpoint(3)).hostname("127.0.0.3").nodeID(3).port(p3); + check(pool.getEndpoint(4)).hostname("127.0.0.2").nodeID(2).port(p2); // random from local dc pool.pessimizeEndpoint(pool.getEndpoint(2)); - check(pool.getEndpoint(null)).hostname("localhost").nodeID(1).port(8081); // new local dc - check(pool.getEndpoint(0)).hostname("localhost").nodeID(1).port(8081); // random from local dc - check(pool.getEndpoint(1)).hostname("localhost").nodeID(1).port(8081); - check(pool.getEndpoint(2)).hostname("localhost").nodeID(2).port(8082); // local dc - check(pool.getEndpoint(3)).hostname("localhost").nodeID(3).port(8083); - check(pool.getEndpoint(4)).hostname("localhost").nodeID(1).port(8081); // random from local dc - - servers.forEach(serverSocket -> { - try { - serverSocket.close(); - } catch (IOException e) { - throw new RuntimeException(e); - } - }); + check(pool.getEndpoint(null)).hostname("127.0.0.1").nodeID(1).port(p1); // new local dc + check(pool.getEndpoint(0)).hostname("127.0.0.1").nodeID(1).port(p1); // random from local dc + check(pool.getEndpoint(1)).hostname("127.0.0.1").nodeID(1).port(p1); + check(pool.getEndpoint(2)).hostname("127.0.0.2").nodeID(2).port(p2); // local dc + check(pool.getEndpoint(3)).hostname("127.0.0.3").nodeID(3).port(p3); + check(pool.getEndpoint(4)).hostname("127.0.0.1").nodeID(1).port(p1); // random from local dc } } @@ -491,7 +478,7 @@ public PoolChecker records(int count) { } public EndpointRecordChecker record(int idx) { - return new EndpointRecordChecker(pool.getRecords().get(idx)); + return new EndpointRecordChecker(pool.getRecords().get(idx).getEndpoint()); } public PoolChecker knownNodes(int size) { @@ -517,6 +504,11 @@ public EndpointRecordChecker(EndpointRecord record) { this.record = record; } + public EndpointRecordChecker isNull() { + Assert.assertNull("Check endpoint is null", record); + return this; + } + public EndpointRecordChecker hostname(String hostname) { Assert.assertNotNull("Check endpoint is ot null", record); Assert.assertEquals("Check endpoint host", hostname, record.getHost()); @@ -556,28 +548,11 @@ private static BalancingSettings detectLocalDC() { return BalancingSettings.detectLocalDs(); } - private static DiscoveryProtos.ListEndpointsResult list(String selfLocation, - DiscoveryProtos.EndpointInfo... endpoints) { - return DiscoveryProtos.ListEndpointsResult.newBuilder() - .setSelfLocation(selfLocation) - .addAllEndpoints(Arrays.asList(endpoints)) - .build(); - } - - private static DiscoveryProtos.EndpointInfo endpoint(int nodeID, String hostname, int port, String location) { - return DiscoveryProtos.EndpointInfo.newBuilder() - .setAddress(hostname) - .setPort(port) - .setNodeId(nodeID) - .setLocation(location) - .build(); + private static List list(EndpointRecord... records) { + return Arrays.asList(records); } - private static DiscoveryProtos.EndpointInfo endpoint(int nodeID, String hostname, int port) { - return DiscoveryProtos.EndpointInfo.newBuilder() - .setAddress(hostname) - .setPort(port) - .setNodeId(nodeID) - .build(); + private static EndpointRecord endpoint(int nodeID, String hostname, int port, String location) { + return new EndpointRecord(hostname, port, nodeID, location); } } From e5b94fea16f606ff90f79f026e26004d2d1f5046 Mon Sep 17 00:00:00 2001 From: Alexandr Gorshenin Date: Fri, 19 Apr 2024 11:20:27 +0100 Subject: [PATCH 05/15] Small style fixes --- .../ydb/core/impl/auth/AuthCallOptions.java | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/tech/ydb/core/impl/auth/AuthCallOptions.java b/core/src/main/java/tech/ydb/core/impl/auth/AuthCallOptions.java index d94df4751..5ae7231b9 100644 --- a/core/src/main/java/tech/ydb/core/impl/auth/AuthCallOptions.java +++ b/core/src/main/java/tech/ydb/core/impl/auth/AuthCallOptions.java @@ -8,6 +8,7 @@ import io.grpc.CallOptions; import tech.ydb.auth.AuthIdentity; +import tech.ydb.auth.AuthRpcProvider; import tech.ydb.core.grpc.GrpcCompression; import tech.ydb.core.grpc.GrpcTransportBuilder; import tech.ydb.core.impl.pool.EndpointRecord; @@ -27,17 +28,19 @@ public AuthCallOptions() { this.callOptions = CallOptions.DEFAULT; this.readTimeoutMillis = 0; } + public AuthCallOptions( ScheduledExecutorService scheduler, String database, List endpoints, ManagedChannelFactory channelFactory, - GrpcTransportBuilder transportBuilder) { + GrpcTransportBuilder builder) { CallOptions options = CallOptions.DEFAULT; - if (transportBuilder.getAuthProvider() != null) { + AuthRpcProvider authProvider = builder.getAuthProvider(); + if (authProvider != null) { GrpcAuthRpc rpc = new GrpcAuthRpc(endpoints, scheduler, database, channelFactory); - authIdentity = transportBuilder.getAuthProvider().createAuthIdentity(rpc); + authIdentity = builder.getAuthProvider().createAuthIdentity(rpc); } else { authIdentity = null; } @@ -46,17 +49,16 @@ public AuthCallOptions( options = options.withCallCredentials(new YdbCallCredentials(authIdentity)); } - if (transportBuilder.getCallExecutor() != null - && transportBuilder.getCallExecutor() != MoreExecutors.directExecutor()) { - options = options.withExecutor(transportBuilder.getCallExecutor()); + if (builder.getCallExecutor() != null && builder.getCallExecutor() != MoreExecutors.directExecutor()) { + options = options.withExecutor(builder.getCallExecutor()); } - if (transportBuilder.getGrpcCompression() != GrpcCompression.NO_COMPRESSION) { - options = options.withCompression(transportBuilder.getGrpcCompression().compressor()); + if (builder.getGrpcCompression() != GrpcCompression.NO_COMPRESSION) { + options = options.withCompression(builder.getGrpcCompression().compressor()); } this.callOptions = options; - this.readTimeoutMillis = transportBuilder.getReadTimeoutMillis(); + this.readTimeoutMillis = builder.getReadTimeoutMillis(); } @Override From 81b067de0c3f71dcf6c7810dd326523a696188f6 Mon Sep 17 00:00:00 2001 From: Alexandr Gorshenin Date: Fri, 19 Apr 2024 10:05:29 +0100 Subject: [PATCH 06/15] New implemenation of discovery --- .../tech/ydb/core/grpc/GrpcTransport.java | 1 - .../ydb/core/grpc/GrpcTransportBuilder.java | 58 ++++- .../java/tech/ydb/core/impl/YdbDiscovery.java | 199 +++++++++++++++ .../tech/ydb/core/impl/YdbTransportImpl.java | 84 ++++--- .../core/impl/discovery/GrpcDiscoveryRpc.java | 78 ------ .../impl/discovery/PeriodicDiscoveryTask.java | 237 ------------------ 6 files changed, 303 insertions(+), 354 deletions(-) create mode 100644 core/src/main/java/tech/ydb/core/impl/YdbDiscovery.java delete mode 100644 core/src/main/java/tech/ydb/core/impl/discovery/GrpcDiscoveryRpc.java delete mode 100644 core/src/main/java/tech/ydb/core/impl/discovery/PeriodicDiscoveryTask.java diff --git a/core/src/main/java/tech/ydb/core/grpc/GrpcTransport.java b/core/src/main/java/tech/ydb/core/grpc/GrpcTransport.java index 463b1337d..9817396ff 100644 --- a/core/src/main/java/tech/ydb/core/grpc/GrpcTransport.java +++ b/core/src/main/java/tech/ydb/core/grpc/GrpcTransport.java @@ -21,7 +21,6 @@ * @author Nikolay Perfilov */ public interface GrpcTransport extends AutoCloseable { - CompletableFuture> unaryCall( MethodDescriptor method, GrpcRequestSettings settings, diff --git a/core/src/main/java/tech/ydb/core/grpc/GrpcTransportBuilder.java b/core/src/main/java/tech/ydb/core/grpc/GrpcTransportBuilder.java index 9c247b4ff..a47f5b3f5 100644 --- a/core/src/main/java/tech/ydb/core/grpc/GrpcTransportBuilder.java +++ b/core/src/main/java/tech/ydb/core/grpc/GrpcTransportBuilder.java @@ -32,6 +32,37 @@ * @author Aleksandr Gorshenin */ public class GrpcTransportBuilder { + /** + * The initialization mode defines the behavior of {@link tech.ydb.core.grpc.GrpcTransportBuilder#build() } + * method. + */ + public enum InitMode { + /** + * In synchronous mode, transport creation will wait for successful discovery of current the database nodes. Any + * of discovery execution like an authentication error or a network issue will be thrown as RuntimeException. + * It allows to catch configuration problems and stops the transport creating. + */ + SYNC, + + /** + * In asynchronous mode, transport creation will not block by the discovery waiting and will not throw any + * exceptions in the case of configuration problems. But any request with the transport will wait for the + * discovery and may throw an exception if it will not be completed. This mode allows to application not to be + * blocked during the transport initialization but guarantees that requests will be created only after + * complete initialization + */ + ASYNC, + + /** + * In fallback asynchronous mode, neither initialization the transport nor making the requests will block by + * the discovery waiting. In this case if the discovery is not completed - all requests will be sent to + * the discovery endpoint. Any discovery problems will be ignored. This mode allows to start working with + * the database without waiting for discovery to complete, but after its completion, existing connections (like + * grpc streams) can be interrupted to switch to new endpoint. + */ + ASYNC_FALLBACK + } + private final String endpoint; private final HostAndPort host; private final String database; @@ -49,6 +80,7 @@ public class GrpcTransportBuilder { private long discoveryTimeoutMillis = 60_000; private boolean useDefaultGrpcResolver = false; private GrpcCompression compression = GrpcCompression.NO_COMPRESSION; + private InitMode initMode = InitMode.SYNC; /** * can cause leaks https://github.com/grpc/grpc-java/issues/9340 @@ -200,6 +232,18 @@ public GrpcTransportBuilder withBalancingSettings(BalancingSettings balancingSet return this; } + /** + * Set GrpcTransport's init mode. + * See {@link tech.ydb.core.grpc.GrpcTransport.InitMode } for details + * + * @param initMode mode of tranport initialization + * @return GrpcTransportBuilder with the given initMode + */ + public GrpcTransportBuilder withInitMode(InitMode initMode) { + this.initMode = initMode; + return this; + } + public GrpcTransportBuilder withAuthProvider(AuthRpcProvider authProvider) { this.authProvider = Objects.requireNonNull(authProvider); return this; @@ -209,7 +253,7 @@ public GrpcTransportBuilder withAuthProvider(AuthRpcProvider currentSchedule = null; + private RuntimeException lastException = null; + + public YdbDiscovery(Clock clock, ScheduledExecutorService scheduler, String database, Duration timeout) { + this.clock = clock; + this.scheduler = scheduler; + this.lastUpdateTime = clock.instant(); + this.discoveryDatabase = database; + this.discoveryTimeout = timeout; + } + + public void start() { + logger.debug("start periodic discovery task"); + currentSchedule = scheduler.submit(() -> { + logger.info("Waiting for init discovery..."); + runDiscovery(); + logger.info("Discovery is finished"); + }); + } + + public void stop() { + logger.debug("stopping PeriodicDiscoveryTask"); + isStopped = true; + if (currentSchedule != null) { + currentSchedule.cancel(false); + currentSchedule = null; + } + } + + public void waitReady() { + synchronized (readyObj) { + try { + long waitMillis = 2 * discoveryTimeout.toMillis(); + readyObj.wait(waitMillis); + } catch (InterruptedException ex) { + Thread.currentThread().interrupt(); + handleThrowable(ex); + } + } + + if (lastException != null) { + throw lastException; + } + } + + private void scheduleNextTick() { + if (!isStopped) { + logger.trace("schedule next discovery in {} seconds", DISCOVERY_PERIOD_MIN_SECONDS); + currentSchedule = scheduler.schedule(this::tick, DISCOVERY_PERIOD_MIN_SECONDS, TimeUnit.SECONDS); + } + } + + private void tick() { + if (isStopped) { + return; + } + + if (forceDiscovery()) { + logger.debug("launching discovery by endpoint pessimization"); + runDiscovery(); + } else { + if (Instant.now().isAfter(lastUpdateTime.plusSeconds(DISCOVERY_PERIOD_NORMAL_SECONDS))) { + logger.debug("launching discovery in normal mode"); + runDiscovery(); + } else { + logger.trace("no need to run discovery yet"); + scheduleNextTick(); + } + } + } + + protected abstract GrpcTransport createDiscoveryTransport(); + protected abstract boolean forceDiscovery(); + protected abstract void handleEndpoints(List endpoints, String selfLocation); + + private void runDiscovery() { + if (isStopped) { + return; + } + + lastUpdateTime = clock.instant(); + + final GrpcTransport transport = createDiscoveryTransport(); + logger.debug("execute list endpoints on {} with timeout {}", transport, discoveryTimeout); + DiscoveryProtos.ListEndpointsRequest request = DiscoveryProtos.ListEndpointsRequest.newBuilder() + .setDatabase(discoveryDatabase) + .build(); + + GrpcRequestSettings grpcSettings = GrpcRequestSettings.newBuilder() + .withDeadline(discoveryTimeout) + .build(); + + transport.unaryCall(DiscoveryServiceGrpc.getListEndpointsMethod(), grpcSettings, request) + .whenComplete((res, ex) -> transport.close()) // close transport for any result + .thenApply(OperationBinder.bindSync( + DiscoveryProtos.ListEndpointsResponse::getOperation, + DiscoveryProtos.ListEndpointsResult.class + )) + .whenComplete(this::handleDiscoveryResult); + } + + private void handleThrowable(Throwable th) { + synchronized (readyObj) { + if (th instanceof RuntimeException) { + lastException = (RuntimeException) th; + } else { + lastException = new RuntimeException("Discovery failed", th); + } + readyObj.notifyAll(); + } + } + + private void handleDiscoveryResult(Result response, Throwable th) { + if (th != null) { + Throwable cause = Async.unwrapCompletionException(th); + logger.warn("couldn't perform discovery with exception", cause); + handleThrowable(cause); + scheduleNextTick(); + return; + } + + try { + DiscoveryProtos.ListEndpointsResult result = response.getValue(); + if (result.getEndpointsList().isEmpty()) { + logger.error("discovery return empty list of endpoints"); + handleThrowable(new UnexpectedResultException("Discovery failed", EMPTY_DISCOVERY)); + scheduleNextTick(); + return; + } + + List records = result.getEndpointsList().stream() + .map(e -> new EndpointRecord(e.getAddress(), e.getPort(), e.getNodeId(), e.getLocation())) + .collect(Collectors.toList()); + + logger.debug("successfully received ListEndpoints result with {} endpoints", records.size()); + synchronized (readyObj) { + lastException = null; + handleEndpoints(records, result.getSelfLocation()); + readyObj.notifyAll(); + } + + scheduleNextTick(); + } catch (UnexpectedResultException ex) { + logger.error("discovery fail {}", response); + handleThrowable(th); + scheduleNextTick(); + } + } +} diff --git a/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java b/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java index fc793ef0f..28ea51451 100644 --- a/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java +++ b/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java @@ -2,30 +2,27 @@ import java.net.URI; import java.net.URISyntaxException; +import java.time.Clock; import java.time.Duration; import java.util.Collections; import java.util.List; import java.util.concurrent.ScheduledExecutorService; -import java.util.stream.Collectors; import com.google.common.base.Strings; import com.google.common.net.HostAndPort; -import io.grpc.Status; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import tech.ydb.core.grpc.BalancingSettings; import tech.ydb.core.grpc.GrpcRequestSettings; +import tech.ydb.core.grpc.GrpcTransport; import tech.ydb.core.grpc.GrpcTransportBuilder; import tech.ydb.core.impl.auth.AuthCallOptions; -import tech.ydb.core.impl.discovery.GrpcDiscoveryRpc; -import tech.ydb.core.impl.discovery.PeriodicDiscoveryTask; import tech.ydb.core.impl.pool.EndpointPool; import tech.ydb.core.impl.pool.EndpointRecord; import tech.ydb.core.impl.pool.GrpcChannel; import tech.ydb.core.impl.pool.GrpcChannelPool; import tech.ydb.core.impl.pool.ManagedChannelFactory; -import tech.ydb.proto.discovery.DiscoveryProtos; /** * @author Nikolay Perfilov @@ -35,12 +32,12 @@ public class YdbTransportImpl extends BaseGrpcTransport { private static final Logger logger = LoggerFactory.getLogger(YdbTransportImpl.class); - private final AuthCallOptions callOptions; private final String database; + private final ScheduledExecutorService scheduler; + private final AuthCallOptions callOptions; private final EndpointPool endpointPool; private final GrpcChannelPool channelPool; - private final PeriodicDiscoveryTask periodicDiscoveryTask; - private final ScheduledExecutorService scheduler; + private final YdbDiscoveryImpl discovery; public YdbTransportImpl(GrpcTransportBuilder builder) { this.database = Strings.nullToEmpty(builder.getDatabase()); @@ -48,6 +45,7 @@ public YdbTransportImpl(GrpcTransportBuilder builder) { ManagedChannelFactory channelFactory = builder.getManagedChannelFactory(); BalancingSettings balancingSettings = getBalancingSettings(builder); EndpointRecord discoveryEndpoint = getDiscoveryEndpoint(builder); + Duration discoveryTimeout = Duration.ofMillis(builder.getDiscoveryTimeoutMillis()); logger.info("Create YDB transport with endpoint {} and {}", discoveryEndpoint, balancingSettings); @@ -59,34 +57,37 @@ public YdbTransportImpl(GrpcTransportBuilder builder) { builder ); - GrpcDiscoveryRpc discoveryRpc = new GrpcDiscoveryRpc(this, - discoveryEndpoint, - channelFactory, - callOptions, - Duration.ofMillis(builder.getDiscoveryTimeoutMillis())); - this.channelPool = new GrpcChannelPool(channelFactory, scheduler); this.endpointPool = new EndpointPool(balancingSettings); - - this.periodicDiscoveryTask = new PeriodicDiscoveryTask( - scheduler, - discoveryRpc, - new YdbDiscoveryHandler(), - builder.getConnectTimeoutMillis() + builder.getDiscoveryTimeoutMillis() - ); + this.discovery = new YdbDiscoveryImpl(channelFactory, discoveryEndpoint, discoveryTimeout); } - public void init() { - periodicDiscoveryTask.start(); + public void start(GrpcTransportBuilder.InitMode mode) { + if (mode == GrpcTransportBuilder.InitMode.ASYNC_FALLBACK) { + endpointPool.setNewState(null, Collections.singletonList(discovery.discoveryEndpoint)); + } + + discovery.start(); + + if (mode == GrpcTransportBuilder.InitMode.SYNC) { + discovery.waitReady(); + } } - public void initAsync(Runnable readyWatcher) { - periodicDiscoveryTask.startAsync(readyWatcher); + @Deprecated + public void startAsync(Runnable readyWatcher) { + discovery.start(); + if (readyWatcher != null) { + scheduler.execute(() -> { + discovery.waitReady(); + readyWatcher.run(); + }); + } } @Override protected void shutdown() { - periodicDiscoveryTask.stop(); + discovery.stop(); channelPool.shutdown(); callOptions.close(); @@ -152,30 +153,45 @@ public AuthCallOptions getAuthCallOptions() { @Override protected GrpcChannel getChannel(GrpcRequestSettings settings) { EndpointRecord endpoint = endpointPool.getEndpoint(settings.getPreferredNodeID()); + while (endpoint == null) { + discovery.waitReady(); + endpoint = endpointPool.getEndpoint(settings.getPreferredNodeID()); + } return channelPool.getChannel(endpoint); } @Override protected void updateChannelStatus(GrpcChannel channel, io.grpc.Status status) { // Usally CANCELLED is received when ClientCall is canceled on client side - if (!status.isOk() && status.getCode() != Status.Code.CANCELLED) { + if (!status.isOk() && status.getCode() != io.grpc.Status.Code.CANCELLED) { endpointPool.pessimizeEndpoint(channel.getEndpoint()); } } - private class YdbDiscoveryHandler implements PeriodicDiscoveryTask.DiscoveryHandler { + private class YdbDiscoveryImpl extends YdbDiscovery { + private final ManagedChannelFactory channelFactory; + private final EndpointRecord discoveryEndpoint; + + YdbDiscoveryImpl(ManagedChannelFactory channelFactory, EndpointRecord endpoint, Duration timeout) { + super(Clock.systemUTC(), scheduler, database, timeout); + this.channelFactory = channelFactory; + this.discoveryEndpoint = endpoint; + } + @Override - public boolean useMinDiscoveryPeriod() { + protected boolean forceDiscovery() { return endpointPool.needToRunDiscovery(); } @Override - public void handleDiscoveryResult(DiscoveryProtos.ListEndpointsResult result) { - List records = result.getEndpointsList().stream() - .map(e -> new EndpointRecord(e.getAddress(), e.getPort(), e.getNodeId(), e.getLocation())) - .collect(Collectors.toList()); - List removed = endpointPool.setNewState(result.getSelfLocation(), records); + protected void handleEndpoints(List endpoints, String selfLocation) { + List removed = endpointPool.setNewState(selfLocation, endpoints); channelPool.removeChannels(removed); } + + @Override + protected GrpcTransport createDiscoveryTransport() { + return new FixedCallOptionsTransport(scheduler, callOptions, database, discoveryEndpoint, channelFactory); + } } } diff --git a/core/src/main/java/tech/ydb/core/impl/discovery/GrpcDiscoveryRpc.java b/core/src/main/java/tech/ydb/core/impl/discovery/GrpcDiscoveryRpc.java deleted file mode 100644 index 0bbb801b4..000000000 --- a/core/src/main/java/tech/ydb/core/impl/discovery/GrpcDiscoveryRpc.java +++ /dev/null @@ -1,78 +0,0 @@ -package tech.ydb.core.impl.discovery; - - -import java.time.Duration; -import java.util.concurrent.CompletableFuture; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import tech.ydb.core.Result; -import tech.ydb.core.grpc.GrpcRequestSettings; -import tech.ydb.core.grpc.GrpcTransport; -import tech.ydb.core.impl.BaseGrpcTransport; -import tech.ydb.core.impl.FixedCallOptionsTransport; -import tech.ydb.core.impl.auth.AuthCallOptions; -import tech.ydb.core.impl.pool.EndpointRecord; -import tech.ydb.core.impl.pool.ManagedChannelFactory; -import tech.ydb.core.operation.OperationBinder; -import tech.ydb.proto.discovery.DiscoveryProtos; -import tech.ydb.proto.discovery.v1.DiscoveryServiceGrpc; - - -/** - * @author Vladimir Gordiychuk - * @author Alexandr Gorshenin - */ -public class GrpcDiscoveryRpc { - private static final Logger logger = LoggerFactory.getLogger(GrpcDiscoveryRpc.class); - - private final BaseGrpcTransport parent; - private final EndpointRecord endpoint; - private final ManagedChannelFactory channelFactory; - private final AuthCallOptions callOptions; - private final Duration discoveryTimeout; - - public GrpcDiscoveryRpc( - BaseGrpcTransport parent, - EndpointRecord endpoint, - ManagedChannelFactory channelFactory, - AuthCallOptions callOptions, - Duration discoveryTimeout) { - this.parent = parent; - this.endpoint = endpoint; - this.channelFactory = channelFactory; - this.callOptions = callOptions; - this.discoveryTimeout = discoveryTimeout; - } - - public CompletableFuture> listEndpoints() { - GrpcTransport transport = createTransport(); - - logger.debug("list endpoints from {} with timeout {}", endpoint.getHostAndPort(), discoveryTimeout); - DiscoveryProtos.ListEndpointsRequest request = DiscoveryProtos.ListEndpointsRequest.newBuilder() - .setDatabase(parent.getDatabase()) - .build(); - - GrpcRequestSettings grpcSettings = GrpcRequestSettings.newBuilder() - .withDeadline(discoveryTimeout) - .build(); - - return transport.unaryCall(DiscoveryServiceGrpc.getListEndpointsMethod(), grpcSettings, request) - .whenComplete((res, ex) -> transport.close()) - .thenApply(OperationBinder.bindSync( - DiscoveryProtos.ListEndpointsResponse::getOperation, - DiscoveryProtos.ListEndpointsResult.class - )); - } - - private GrpcTransport createTransport() { - return new FixedCallOptionsTransport( - parent.getScheduler(), - callOptions, - parent.getDatabase(), - endpoint, - channelFactory - ); - } -} diff --git a/core/src/main/java/tech/ydb/core/impl/discovery/PeriodicDiscoveryTask.java b/core/src/main/java/tech/ydb/core/impl/discovery/PeriodicDiscoveryTask.java deleted file mode 100644 index 0256edeca..000000000 --- a/core/src/main/java/tech/ydb/core/impl/discovery/PeriodicDiscoveryTask.java +++ /dev/null @@ -1,237 +0,0 @@ -package tech.ydb.core.impl.discovery; - -import java.time.Instant; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ScheduledFuture; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicBoolean; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import tech.ydb.core.Issue; -import tech.ydb.core.Result; -import tech.ydb.core.Status; -import tech.ydb.core.StatusCode; -import tech.ydb.core.UnexpectedResultException; -import tech.ydb.core.utils.Async; -import tech.ydb.proto.discovery.DiscoveryProtos; - -/** - * @author Nikolay Perfilov - * @author Aleksandr Gorshenin - */ -public class PeriodicDiscoveryTask implements Runnable { - private static final Status EMPTY_DISCOVERY = Status.of(StatusCode.CLIENT_DISCOVERY_FAILED) - .withIssues(Issue.of("Discovery return empty list of endpoints", Issue.Severity.ERROR)); - - public interface DiscoveryHandler { - boolean useMinDiscoveryPeriod(); - void handleDiscoveryResult(DiscoveryProtos.ListEndpointsResult result); - } - - // Interval between discovery requests when everything is ok - private static final long DISCOVERY_PERIOD_NORMAL_SECONDS = 60; - // Interval between discovery requests when pessimization threshold is exceeded - private static final long DISCOVERY_PERIOD_MIN_SECONDS = 5; - - private static final Logger logger = LoggerFactory.getLogger(PeriodicDiscoveryTask.class); - - private final ScheduledExecutorService scheduler; - private final GrpcDiscoveryRpc discoveryRpc; - private final DiscoveryHandler discoveryHandler; - private final long waitingTimeoutMillis; - - private final AtomicBoolean updateInProgress = new AtomicBoolean(); - private final State state = new State(); - private volatile ScheduledFuture currentSchedule; - - public PeriodicDiscoveryTask( - ScheduledExecutorService scheduler, - GrpcDiscoveryRpc rpc, - DiscoveryHandler handler, - long waitingTimeoutMillis) { - this.scheduler = scheduler; - this.discoveryRpc = rpc; - this.discoveryHandler = handler; - this.waitingTimeoutMillis = waitingTimeoutMillis; - } - - public void stop() { - logger.debug("stopping PeriodicDiscoveryTask"); - state.stopped = true; - if (currentSchedule != null) { - currentSchedule.cancel(false); - currentSchedule = null; - } - } - - public void start() { - logger.info("Waiting for init discovery..."); - runDiscovery(); - state.waitReady(waitingTimeoutMillis); - logger.info("Discovery is finished"); - } - - public void startAsync(Runnable readyWatcher) { - scheduler.execute(() -> { - logger.info("Waiting for init discovery..."); - runDiscovery(); - state.waitReady(waitingTimeoutMillis); - logger.info("Discovery is finished"); - if (readyWatcher != null) { - readyWatcher.run(); - } - }); - } - - @Override - public void run() { - if (state.stopped) { - return; - } - - if (discoveryHandler.useMinDiscoveryPeriod()) { - runDiscovery(); - } else { - if (Instant.now().isAfter(state.lastUpdateTime.plusSeconds(DISCOVERY_PERIOD_NORMAL_SECONDS))) { - logger.debug("launching discovery in normal mode"); - runDiscovery(); - } else { - logger.trace("no need to run discovery yet"); - scheduleNextDiscovery(); - } - } - } - - private void scheduleNextDiscovery() { - logger.trace("schedule next discovery in {} seconds", DISCOVERY_PERIOD_MIN_SECONDS); - currentSchedule = scheduler.schedule(this, DISCOVERY_PERIOD_MIN_SECONDS, TimeUnit.SECONDS); - } - - private void handleDiscoveryResponse(Result response) { - try { - DiscoveryProtos.ListEndpointsResult result = response.getValue(); - if (result.getEndpointsList().isEmpty()) { - logger.error("discovery return empty list of endpoints"); - state.handleProblem(new UnexpectedResultException("discovery fail", EMPTY_DISCOVERY)); - return; - } - - logger.debug("successfully received ListEndpoints result with {} endpoints", - result.getEndpointsList().size()); - discoveryHandler.handleDiscoveryResult(result); - - state.handleOK(); - } catch (UnexpectedResultException ex) { - logger.error("discovery fail {}", response); - state.handleProblem(ex); - } - } - - private void runDiscovery() { - if (!updateInProgress.compareAndSet(false, true)) { - logger.debug("couldn't start update: already in progress"); - return; - } - - logger.debug("updating endpoints, calling ListEndpoints..."); - discoveryRpc.listEndpoints().whenComplete((response, ex) -> { - if (state.stopped) { - updateInProgress.set(false); - return; - } - - if (ex != null) { - Throwable cause = Async.unwrapCompletionException(ex); - logger.warn("couldn't perform discovery with exception", cause); - state.handleProblem(cause); - } - if (response != null) { - handleDiscoveryResponse(response); - } - - updateInProgress.set(false); - - if (state.isReady && !state.stopped) { - scheduleNextDiscovery(); - } - }); - } - - private static class State { - private volatile Instant lastUpdateTime = Instant.now(); - private volatile boolean isReady; - private volatile boolean stopped; - private volatile RuntimeException lastProblem; - private final Object readyLock = new Object(); - - public void handleOK() { - this.lastUpdateTime = Instant.now(); - - if (!isReady) { - // Wake up all waiting locks - synchronized (readyLock) { - isReady = true; - lastProblem = null; - readyLock.notifyAll(); - } - } - } - - public void handleProblem(Throwable ex) { - if (isReady) { - logger.error("discovery problem", ex); - return; - } - - // Wake up all waiting locks - synchronized (readyLock) { - if (isReady) { - logger.error("discovery problem", ex); - return; - } - - isReady = false; - if (ex instanceof RuntimeException) { - lastProblem = (RuntimeException) ex; - } else { - lastProblem = new RuntimeException("Check ready problem", ex); - } - readyLock.notifyAll(); - } - } - - public void waitReady(long timeoutMillis) { - if (isReady) { - return; - } - - synchronized (readyLock) { - if (isReady) { - return; - } - - if (lastProblem != null) { - throw lastProblem; - } - - try { - // waiting for initialization - readyLock.wait(timeoutMillis); - - if (lastProblem != null) { - throw lastProblem; - } - - if (!isReady) { - throw new RuntimeException("Ydb transport in not ready"); - } - } catch (InterruptedException ex) { - logger.warn("ydb transport wait for ready interrupted", ex); - Thread.currentThread().interrupt(); - } - } - } - } -} From 82b52a4d562b117e23a0aef0a0bbf314555c0706 Mon Sep 17 00:00:00 2001 From: Alexandr Gorshenin Date: Mon, 22 Apr 2024 18:17:46 +0100 Subject: [PATCH 07/15] Update implementation of YdbDiscovery --- .../ydb/core/impl/MultiChannelTransport.java | 8 +- .../ydb/core/impl/SingleChannelTransport.java | 9 +- .../java/tech/ydb/core/impl/YdbDiscovery.java | 94 ++++++++++--------- .../tech/ydb/core/impl/YdbTransportImpl.java | 52 +++++----- .../ydb/core/impl/auth/AuthCallOptions.java | 3 +- 5 files changed, 80 insertions(+), 86 deletions(-) diff --git a/core/src/main/java/tech/ydb/core/impl/MultiChannelTransport.java b/core/src/main/java/tech/ydb/core/impl/MultiChannelTransport.java index d1ddbfaba..0eb9ce548 100644 --- a/core/src/main/java/tech/ydb/core/impl/MultiChannelTransport.java +++ b/core/src/main/java/tech/ydb/core/impl/MultiChannelTransport.java @@ -48,13 +48,7 @@ public MultiChannelTransport(GrpcTransportBuilder builder, List hos .map(host -> new EndpointRecord(host.getHost(), host.getPortOrDefault(YdbTransportImpl.DEFAULT_PORT))) .collect(Collectors.toList()); - this.callOptions = new AuthCallOptions(scheduler, - database, - endpoints, - channelFactory, - builder - ); - + this.callOptions = new AuthCallOptions(scheduler, endpoints, channelFactory, builder); this.channelPool = new GrpcChannelPool(channelFactory, scheduler); this.endpointPool = new EndpointPool(BalancingSettings.defaultInstance()); this.endpointPool.setNewState(null, endpoints); diff --git a/core/src/main/java/tech/ydb/core/impl/SingleChannelTransport.java b/core/src/main/java/tech/ydb/core/impl/SingleChannelTransport.java index 7c60c7da6..4b8a6ef16 100644 --- a/core/src/main/java/tech/ydb/core/impl/SingleChannelTransport.java +++ b/core/src/main/java/tech/ydb/core/impl/SingleChannelTransport.java @@ -1,6 +1,6 @@ package tech.ydb.core.impl; -import java.util.Collections; +import java.util.Arrays; import java.util.concurrent.ScheduledExecutorService; import com.google.common.base.Strings; @@ -36,12 +36,7 @@ public SingleChannelTransport(GrpcTransportBuilder builder) { this.channel = new GrpcChannel(endpoint, channelFactory, true); this.scheduler = builder.getSchedulerFactory().get(); - this.callOptions = new AuthCallOptions(scheduler, - database, - Collections.singletonList(endpoint), - channelFactory, - builder - ); + this.callOptions = new AuthCallOptions(scheduler, Arrays.asList(endpoint), channelFactory, builder); } @Override diff --git a/core/src/main/java/tech/ydb/core/impl/YdbDiscovery.java b/core/src/main/java/tech/ydb/core/impl/YdbDiscovery.java index 0a127eeb0..415fc8a70 100644 --- a/core/src/main/java/tech/ydb/core/impl/YdbDiscovery.java +++ b/core/src/main/java/tech/ydb/core/impl/YdbDiscovery.java @@ -1,6 +1,5 @@ package tech.ydb.core.impl; -import java.time.Clock; import java.time.Duration; import java.time.Instant; import java.util.List; @@ -29,7 +28,7 @@ * @author Nikolay Perfilov * @author Aleksandr Gorshenin */ -public abstract class YdbDiscovery { +public class YdbDiscovery { private static final Status EMPTY_DISCOVERY = Status.of(StatusCode.CLIENT_DISCOVERY_FAILED) .withIssues(Issue.of("Discovery return empty list of endpoints", Issue.Severity.ERROR)); @@ -40,21 +39,29 @@ public abstract class YdbDiscovery { private static final Logger logger = LoggerFactory.getLogger(YdbDiscovery.class); - private final Clock clock; + public interface Handler { + Instant instant(); + GrpcTransport createDiscoveryTransport(); + boolean forceDiscovery(); + void handleEndpoints(List endpoints, String selfLocation); + } + + private final Handler handler; private final ScheduledExecutorService scheduler; private final String discoveryDatabase; private final Duration discoveryTimeout; private final Object readyObj = new Object(); - private volatile boolean isStopped = false; private volatile Instant lastUpdateTime; private volatile Future currentSchedule = null; - private RuntimeException lastException = null; + private volatile boolean isStarted = false; + private volatile boolean isStopped = false; + private volatile Throwable lastException = null; - public YdbDiscovery(Clock clock, ScheduledExecutorService scheduler, String database, Duration timeout) { - this.clock = clock; + public YdbDiscovery(Handler handler, ScheduledExecutorService scheduler, String database, Duration timeout) { + this.handler = handler; this.scheduler = scheduler; - this.lastUpdateTime = clock.instant(); + this.lastUpdateTime = handler.instant(); this.discoveryDatabase = database; this.discoveryTimeout = timeout; } @@ -77,19 +84,30 @@ public void stop() { } } - public void waitReady() { + public void waitReady() throws IllegalStateException { + if (isStarted) { + return; + } + synchronized (readyObj) { try { - long waitMillis = 2 * discoveryTimeout.toMillis(); - readyObj.wait(waitMillis); + if (isStarted) { + return; + } + + readyObj.wait(discoveryTimeout.toMillis()); } catch (InterruptedException ex) { Thread.currentThread().interrupt(); - handleThrowable(ex); + lastException = new IllegalStateException("Discovery waiting interrupted", ex); } } - if (lastException != null) { - throw lastException; + if (!isStarted) { + if (lastException != null) { + throw new IllegalStateException("Discovery failed", lastException); + } else { + throw new IllegalStateException("Discovery is not ready"); + } } } @@ -105,11 +123,11 @@ private void tick() { return; } - if (forceDiscovery()) { + if (handler.forceDiscovery()) { logger.debug("launching discovery by endpoint pessimization"); runDiscovery(); } else { - if (Instant.now().isAfter(lastUpdateTime.plusSeconds(DISCOVERY_PERIOD_NORMAL_SECONDS))) { + if (handler.instant().isAfter(lastUpdateTime.plusSeconds(DISCOVERY_PERIOD_NORMAL_SECONDS))) { logger.debug("launching discovery in normal mode"); runDiscovery(); } else { @@ -119,18 +137,10 @@ private void tick() { } } - protected abstract GrpcTransport createDiscoveryTransport(); - protected abstract boolean forceDiscovery(); - protected abstract void handleEndpoints(List endpoints, String selfLocation); - private void runDiscovery() { - if (isStopped) { - return; - } - - lastUpdateTime = clock.instant(); + lastUpdateTime = handler.instant(); - final GrpcTransport transport = createDiscoveryTransport(); + final GrpcTransport transport = handler.createDiscoveryTransport(); logger.debug("execute list endpoints on {} with timeout {}", transport, discoveryTimeout); DiscoveryProtos.ListEndpointsRequest request = DiscoveryProtos.ListEndpointsRequest.newBuilder() .setDatabase(discoveryDatabase) @@ -151,13 +161,20 @@ private void runDiscovery() { private void handleThrowable(Throwable th) { synchronized (readyObj) { - if (th instanceof RuntimeException) { - lastException = (RuntimeException) th; - } else { - lastException = new RuntimeException("Discovery failed", th); - } + lastException = th; + readyObj.notifyAll(); + } + scheduleNextTick(); + } + + private void handleOk(String selfLocation, List endpoints) { + synchronized (readyObj) { + isStarted = true; + lastException = null; + handler.handleEndpoints(endpoints, selfLocation); readyObj.notifyAll(); } + scheduleNextTick(); } private void handleDiscoveryResult(Result response, Throwable th) { @@ -165,7 +182,6 @@ private void handleDiscoveryResult(Result r Throwable cause = Async.unwrapCompletionException(th); logger.warn("couldn't perform discovery with exception", cause); handleThrowable(cause); - scheduleNextTick(); return; } @@ -173,8 +189,7 @@ private void handleDiscoveryResult(Result r DiscoveryProtos.ListEndpointsResult result = response.getValue(); if (result.getEndpointsList().isEmpty()) { logger.error("discovery return empty list of endpoints"); - handleThrowable(new UnexpectedResultException("Discovery failed", EMPTY_DISCOVERY)); - scheduleNextTick(); + handleThrowable(new UnexpectedResultException("Discovery list is empty", EMPTY_DISCOVERY)); return; } @@ -183,17 +198,10 @@ private void handleDiscoveryResult(Result r .collect(Collectors.toList()); logger.debug("successfully received ListEndpoints result with {} endpoints", records.size()); - synchronized (readyObj) { - lastException = null; - handleEndpoints(records, result.getSelfLocation()); - readyObj.notifyAll(); - } - - scheduleNextTick(); + handleOk(result.getSelfLocation(), records); } catch (UnexpectedResultException ex) { logger.error("discovery fail {}", response); - handleThrowable(th); - scheduleNextTick(); + handleThrowable(ex); } } } diff --git a/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java b/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java index 28ea51451..0b3172d46 100644 --- a/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java +++ b/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java @@ -2,8 +2,9 @@ import java.net.URI; import java.net.URISyntaxException; -import java.time.Clock; import java.time.Duration; +import java.time.Instant; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.concurrent.ScheduledExecutorService; @@ -33,38 +34,34 @@ public class YdbTransportImpl extends BaseGrpcTransport { private static final Logger logger = LoggerFactory.getLogger(YdbTransportImpl.class); private final String database; + private final EndpointRecord discoveryEndpoint; private final ScheduledExecutorService scheduler; + private final ManagedChannelFactory channelFactory; private final AuthCallOptions callOptions; private final EndpointPool endpointPool; private final GrpcChannelPool channelPool; - private final YdbDiscoveryImpl discovery; + private final YdbDiscovery discovery; public YdbTransportImpl(GrpcTransportBuilder builder) { - this.database = Strings.nullToEmpty(builder.getDatabase()); - - ManagedChannelFactory channelFactory = builder.getManagedChannelFactory(); BalancingSettings balancingSettings = getBalancingSettings(builder); - EndpointRecord discoveryEndpoint = getDiscoveryEndpoint(builder); Duration discoveryTimeout = Duration.ofMillis(builder.getDiscoveryTimeoutMillis()); + this.database = Strings.nullToEmpty(builder.getDatabase()); + this.discoveryEndpoint = getDiscoveryEndpoint(builder); + logger.info("Create YDB transport with endpoint {} and {}", discoveryEndpoint, balancingSettings); + this.channelFactory = builder.getManagedChannelFactory(); this.scheduler = builder.getSchedulerFactory().get(); - this.callOptions = new AuthCallOptions(scheduler, - database, - Collections.singletonList(discoveryEndpoint), - channelFactory, - builder - ); - + this.callOptions = new AuthCallOptions(scheduler, Arrays.asList(discoveryEndpoint), channelFactory, builder); this.channelPool = new GrpcChannelPool(channelFactory, scheduler); this.endpointPool = new EndpointPool(balancingSettings); - this.discovery = new YdbDiscoveryImpl(channelFactory, discoveryEndpoint, discoveryTimeout); + this.discovery = new YdbDiscovery(new DiscoveryHandler(), scheduler, database, discoveryTimeout); } public void start(GrpcTransportBuilder.InitMode mode) { if (mode == GrpcTransportBuilder.InitMode.ASYNC_FALLBACK) { - endpointPool.setNewState(null, Collections.singletonList(discovery.discoveryEndpoint)); + endpointPool.setNewState(null, Collections.singletonList(discoveryEndpoint)); } discovery.start(); @@ -74,6 +71,11 @@ public void start(GrpcTransportBuilder.InitMode mode) { } } + @Override + public String toString() { + return "YdbTransport{endpoint=" + discoveryEndpoint + ", database=" + database + "}"; + } + @Deprecated public void startAsync(Runnable readyWatcher) { discovery.start(); @@ -153,7 +155,7 @@ public AuthCallOptions getAuthCallOptions() { @Override protected GrpcChannel getChannel(GrpcRequestSettings settings) { EndpointRecord endpoint = endpointPool.getEndpoint(settings.getPreferredNodeID()); - while (endpoint == null) { + if (endpoint == null) { discovery.waitReady(); endpoint = endpointPool.getEndpoint(settings.getPreferredNodeID()); } @@ -168,29 +170,25 @@ protected void updateChannelStatus(GrpcChannel channel, io.grpc.Status status) { } } - private class YdbDiscoveryImpl extends YdbDiscovery { - private final ManagedChannelFactory channelFactory; - private final EndpointRecord discoveryEndpoint; - - YdbDiscoveryImpl(ManagedChannelFactory channelFactory, EndpointRecord endpoint, Duration timeout) { - super(Clock.systemUTC(), scheduler, database, timeout); - this.channelFactory = channelFactory; - this.discoveryEndpoint = endpoint; + private class DiscoveryHandler implements YdbDiscovery.Handler { + @Override + public Instant instant() { + return Instant.now(); } @Override - protected boolean forceDiscovery() { + public boolean forceDiscovery() { return endpointPool.needToRunDiscovery(); } @Override - protected void handleEndpoints(List endpoints, String selfLocation) { + public void handleEndpoints(List endpoints, String selfLocation) { List removed = endpointPool.setNewState(selfLocation, endpoints); channelPool.removeChannels(removed); } @Override - protected GrpcTransport createDiscoveryTransport() { + public GrpcTransport createDiscoveryTransport() { return new FixedCallOptionsTransport(scheduler, callOptions, database, discoveryEndpoint, channelFactory); } } diff --git a/core/src/main/java/tech/ydb/core/impl/auth/AuthCallOptions.java b/core/src/main/java/tech/ydb/core/impl/auth/AuthCallOptions.java index 5ae7231b9..4cd9ac4c0 100644 --- a/core/src/main/java/tech/ydb/core/impl/auth/AuthCallOptions.java +++ b/core/src/main/java/tech/ydb/core/impl/auth/AuthCallOptions.java @@ -31,7 +31,6 @@ public AuthCallOptions() { public AuthCallOptions( ScheduledExecutorService scheduler, - String database, List endpoints, ManagedChannelFactory channelFactory, GrpcTransportBuilder builder) { @@ -39,7 +38,7 @@ public AuthCallOptions( AuthRpcProvider authProvider = builder.getAuthProvider(); if (authProvider != null) { - GrpcAuthRpc rpc = new GrpcAuthRpc(endpoints, scheduler, database, channelFactory); + GrpcAuthRpc rpc = new GrpcAuthRpc(endpoints, scheduler, builder.getDatabase(), channelFactory); authIdentity = builder.getAuthProvider().createAuthIdentity(rpc); } else { authIdentity = null; From d9aad8d11d441e3d68baef39d677a67386b30e5d Mon Sep 17 00:00:00 2001 From: Alexandr Gorshenin Date: Mon, 22 Apr 2024 18:18:01 +0100 Subject: [PATCH 08/15] Added tests for YdbDiscovery --- .../java/tech/ydb/core/impl/MockedClock.java | 68 +++++ .../tech/ydb/core/impl/MockedScheduler.java | 246 +++++++++++++++++ .../tech/ydb/core/impl/MockedTransport.java | 125 +++++++++ .../tech/ydb/core/impl/YdbDiscoveryTest.java | 247 ++++++++++++++++++ 4 files changed, 686 insertions(+) create mode 100644 core/src/test/java/tech/ydb/core/impl/MockedClock.java create mode 100644 core/src/test/java/tech/ydb/core/impl/MockedScheduler.java create mode 100644 core/src/test/java/tech/ydb/core/impl/MockedTransport.java create mode 100644 core/src/test/java/tech/ydb/core/impl/YdbDiscoveryTest.java diff --git a/core/src/test/java/tech/ydb/core/impl/MockedClock.java b/core/src/test/java/tech/ydb/core/impl/MockedClock.java new file mode 100644 index 000000000..d875eed9e --- /dev/null +++ b/core/src/test/java/tech/ydb/core/impl/MockedClock.java @@ -0,0 +1,68 @@ +package tech.ydb.core.impl; + +import java.time.Clock; +import java.time.Instant; +import java.time.ZoneId; + +/** + * + * @author Aleksandr Gorshenin + */ +public class MockedClock extends Clock { + private final ZoneId zoneID; + private final MockedInstant mock; + + private MockedClock(ZoneId zoneId, MockedInstant instant) { + this.zoneID = zoneId; + this.mock = instant; + } + + @Override + public ZoneId getZone() { + return zoneID; + } + + @Override + public Clock withZone(ZoneId zone) { + return new MockedClock(zone, mock); + } + + @Override + public Instant instant() { + return mock.instant(); + } + + public void goToFuture(Instant future) { + mock.goToFuture(future); + } + + public void reset(Instant now) { + mock.reset(now); + } + + public static MockedClock create(ZoneId zoneId) { + return new MockedClock(zoneId, new MockedInstant(Instant.now())); + } + + private static class MockedInstant { + private volatile Instant now; + + public MockedInstant(Instant now) { + reset(now); + } + + private void reset(Instant now) { + this.now = now; + } + + private Instant instant() { + return now; + } + + private void goToFuture(Instant future) { + if (future.isAfter(now)) { + this.now = future; + } + } + } +} diff --git a/core/src/test/java/tech/ydb/core/impl/MockedScheduler.java b/core/src/test/java/tech/ydb/core/impl/MockedScheduler.java new file mode 100644 index 000000000..0a264b133 --- /dev/null +++ b/core/src/test/java/tech/ydb/core/impl/MockedScheduler.java @@ -0,0 +1,246 @@ +package tech.ydb.core.impl; + +import java.time.Instant; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Queue; +import java.util.concurrent.Callable; +import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.Delayed; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.FutureTask; +import java.util.concurrent.RunnableScheduledFuture; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + +import org.junit.Assert; + +/** + * + * @author Aleksandr Gorshenin + */ +public class MockedScheduler implements ScheduledExecutorService { + private final MockedClock clock; + private final Queue> tasks = new ConcurrentLinkedQueue<>(); + private volatile boolean stopped; + + public MockedScheduler(MockedClock clock) { + this.clock = clock; + } + + public Checker check() { + return new Checker(); + } + + public void runNextTask() { + MockedTask next = tasks.poll(); + Assert.assertNotNull("Scheduler's queue is empty", next); + clock.goToFuture(next.time); + next.run(); + if (next.time != null) { + tasks.add(next); + } + } + + @Override + public ScheduledFuture schedule(Runnable command, long delay, TimeUnit unit) { + Instant time = clock.instant().plusNanos(unit.toNanos(delay)); + MockedTask task = new MockedTask(command, null, time); + tasks.add(task); + return task; + } + + @Override + public ScheduledFuture schedule(Callable callable, long delay, TimeUnit unit) { + Instant time = clock.instant().plusNanos(unit.toNanos(delay)); + MockedTask task = new MockedTask<>(callable, time); + tasks.add(task); + return task; + } + + @Override + public ScheduledFuture scheduleAtFixedRate(Runnable command, long initialDelay, long period, TimeUnit unit) { + Instant time = clock.instant().plusNanos(unit.toNanos(initialDelay)); + MockedTask task = new MockedTask(command, null, time, unit.toMillis(period)); + tasks.add(task); + return task; + } + + @Override + public ScheduledFuture scheduleWithFixedDelay(Runnable command, long initialDelay, long delay, TimeUnit unit) { + Instant time = clock.instant().plusNanos(unit.toNanos(initialDelay)); + MockedTask task = new MockedTask(command, null, time, -unit.toMillis(initialDelay)); + tasks.add(task); + return task; + } + + @Override + public void shutdown() { + stopped = true; + } + + @Override + public List shutdownNow() { + stopped = true; + return Collections.emptyList(); + } + + @Override + public boolean isShutdown() { + return stopped; + } + + @Override + public boolean isTerminated() { + return stopped; + } + + @Override + public boolean awaitTermination(long timeout, TimeUnit unit) throws InterruptedException { + return true; + } + + @Override + public Future submit(Runnable task) { + return schedule(task, 0, TimeUnit.MILLISECONDS); + } + + @Override + public Future submit(Runnable task, T result) { + return schedule(Executors.callable(task, result), 0, TimeUnit.MILLISECONDS); + } + + @Override + public Future submit(Callable task) { + return schedule(task, 0, TimeUnit.MILLISECONDS); + } + + @Override + public List> invokeAll(Collection> tasks) throws InterruptedException { + throw new UnsupportedOperationException("Not supported yet."); + } + + @Override + public List> invokeAll(Collection> tasks, long timeout, TimeUnit unit) throws InterruptedException { + throw new UnsupportedOperationException("Not supported yet."); + } + + @Override + public T invokeAny(Collection> tasks) throws InterruptedException, ExecutionException { + throw new UnsupportedOperationException("Not supported yet."); + } + + @Override + public T invokeAny(Collection> tasks, long timeout, TimeUnit unit) throws InterruptedException, ExecutionException, TimeoutException { + throw new UnsupportedOperationException("Not supported yet."); + } + + @Override + public void execute(Runnable command) { + throw new UnsupportedOperationException("Not supported yet."); + } + + private class MockedTask extends FutureTask implements RunnableScheduledFuture { + private volatile Instant time; + private final long period; + + /** + * Creates a one-shot action with given trigger time. + */ + MockedTask(Runnable r, V result, Instant triggerTime) { + super(r, result); + this.time = triggerTime; + this.period = 0; + } + + MockedTask(Runnable r, V result, Instant triggerTime, + long period) { + super(r, result); + this.time = triggerTime; + this.period = period; + } + + MockedTask(Callable callable, Instant triggerTime) { + super(callable); + this.time = triggerTime; + this.period = 0; + } + + @Override + public long getDelay(TimeUnit unit) { + return unit.convert(time.toEpochMilli() - clock.millis(), TimeUnit.MILLISECONDS); + } + + @Override + public int compareTo(Delayed other) { + if (other == this) // compare zero if same object + return 0; + if (other instanceof MockedTask) { + MockedTask x = (MockedTask)other; + return time.compareTo(x.time); + } + + @SuppressWarnings("null") + long diff = getDelay(TimeUnit.MILLISECONDS) - other.getDelay(TimeUnit.MILLISECONDS); + return (diff < 0) ? -1 : (diff > 0) ? 1 : 0; + } + + @Override + public boolean isPeriodic() { + return period != 0; + } + + private void setNextRunTime() { + time = period > 0 ? time.plusMillis(period) : clock.instant().plusMillis(-period); + } + + @Override + public boolean cancel(boolean mayInterruptIfRunning) { + boolean cancelled = super.cancel(mayInterruptIfRunning); + if (cancelled) { + tasks.remove(this); + } + return cancelled; + } + + @Override + public void run() { + if (stopped) { + cancel(false); + return; + } + + if (isPeriodic()) { + if (super.runAndReset()) { + setNextRunTime(); + } + } else { + super.run(); + time = null; + } + } + } + + public class Checker { + public Checker isClosed() { + Assert.assertTrue("Scheduler is shutdown", isShutdown()); + Assert.assertTrue("Scheduler is terminated", isTerminated()); + return this; + } + + public Checker hasNoTasks() { + Assert.assertTrue("Scheduler hasn't tasks", tasks.isEmpty()); + return this; + } + + public Checker taskCount(int count) { + Assert.assertEquals("Scheduler has invalid count of tasks", count, tasks.size()); + return this; + } + } +} diff --git a/core/src/test/java/tech/ydb/core/impl/MockedTransport.java b/core/src/test/java/tech/ydb/core/impl/MockedTransport.java new file mode 100644 index 000000000..c37016dee --- /dev/null +++ b/core/src/test/java/tech/ydb/core/impl/MockedTransport.java @@ -0,0 +1,125 @@ +package tech.ydb.core.impl; + +import java.util.Queue; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.ScheduledExecutorService; + +import com.google.protobuf.Any; +import io.grpc.MethodDescriptor; +import org.junit.Assert; + +import tech.ydb.core.Result; +import tech.ydb.core.Status; +import tech.ydb.core.grpc.GrpcReadStream; +import tech.ydb.core.grpc.GrpcReadWriteStream; +import tech.ydb.core.grpc.GrpcRequestSettings; +import tech.ydb.core.grpc.GrpcTransport; +import tech.ydb.core.impl.pool.EndpointRecord; +import tech.ydb.proto.OperationProtos; +import tech.ydb.proto.StatusCodesProtos; +import tech.ydb.proto.discovery.DiscoveryProtos; +import tech.ydb.proto.discovery.v1.DiscoveryServiceGrpc; + +/** + * + * @author Aleksandr Gorshenin + */ +public class MockedTransport implements GrpcTransport { + private final String database; + private final ScheduledExecutorService scheduler; + private final Queue>> discovery; + + public MockedTransport(ScheduledExecutorService scheduler, String database) { + this.scheduler = scheduler; + this.database = database; + this.discovery = new ConcurrentLinkedQueue<>(); + } + + @Override + @SuppressWarnings("unchecked") + public CompletableFuture> unaryCall( + MethodDescriptor method, GrpcRequestSettings settings, ReqT request + ) { + if (method == DiscoveryServiceGrpc.getListEndpointsMethod()) { + CompletableFuture> future = new CompletableFuture<>(); + discovery.offer(future); + return (CompletableFuture) future; + } + throw new UnsupportedOperationException("Not supported yet."); + } + + @Override + public GrpcReadStream readStreamCall( + MethodDescriptor method, GrpcRequestSettings settings, ReqT request + ) { + throw new UnsupportedOperationException("Not supported yet."); + } + + @Override + public GrpcReadWriteStream readWriteStreamCall( + MethodDescriptor method, GrpcRequestSettings settings + ) { + throw new UnsupportedOperationException("Not supported yet."); + } + + @Override + public String getDatabase() { + return database; + } + + @Override + public ScheduledExecutorService getScheduler() { + return scheduler; + } + + @Override + public void close() { + } + + public void checkDiscoveryCallCount(int count) { + Assert.assertEquals("discovery mock queue size!", count, discovery.size()); + } + + public void completeNextDiscovery(String selfLocation, EndpointRecord... endpoints) { + CompletableFuture> future = discovery.poll(); + Assert.assertNotNull("discovery mock queue is empty!", future); + + DiscoveryProtos.ListEndpointsResult.Builder builder = DiscoveryProtos.ListEndpointsResult.newBuilder(); + for (EndpointRecord e : endpoints) { + DiscoveryProtos.EndpointInfo.Builder b = DiscoveryProtos.EndpointInfo.newBuilder(); + b.setAddress(e.getHost()); + b.setPort(e.getPort()); + b.setNodeId(e.getNodeId()); + if (e.getLocation() != null) { + b.setLocation(b.getLocation()); + } + + builder.addEndpoints(b.build()); + } + builder.setSelfLocation(selfLocation); + + OperationProtos.Operation operation = OperationProtos.Operation.newBuilder() + .setReady(true) + .setResult(Any.pack(builder.build())) + .setId("discovery-id") + .setStatus(StatusCodesProtos.StatusIds.StatusCode.SUCCESS) + .build(); + + future.complete(Result.success( + DiscoveryProtos.ListEndpointsResponse.newBuilder().setOperation(operation).build() + )); + } + + public void completeNextDiscovery(Status status) { + CompletableFuture> future = discovery.poll(); + Assert.assertNotNull("discovery mock queue is empty!", future); + future.complete(Result.fail(status)); + } + + public void completeNextDiscovery(Throwable th) { + CompletableFuture> future = discovery.poll(); + Assert.assertNotNull("discovery mock queue is empty!", future); + future.completeExceptionally(th); + } +} \ No newline at end of file diff --git a/core/src/test/java/tech/ydb/core/impl/YdbDiscoveryTest.java b/core/src/test/java/tech/ydb/core/impl/YdbDiscoveryTest.java new file mode 100644 index 000000000..38ad344d7 --- /dev/null +++ b/core/src/test/java/tech/ydb/core/impl/YdbDiscoveryTest.java @@ -0,0 +1,247 @@ +package tech.ydb.core.impl; + +import java.io.IOException; +import java.time.Duration; +import java.time.Instant; +import java.time.ZoneId; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; + +import org.junit.Assert; +import org.junit.Test; + +import tech.ydb.core.Status; +import tech.ydb.core.StatusCode; +import tech.ydb.core.UnexpectedResultException; +import tech.ydb.core.grpc.GrpcTransport; +import tech.ydb.core.impl.pool.EndpointRecord; + +/** + * + * @author Aleksandr Gorshenin + */ +public class YdbDiscoveryTest { + private final MockedClock clock = MockedClock.create(ZoneId.of("UTC")); + private final MockedScheduler scheduler = new MockedScheduler(clock); + private final MockedTransport transport = new MockedTransport(scheduler, "/testdb"); + + private class TestHandler implements YdbDiscovery.Handler { + private volatile boolean force = false; + + @Override + public Instant instant() { + return clock.instant(); + } + + @Override + public GrpcTransport createDiscoveryTransport() { + return transport; + } + + @Override + public boolean forceDiscovery() { + return force; + } + + @Override + public void handleEndpoints(List endpoints, String selfLocation) { + // nothing + } + } + + private T checkFutureException(CompletableFuture f, String message, Class clazz) { + CompletionException ce = Assert.assertThrows(CompletionException.class, f::join); + Assert.assertNotNull(ce.getCause()); + Assert.assertTrue(ce.getCause() instanceof IllegalStateException); + + IllegalStateException ex = (IllegalStateException) ce.getCause(); + Assert.assertEquals(message, ex.getMessage()); + if (clazz == null) { + Assert.assertNull(ex.getCause()); + return null; + } + + Assert.assertTrue(clazz.isAssignableFrom(ex.getCause().getClass())); + return clazz.cast(ex.getCause()); + } + + private CompletableFuture createWaitingFuture(YdbDiscovery discovery) { + return CompletableFuture.supplyAsync(() -> { + discovery.waitReady(); + return Boolean.TRUE; + }); + } + + + @Test + public void baseTest() { + scheduler.check().hasNoTasks(); + + YdbDiscovery discovery = new YdbDiscovery(new TestHandler(), scheduler, "/base", Duration.ofMillis(100)); + discovery.start(); + + scheduler.check().taskCount(1); + transport.checkDiscoveryCallCount(0); + + scheduler.runNextTask(); + transport.checkDiscoveryCallCount(1); + transport.completeNextDiscovery("self", new EndpointRecord("localhost", 12340)); + + discovery.waitReady(); + discovery.stop(); + + // stop is imdepotent operation + discovery.stop(); + } + + @Test + public void baseDiscoveryForcing() { + Duration tick = Duration.ofSeconds(5); // == YdbDiscovery.DISCOVERY_PERIOD_MIN_SECONDS + Duration normal = Duration.ofSeconds(60); // == YdbDiscovery.DISCOVERY_PERIOD_NORMAL_SECONDS + + TestHandler handler = new TestHandler(); + handler.force = false; + + scheduler.check().hasNoTasks(); + + YdbDiscovery discovery = new YdbDiscovery(handler, scheduler, "/forcing", Duration.ofMillis(100)); + + discovery.start(); + // first discovery + scheduler.check().taskCount(1); + transport.checkDiscoveryCallCount(0); + scheduler.runNextTask(); + transport.checkDiscoveryCallCount(1); + transport.completeNextDiscovery("self", new EndpointRecord("localhost", 12340)); + + // few next ticks will be dump + Instant started = clock.instant(); + Instant lastUpdate = started; + + scheduler.check().taskCount(1); + scheduler.runNextTask(); + while (!clock.instant().isAfter(started.plus(normal))) { + Assert.assertFalse("" + clock.instant() + " must be after " + lastUpdate.plus(tick), + clock.instant().isBefore(lastUpdate.plus(tick))); + lastUpdate = clock.instant(); + + transport.checkDiscoveryCallCount(0); + scheduler.check().taskCount(1); + scheduler.runNextTask(); + } + + // second discovery + scheduler.check().taskCount(0); + transport.checkDiscoveryCallCount(1); + transport.completeNextDiscovery("self", new EndpointRecord("localhost", 12340)); + + // force discovery + handler.force = true; + + scheduler.check().taskCount(1); + scheduler.runNextTask(); + scheduler.check().taskCount(0); + transport.checkDiscoveryCallCount(1); + + discovery.stop(); + } + + @Test + public void waitWaitingTest() { + scheduler.check().hasNoTasks(); + + final YdbDiscovery discovery = new YdbDiscovery(new TestHandler(), scheduler, "/wait", Duration.ofMillis(500)); + discovery.start(); + + scheduler.check().taskCount(1); + + CompletableFuture req1 = createWaitingFuture(discovery); + CompletableFuture req2 = createWaitingFuture(discovery); + Assert.assertFalse(req1.isDone()); + Assert.assertFalse(req2.isDone()); + + scheduler.runNextTask(); + + transport.checkDiscoveryCallCount(1); + transport.completeNextDiscovery("self", new EndpointRecord("localhost", 12340)); + + Assert.assertTrue(req1.join()); + Assert.assertTrue(req1.join()); + + CompletableFuture req3 = createWaitingFuture(discovery); + CompletableFuture req4 = createWaitingFuture(discovery); + + Assert.assertTrue(req3.join()); + Assert.assertTrue(req4.join()); + + discovery.stop(); + } + + @Test + public void failedDiscoveryTest() { + TestHandler handler = new TestHandler(); + handler.force = true; + + scheduler.check().hasNoTasks(); + + YdbDiscovery discovery = new YdbDiscovery(handler, scheduler, "/failed", Duration.ofMillis(100)); + discovery.start(); + + // test not ready error + CompletableFuture req1 = createWaitingFuture(discovery); + scheduler.check().taskCount(1); + Assert.assertFalse(req1.isDone()); + Assert.assertNull(checkFutureException(req1, "Discovery is not ready", null)); + + // test discovery status + CompletableFuture req2 = createWaitingFuture(discovery); + Assert.assertFalse(req2.isDone()); + + scheduler.check().taskCount(1); + scheduler.runNextTask(); + transport.checkDiscoveryCallCount(1); + transport.completeNextDiscovery(Status.of(StatusCode.UNAUTHORIZED)); + + UnexpectedResultException ex2 = checkFutureException(req2, "Discovery failed", UnexpectedResultException.class); + Assert.assertEquals(StatusCode.UNAUTHORIZED, ex2.getStatus().getCode()); + + // test discovery other status + CompletableFuture req3 = createWaitingFuture(discovery); + Assert.assertFalse(req3.isDone()); + + scheduler.check().taskCount(1); + scheduler.runNextTask(); + transport.checkDiscoveryCallCount(1); + transport.completeNextDiscovery(Status.of(StatusCode.TRANSPORT_UNAVAILABLE)); + + UnexpectedResultException ex3 = checkFutureException(req3, "Discovery failed", UnexpectedResultException.class); + Assert.assertEquals(StatusCode.TRANSPORT_UNAVAILABLE, ex3.getStatus().getCode()); + + // test empty discovery + CompletableFuture req4 = createWaitingFuture(discovery); + Assert.assertFalse(req4.isDone()); + + scheduler.check().taskCount(1); + scheduler.runNextTask(); + transport.checkDiscoveryCallCount(1); + transport.completeNextDiscovery("self"); // empty discovery + + UnexpectedResultException ex4 = checkFutureException(req4, "Discovery failed", UnexpectedResultException.class); + Assert.assertEquals(StatusCode.CLIENT_DISCOVERY_FAILED, ex4.getStatus().getCode()); + + // test discovery throwable + CompletableFuture req5 = createWaitingFuture(discovery); + Assert.assertFalse(req5.isDone()); + + scheduler.check().taskCount(1); + scheduler.runNextTask(); + transport.checkDiscoveryCallCount(1); + transport.completeNextDiscovery(new IOException("cannot open socket")); + + IOException ex5 = checkFutureException(req5, "Discovery failed", IOException.class); + Assert.assertEquals("cannot open socket", ex5.getMessage()); + + discovery.stop(); + } +} From 185d84928fbe930be3d0d8c196ba7f2e1685a4f2 Mon Sep 17 00:00:00 2001 From: Alexandr Gorshenin Date: Tue, 23 Apr 2024 12:13:26 +0100 Subject: [PATCH 09/15] Fixed obsolete buildAsync --- .../java/tech/ydb/core/impl/YdbDiscovery.java | 5 +- .../tech/ydb/core/impl/YdbTransportImpl.java | 1 + .../java/tech/ydb/core/impl/MockedCall.java | 165 +++++++++++++++ .../tech/ydb/core/impl/MockedScheduler.java | 34 ++- .../tech/ydb/core/impl/MockedTransport.java | 125 ------------ .../tech/ydb/core/impl/YdbDiscoveryTest.java | 173 +++++++++------- .../ydb/core/impl/YdbTransportImplTest.java | 193 ++++++++++++++++++ .../core/impl/pool/ManagedChannelMock.java | 14 ++ 8 files changed, 485 insertions(+), 225 deletions(-) create mode 100644 core/src/test/java/tech/ydb/core/impl/MockedCall.java delete mode 100644 core/src/test/java/tech/ydb/core/impl/MockedTransport.java create mode 100644 core/src/test/java/tech/ydb/core/impl/YdbTransportImplTest.java diff --git a/core/src/main/java/tech/ydb/core/impl/YdbDiscovery.java b/core/src/main/java/tech/ydb/core/impl/YdbDiscovery.java index 415fc8a70..61e1ddca1 100644 --- a/core/src/main/java/tech/ydb/core/impl/YdbDiscovery.java +++ b/core/src/main/java/tech/ydb/core/impl/YdbDiscovery.java @@ -71,7 +71,6 @@ public void start() { currentSchedule = scheduler.submit(() -> { logger.info("Waiting for init discovery..."); runDiscovery(); - logger.info("Discovery is finished"); }); } @@ -162,9 +161,9 @@ private void runDiscovery() { private void handleThrowable(Throwable th) { synchronized (readyObj) { lastException = th; + scheduleNextTick(); readyObj.notifyAll(); } - scheduleNextTick(); } private void handleOk(String selfLocation, List endpoints) { @@ -172,9 +171,9 @@ private void handleOk(String selfLocation, List endpoints) { isStarted = true; lastException = null; handler.handleEndpoints(endpoints, selfLocation); + scheduleNextTick(); readyObj.notifyAll(); } - scheduleNextTick(); } private void handleDiscoveryResult(Result response, Throwable th) { diff --git a/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java b/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java index 0b3172d46..44d47d45b 100644 --- a/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java +++ b/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java @@ -78,6 +78,7 @@ public String toString() { @Deprecated public void startAsync(Runnable readyWatcher) { + endpointPool.setNewState(null, Collections.singletonList(discoveryEndpoint)); discovery.start(); if (readyWatcher != null) { scheduler.execute(() -> { diff --git a/core/src/test/java/tech/ydb/core/impl/MockedCall.java b/core/src/test/java/tech/ydb/core/impl/MockedCall.java new file mode 100644 index 000000000..fa4b7bae4 --- /dev/null +++ b/core/src/test/java/tech/ydb/core/impl/MockedCall.java @@ -0,0 +1,165 @@ +package tech.ydb.core.impl; + + +import java.util.concurrent.Executor; + +import com.google.protobuf.Any; +import io.grpc.ClientCall; +import io.grpc.Metadata; +import io.grpc.Status; + +import tech.ydb.core.impl.pool.EndpointRecord; +import tech.ydb.proto.OperationProtos; +import tech.ydb.proto.StatusCodesProtos; +import tech.ydb.proto.discovery.DiscoveryProtos; + +public abstract class MockedCall extends ClientCall { + private final Executor executor; + + protected MockedCall(Executor executor) { + this.executor = executor; + } + + protected abstract void complete(Listener listener); + + @Override + public void start(Listener listener, Metadata headers) { + executor.execute(() -> complete(listener)); + } + + @Override + public void request(int numMessages) { + // nothing + } + + @Override + public void cancel(String message, Throwable cause) { + // nothing + } + + @Override + public void halfClose() { + // nothing + } + + @Override + public void sendMessage(ResT message) { + // nothing + } + + public static MockedCall neverAnswer() { + return neverAnswer(Runnable::run); + } + + public static MockedCall neverAnswer(Executor executor) { + return new MockedCall(executor) { + @Override + protected void complete(Listener listener) { } + }; + } + + public static MockedCall unavailable() { + return unavailable(Runnable::run); + } + + public static MockedCall unavailable(Executor executor) { + return new MockedCall(executor) { + @Override + protected void complete(Listener listener) { + listener.onClose(Status.UNAVAILABLE, null); + } + }; + } + + public static abstract class WhoAmICall extends + MockedCall { + protected WhoAmICall(Executor executor) { + super(executor); + } + } + + public static WhoAmICall whoAmICall(String user) { + return whoAmICall(Runnable::run, user); + } + + public static WhoAmICall whoAmICall(Executor executor, String user) { + OperationProtos.Operation operation = OperationProtos.Operation.newBuilder() + .setReady(true) + .setResult(Any.pack(DiscoveryProtos.WhoAmIResult.newBuilder().setUser(user).build())) + .setId("discovery-id") + .setStatus(StatusCodesProtos.StatusIds.StatusCode.SUCCESS) + .build(); + + + return new WhoAmICall(executor) { + @Override + protected void complete(Listener listener) { + listener.onMessage(DiscoveryProtos.WhoAmIResponse.newBuilder().setOperation(operation).build()); + listener.onClose(Status.OK, null); + } + }; + } + + public static abstract class DiscoveryCall extends + MockedCall { + protected DiscoveryCall(Executor executor) { + super(executor); + } + } + + public static DiscoveryCall discovery(String selfLocation, EndpointRecord... endpoints) { + return discovery(Runnable::run, selfLocation, endpoints); + } + + public static DiscoveryCall discovery(Executor executor, String selfLocation, EndpointRecord... endpoints) { + DiscoveryProtos.ListEndpointsResult.Builder builder = DiscoveryProtos.ListEndpointsResult.newBuilder(); + for (EndpointRecord e : endpoints) { + DiscoveryProtos.EndpointInfo.Builder b = DiscoveryProtos.EndpointInfo.newBuilder(); + b.setAddress(e.getHost()); + b.setPort(e.getPort()); + b.setNodeId(e.getNodeId()); + if (e.getLocation() != null) { + b.setLocation(b.getLocation()); + } + + builder.addEndpoints(b.build()); + } + builder.setSelfLocation(selfLocation); + + OperationProtos.Operation operation = OperationProtos.Operation.newBuilder() + .setReady(true) + .setResult(Any.pack(builder.build())) + .setId("discovery-id") + .setStatus(StatusCodesProtos.StatusIds.StatusCode.SUCCESS) + .build(); + + + return new DiscoveryCall(executor) { + @Override + protected void complete(Listener listener) { + listener.onMessage(DiscoveryProtos.ListEndpointsResponse.newBuilder().setOperation(operation).build()); + listener.onClose(Status.OK, null); + } + }; + } + + public static DiscoveryCall discoveryInternalError() { + return discoveryInternalError(Runnable::run); + } + + public static DiscoveryCall discoveryInternalError(Executor executor) { + OperationProtos.Operation operation = OperationProtos.Operation.newBuilder() + .setReady(true) + .setId("discovery-id") + .setStatus(StatusCodesProtos.StatusIds.StatusCode.INTERNAL_ERROR) + .build(); + + return new DiscoveryCall(executor) { + @Override + protected void complete(Listener listener) { + listener.onMessage(DiscoveryProtos.ListEndpointsResponse.newBuilder().setOperation(operation).build()); + listener.onClose(Status.OK, null); + } + }; + } +} diff --git a/core/src/test/java/tech/ydb/core/impl/MockedScheduler.java b/core/src/test/java/tech/ydb/core/impl/MockedScheduler.java index 0a264b133..3b8ff6746 100644 --- a/core/src/test/java/tech/ydb/core/impl/MockedScheduler.java +++ b/core/src/test/java/tech/ydb/core/impl/MockedScheduler.java @@ -33,11 +33,17 @@ public MockedScheduler(MockedClock clock) { this.clock = clock; } - public Checker check() { - return new Checker(); + public MockedScheduler hasNoTasks() { + Assert.assertTrue("Scheduler hasn't tasks", tasks.isEmpty()); + return this; } - public void runNextTask() { + public MockedScheduler hasTasksCount(int count) { + Assert.assertEquals("Scheduler has invalid count of tasks", count, tasks.size()); + return this; + } + + public MockedScheduler runNextTask() { MockedTask next = tasks.poll(); Assert.assertNotNull("Scheduler's queue is empty", next); clock.goToFuture(next.time); @@ -45,6 +51,8 @@ public void runNextTask() { if (next.time != null) { tasks.add(next); } + + return this; } @Override @@ -142,7 +150,7 @@ public T invokeAny(Collection> tasks, long timeout, Ti @Override public void execute(Runnable command) { - throw new UnsupportedOperationException("Not supported yet."); + schedule(command, 0, TimeUnit.MILLISECONDS); } private class MockedTask extends FutureTask implements RunnableScheduledFuture { @@ -225,22 +233,4 @@ public void run() { } } } - - public class Checker { - public Checker isClosed() { - Assert.assertTrue("Scheduler is shutdown", isShutdown()); - Assert.assertTrue("Scheduler is terminated", isTerminated()); - return this; - } - - public Checker hasNoTasks() { - Assert.assertTrue("Scheduler hasn't tasks", tasks.isEmpty()); - return this; - } - - public Checker taskCount(int count) { - Assert.assertEquals("Scheduler has invalid count of tasks", count, tasks.size()); - return this; - } - } } diff --git a/core/src/test/java/tech/ydb/core/impl/MockedTransport.java b/core/src/test/java/tech/ydb/core/impl/MockedTransport.java deleted file mode 100644 index c37016dee..000000000 --- a/core/src/test/java/tech/ydb/core/impl/MockedTransport.java +++ /dev/null @@ -1,125 +0,0 @@ -package tech.ydb.core.impl; - -import java.util.Queue; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ConcurrentLinkedQueue; -import java.util.concurrent.ScheduledExecutorService; - -import com.google.protobuf.Any; -import io.grpc.MethodDescriptor; -import org.junit.Assert; - -import tech.ydb.core.Result; -import tech.ydb.core.Status; -import tech.ydb.core.grpc.GrpcReadStream; -import tech.ydb.core.grpc.GrpcReadWriteStream; -import tech.ydb.core.grpc.GrpcRequestSettings; -import tech.ydb.core.grpc.GrpcTransport; -import tech.ydb.core.impl.pool.EndpointRecord; -import tech.ydb.proto.OperationProtos; -import tech.ydb.proto.StatusCodesProtos; -import tech.ydb.proto.discovery.DiscoveryProtos; -import tech.ydb.proto.discovery.v1.DiscoveryServiceGrpc; - -/** - * - * @author Aleksandr Gorshenin - */ -public class MockedTransport implements GrpcTransport { - private final String database; - private final ScheduledExecutorService scheduler; - private final Queue>> discovery; - - public MockedTransport(ScheduledExecutorService scheduler, String database) { - this.scheduler = scheduler; - this.database = database; - this.discovery = new ConcurrentLinkedQueue<>(); - } - - @Override - @SuppressWarnings("unchecked") - public CompletableFuture> unaryCall( - MethodDescriptor method, GrpcRequestSettings settings, ReqT request - ) { - if (method == DiscoveryServiceGrpc.getListEndpointsMethod()) { - CompletableFuture> future = new CompletableFuture<>(); - discovery.offer(future); - return (CompletableFuture) future; - } - throw new UnsupportedOperationException("Not supported yet."); - } - - @Override - public GrpcReadStream readStreamCall( - MethodDescriptor method, GrpcRequestSettings settings, ReqT request - ) { - throw new UnsupportedOperationException("Not supported yet."); - } - - @Override - public GrpcReadWriteStream readWriteStreamCall( - MethodDescriptor method, GrpcRequestSettings settings - ) { - throw new UnsupportedOperationException("Not supported yet."); - } - - @Override - public String getDatabase() { - return database; - } - - @Override - public ScheduledExecutorService getScheduler() { - return scheduler; - } - - @Override - public void close() { - } - - public void checkDiscoveryCallCount(int count) { - Assert.assertEquals("discovery mock queue size!", count, discovery.size()); - } - - public void completeNextDiscovery(String selfLocation, EndpointRecord... endpoints) { - CompletableFuture> future = discovery.poll(); - Assert.assertNotNull("discovery mock queue is empty!", future); - - DiscoveryProtos.ListEndpointsResult.Builder builder = DiscoveryProtos.ListEndpointsResult.newBuilder(); - for (EndpointRecord e : endpoints) { - DiscoveryProtos.EndpointInfo.Builder b = DiscoveryProtos.EndpointInfo.newBuilder(); - b.setAddress(e.getHost()); - b.setPort(e.getPort()); - b.setNodeId(e.getNodeId()); - if (e.getLocation() != null) { - b.setLocation(b.getLocation()); - } - - builder.addEndpoints(b.build()); - } - builder.setSelfLocation(selfLocation); - - OperationProtos.Operation operation = OperationProtos.Operation.newBuilder() - .setReady(true) - .setResult(Any.pack(builder.build())) - .setId("discovery-id") - .setStatus(StatusCodesProtos.StatusIds.StatusCode.SUCCESS) - .build(); - - future.complete(Result.success( - DiscoveryProtos.ListEndpointsResponse.newBuilder().setOperation(operation).build() - )); - } - - public void completeNextDiscovery(Status status) { - CompletableFuture> future = discovery.poll(); - Assert.assertNotNull("discovery mock queue is empty!", future); - future.complete(Result.fail(status)); - } - - public void completeNextDiscovery(Throwable th) { - CompletableFuture> future = discovery.poll(); - Assert.assertNotNull("discovery mock queue is empty!", future); - future.completeExceptionally(th); - } -} \ No newline at end of file diff --git a/core/src/test/java/tech/ydb/core/impl/YdbDiscoveryTest.java b/core/src/test/java/tech/ydb/core/impl/YdbDiscoveryTest.java index 38ad344d7..49ec1cf7a 100644 --- a/core/src/test/java/tech/ydb/core/impl/YdbDiscoveryTest.java +++ b/core/src/test/java/tech/ydb/core/impl/YdbDiscoveryTest.java @@ -1,6 +1,5 @@ package tech.ydb.core.impl; -import java.io.IOException; import java.time.Duration; import java.time.Instant; import java.time.ZoneId; @@ -8,14 +7,21 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; +import io.grpc.ConnectivityState; +import io.grpc.ManagedChannel; import org.junit.Assert; +import org.junit.Before; import org.junit.Test; +import org.mockito.Mockito; -import tech.ydb.core.Status; import tech.ydb.core.StatusCode; import tech.ydb.core.UnexpectedResultException; import tech.ydb.core.grpc.GrpcTransport; +import tech.ydb.core.impl.auth.AuthCallOptions; import tech.ydb.core.impl.pool.EndpointRecord; +import tech.ydb.core.impl.pool.ManagedChannelFactory; +import tech.ydb.proto.discovery.v1.DiscoveryServiceGrpc; + /** * @@ -24,30 +30,17 @@ public class YdbDiscoveryTest { private final MockedClock clock = MockedClock.create(ZoneId.of("UTC")); private final MockedScheduler scheduler = new MockedScheduler(clock); - private final MockedTransport transport = new MockedTransport(scheduler, "/testdb"); - - private class TestHandler implements YdbDiscovery.Handler { - private volatile boolean force = false; + private final ManagedChannel channel = Mockito.mock(ManagedChannel.class); + private final ManagedChannelFactory channelFactory = Mockito.mock(ManagedChannelFactory.class); - @Override - public Instant instant() { - return clock.instant(); - } - - @Override - public GrpcTransport createDiscoveryTransport() { - return transport; - } - - @Override - public boolean forceDiscovery() { - return force; - } + @Before + public void setUp() throws InterruptedException { + Mockito.when(channel.getState(Mockito.anyBoolean())).thenReturn(ConnectivityState.READY); + Mockito.when(channel.shutdown()).thenReturn(channel); + Mockito.when(channel.shutdownNow()).thenReturn(channel); + Mockito.when(channel.awaitTermination(Mockito.anyLong(), Mockito.any())).thenReturn(true); - @Override - public void handleEndpoints(List endpoints, String selfLocation) { - // nothing - } + Mockito.when(channelFactory.newManagedChannel(Mockito.any(), Mockito.anyInt())).thenReturn(channel); } private T checkFutureException(CompletableFuture f, String message, Class clazz) { @@ -73,20 +66,26 @@ private CompletableFuture createWaitingFuture(YdbDiscovery discovery) { }); } + private void verifyDiscoveryCount(int times) { + Mockito.verify(channel, Mockito.times(times)).newCall( + Mockito.eq(DiscoveryServiceGrpc.getListEndpointsMethod()), Mockito.any() + ); + } + @Test public void baseTest() { - scheduler.check().hasNoTasks(); + Mockito.when(channel.newCall(Mockito.eq(DiscoveryServiceGrpc.getListEndpointsMethod()), Mockito.any())) + .thenReturn(MockedCall.discovery("self", new EndpointRecord("localhost", 12340))); + + scheduler.hasNoTasks(); YdbDiscovery discovery = new YdbDiscovery(new TestHandler(), scheduler, "/base", Duration.ofMillis(100)); discovery.start(); - scheduler.check().taskCount(1); - transport.checkDiscoveryCallCount(0); - - scheduler.runNextTask(); - transport.checkDiscoveryCallCount(1); - transport.completeNextDiscovery("self", new EndpointRecord("localhost", 12340)); + verifyDiscoveryCount(0); + scheduler.hasTasksCount(1).runNextTask(); + verifyDiscoveryCount(1); discovery.waitReady(); discovery.stop(); @@ -97,74 +96,75 @@ public void baseTest() { @Test public void baseDiscoveryForcing() { + Mockito.when(channel.newCall(Mockito.eq(DiscoveryServiceGrpc.getListEndpointsMethod()), Mockito.any())) + .thenReturn(MockedCall.discovery("self", new EndpointRecord("localhost", 12340))) + .thenReturn(MockedCall.discovery("self", new EndpointRecord("localhost", 12340))) + .thenReturn(MockedCall.discovery("self", new EndpointRecord("localhost", 12340))); + Duration tick = Duration.ofSeconds(5); // == YdbDiscovery.DISCOVERY_PERIOD_MIN_SECONDS Duration normal = Duration.ofSeconds(60); // == YdbDiscovery.DISCOVERY_PERIOD_NORMAL_SECONDS TestHandler handler = new TestHandler(); handler.force = false; - scheduler.check().hasNoTasks(); + scheduler.hasNoTasks(); YdbDiscovery discovery = new YdbDiscovery(handler, scheduler, "/forcing", Duration.ofMillis(100)); discovery.start(); // first discovery - scheduler.check().taskCount(1); - transport.checkDiscoveryCallCount(0); - scheduler.runNextTask(); - transport.checkDiscoveryCallCount(1); - transport.completeNextDiscovery("self", new EndpointRecord("localhost", 12340)); + verifyDiscoveryCount(0); + scheduler.hasTasksCount(1).runNextTask(); + verifyDiscoveryCount(1); // few next ticks will be dump Instant started = clock.instant(); Instant lastUpdate = started; - scheduler.check().taskCount(1); - scheduler.runNextTask(); + scheduler.hasTasksCount(1).runNextTask(); while (!clock.instant().isAfter(started.plus(normal))) { + verifyDiscoveryCount(1); + Assert.assertFalse("" + clock.instant() + " must be after " + lastUpdate.plus(tick), clock.instant().isBefore(lastUpdate.plus(tick))); lastUpdate = clock.instant(); - transport.checkDiscoveryCallCount(0); - scheduler.check().taskCount(1); - scheduler.runNextTask(); + scheduler.hasTasksCount(1).runNextTask(); } // second discovery - scheduler.check().taskCount(0); - transport.checkDiscoveryCallCount(1); - transport.completeNextDiscovery("self", new EndpointRecord("localhost", 12340)); + verifyDiscoveryCount(2); + scheduler.hasTasksCount(1); // force discovery handler.force = true; - scheduler.check().taskCount(1); scheduler.runNextTask(); - scheduler.check().taskCount(0); - transport.checkDiscoveryCallCount(1); + verifyDiscoveryCount(3); discovery.stop(); } @Test public void waitWaitingTest() { - scheduler.check().hasNoTasks(); + Mockito.when(channel.newCall(Mockito.eq(DiscoveryServiceGrpc.getListEndpointsMethod()), Mockito.any())) + .thenReturn(MockedCall.discovery("self", new EndpointRecord("localhost", 12340))); + + scheduler.hasNoTasks(); final YdbDiscovery discovery = new YdbDiscovery(new TestHandler(), scheduler, "/wait", Duration.ofMillis(500)); discovery.start(); - scheduler.check().taskCount(1); + scheduler.hasTasksCount(1); CompletableFuture req1 = createWaitingFuture(discovery); CompletableFuture req2 = createWaitingFuture(discovery); Assert.assertFalse(req1.isDone()); Assert.assertFalse(req2.isDone()); + verifyDiscoveryCount(0); scheduler.runNextTask(); - - transport.checkDiscoveryCallCount(1); - transport.completeNextDiscovery("self", new EndpointRecord("localhost", 12340)); + verifyDiscoveryCount(1); Assert.assertTrue(req1.join()); Assert.assertTrue(req1.join()); @@ -180,17 +180,23 @@ public void waitWaitingTest() { @Test public void failedDiscoveryTest() { + Mockito.when(channel.newCall(Mockito.eq(DiscoveryServiceGrpc.getListEndpointsMethod()), Mockito.any())) + .thenReturn(MockedCall.unavailable()) + .thenReturn(MockedCall.discoveryInternalError()) + .thenReturn(MockedCall.discovery("self")) // Empty discovery is an error too + .thenThrow(new RuntimeException("Test io problem")); + TestHandler handler = new TestHandler(); handler.force = true; - scheduler.check().hasNoTasks(); + scheduler.hasNoTasks(); YdbDiscovery discovery = new YdbDiscovery(handler, scheduler, "/failed", Duration.ofMillis(100)); discovery.start(); // test not ready error CompletableFuture req1 = createWaitingFuture(discovery); - scheduler.check().taskCount(1); + scheduler.hasTasksCount(1); Assert.assertFalse(req1.isDone()); Assert.assertNull(checkFutureException(req1, "Discovery is not ready", null)); @@ -198,34 +204,28 @@ public void failedDiscoveryTest() { CompletableFuture req2 = createWaitingFuture(discovery); Assert.assertFalse(req2.isDone()); - scheduler.check().taskCount(1); - scheduler.runNextTask(); - transport.checkDiscoveryCallCount(1); - transport.completeNextDiscovery(Status.of(StatusCode.UNAUTHORIZED)); + scheduler.hasTasksCount(1).runNextTask(); + verifyDiscoveryCount(1); UnexpectedResultException ex2 = checkFutureException(req2, "Discovery failed", UnexpectedResultException.class); - Assert.assertEquals(StatusCode.UNAUTHORIZED, ex2.getStatus().getCode()); + Assert.assertEquals(StatusCode.TRANSPORT_UNAVAILABLE, ex2.getStatus().getCode()); // test discovery other status CompletableFuture req3 = createWaitingFuture(discovery); Assert.assertFalse(req3.isDone()); - scheduler.check().taskCount(1); - scheduler.runNextTask(); - transport.checkDiscoveryCallCount(1); - transport.completeNextDiscovery(Status.of(StatusCode.TRANSPORT_UNAVAILABLE)); + scheduler.hasTasksCount(1).runNextTask(); + verifyDiscoveryCount(2); UnexpectedResultException ex3 = checkFutureException(req3, "Discovery failed", UnexpectedResultException.class); - Assert.assertEquals(StatusCode.TRANSPORT_UNAVAILABLE, ex3.getStatus().getCode()); + Assert.assertEquals(StatusCode.INTERNAL_ERROR, ex3.getStatus().getCode()); // test empty discovery CompletableFuture req4 = createWaitingFuture(discovery); Assert.assertFalse(req4.isDone()); - scheduler.check().taskCount(1); - scheduler.runNextTask(); - transport.checkDiscoveryCallCount(1); - transport.completeNextDiscovery("self"); // empty discovery + scheduler.hasTasksCount(1).runNextTask(); + verifyDiscoveryCount(3); UnexpectedResultException ex4 = checkFutureException(req4, "Discovery failed", UnexpectedResultException.class); Assert.assertEquals(StatusCode.CLIENT_DISCOVERY_FAILED, ex4.getStatus().getCode()); @@ -234,14 +234,37 @@ public void failedDiscoveryTest() { CompletableFuture req5 = createWaitingFuture(discovery); Assert.assertFalse(req5.isDone()); - scheduler.check().taskCount(1); - scheduler.runNextTask(); - transport.checkDiscoveryCallCount(1); - transport.completeNextDiscovery(new IOException("cannot open socket")); + scheduler.hasTasksCount(1).runNextTask(); + verifyDiscoveryCount(4); - IOException ex5 = checkFutureException(req5, "Discovery failed", IOException.class); - Assert.assertEquals("cannot open socket", ex5.getMessage()); + RuntimeException ex5 = checkFutureException(req5, "Discovery failed", RuntimeException.class); + Assert.assertEquals("Test io problem", ex5.getMessage()); discovery.stop(); } + + private class TestHandler implements YdbDiscovery.Handler { + private volatile boolean force = false; + + @Override + public Instant instant() { + return clock.instant(); + } + + @Override + public GrpcTransport createDiscoveryTransport() { + EndpointRecord discovery = new EndpointRecord("unknown", 1234); + return new FixedCallOptionsTransport(scheduler, new AuthCallOptions(), "/test", discovery, channelFactory); + } + + @Override + public boolean forceDiscovery() { + return force; + } + + @Override + public void handleEndpoints(List endpoints, String selfLocation) { + // nothing + } + } } diff --git a/core/src/test/java/tech/ydb/core/impl/YdbTransportImplTest.java b/core/src/test/java/tech/ydb/core/impl/YdbTransportImplTest.java new file mode 100644 index 000000000..0f308cab6 --- /dev/null +++ b/core/src/test/java/tech/ydb/core/impl/YdbTransportImplTest.java @@ -0,0 +1,193 @@ +package tech.ydb.core.impl; + + +import java.time.Duration; +import java.util.Queue; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.Executor; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; + +import io.grpc.ConnectivityState; +import io.grpc.ManagedChannel; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mockito; + +import tech.ydb.core.Result; +import tech.ydb.core.StatusCode; +import tech.ydb.core.UnexpectedResultException; +import tech.ydb.core.grpc.GrpcRequestSettings; +import tech.ydb.core.grpc.GrpcTransport; +import tech.ydb.core.impl.pool.EndpointRecord; +import tech.ydb.core.impl.pool.ManagedChannelFactory; +import tech.ydb.core.operation.OperationBinder; +import tech.ydb.proto.discovery.DiscoveryProtos; +import tech.ydb.proto.discovery.v1.DiscoveryServiceGrpc; + +/** + * + * @author Aleksandr Gorshenin + */ +public class YdbTransportImplTest { + private final ScheduledExecutorService testScheduler = Executors.newSingleThreadScheduledExecutor(); + private final ManagedChannel discoveryChannel = Mockito.mock(ManagedChannel.class); + private final ManagedChannel transportChannel = Mockito.mock(ManagedChannel.class); + private final ManagedChannelFactory channelFactory = Mockito.mock(ManagedChannelFactory.class); + + @Before + public void setUp() throws InterruptedException { + Mockito.when(discoveryChannel.getState(Mockito.anyBoolean())).thenReturn(ConnectivityState.READY); + Mockito.when(discoveryChannel.shutdown()).thenReturn(discoveryChannel); + Mockito.when(discoveryChannel.shutdownNow()).thenReturn(discoveryChannel); + Mockito.when(discoveryChannel.awaitTermination(Mockito.anyLong(), Mockito.any())).thenReturn(true); + + Mockito.when(transportChannel.getState(Mockito.anyBoolean())).thenReturn(ConnectivityState.READY); + Mockito.when(transportChannel.shutdown()).thenReturn(transportChannel); + Mockito.when(transportChannel.shutdownNow()).thenReturn(transportChannel); + Mockito.when(transportChannel.awaitTermination(Mockito.anyLong(), Mockito.any())).thenReturn(true); + + Mockito.when(channelFactory.newManagedChannel(Mockito.eq("mocked"), Mockito.eq(2136))) + .thenReturn(discoveryChannel); + Mockito.when(channelFactory.newManagedChannel(Mockito.eq("node"), Mockito.eq(2136))) + .thenReturn(transportChannel); + } + + @After + public void shutdown() { + testScheduler.shutdown(); + } + + private CompletableFuture> whoAmI(GrpcTransport transport) { + GrpcRequestSettings settings = GrpcRequestSettings.newBuilder().build(); + DiscoveryProtos.WhoAmIRequest request = DiscoveryProtos.WhoAmIRequest.newBuilder().build(); + return transport.unaryCall(DiscoveryServiceGrpc.getWhoAmIMethod(), settings, request) + .thenApply(OperationBinder.bindSync( + DiscoveryProtos.WhoAmIResponse::getOperation, + DiscoveryProtos.WhoAmIResult.class + )); + } + + @Test + public void defaultBuildGoodTest() { + Mockito.when(discoveryChannel.newCall(Mockito.eq(DiscoveryServiceGrpc.getListEndpointsMethod()), Mockito.any())) + .thenReturn(MockedCall.discovery(testScheduler, "self", new EndpointRecord("node", 2136))); + + Mockito.when(transportChannel.newCall(Mockito.eq(DiscoveryServiceGrpc.getWhoAmIMethod()), Mockito.any())) + .thenReturn(MockedCall.whoAmICall(testScheduler, "i am node")); + + GrpcTransport transport = GrpcTransport.forConnectionString("grpc://mocked:2136/local") + .withChannelFactoryBuilder(builder -> channelFactory) + .build(); + + Result result = whoAmI(transport).join(); + Assert.assertTrue(result.isSuccess()); + Assert.assertEquals("i am node", result.getValue().getUser()); + + transport.close(); + } + + @Test + public void defaultBuildUnavailabledTest() { + Mockito.when(discoveryChannel.newCall(Mockito.eq(DiscoveryServiceGrpc.getListEndpointsMethod()), Mockito.any())) + .thenReturn(MockedCall.unavailable(testScheduler)); + + IllegalStateException ex = Assert.assertThrows(IllegalStateException.class, + () -> GrpcTransport.forConnectionString("grpc://mocked:2136/local") + .withDiscoveryTimeout(Duration.ofMillis(100)) + .withChannelFactoryBuilder(builder -> channelFactory) + .build() + ); + + Assert.assertEquals("Discovery failed", ex.getMessage()); + Assert.assertTrue(ex.getCause() instanceof UnexpectedResultException); + + UnexpectedResultException unexpecteed = (UnexpectedResultException) ex.getCause(); + Assert.assertEquals(StatusCode.TRANSPORT_UNAVAILABLE, unexpecteed.getStatus().getCode()); + } + + @Test + public void defaultBuildNotReadyTest() { + Mockito.when(discoveryChannel.newCall(Mockito.eq(DiscoveryServiceGrpc.getListEndpointsMethod()), Mockito.any())) + .thenReturn(MockedCall.neverAnswer(testScheduler)); + + IllegalStateException ex = Assert.assertThrows(IllegalStateException.class, + () -> GrpcTransport.forConnectionString("grpc://mocked:2136/local") + .withDiscoveryTimeout(Duration.ofMillis(200)) + .withChannelFactoryBuilder(builder -> channelFactory) + .build() + ); + + Assert.assertEquals("Discovery is not ready", ex.getMessage()); + Assert.assertNull(ex.getCause()); + } + + @Test + public void asyncBuildGoodTest() { + Ticker tickerDiscovery = new Ticker(); + Ticker tickerRequests = new Ticker(); + + Mockito.when(discoveryChannel.newCall(Mockito.eq(DiscoveryServiceGrpc.getListEndpointsMethod()), Mockito.any())) + .thenReturn(MockedCall.discovery(tickerDiscovery, "self", new EndpointRecord("node", 2136))); + + Mockito.when(discoveryChannel.newCall(Mockito.eq(DiscoveryServiceGrpc.getWhoAmIMethod()), Mockito.any())) + .thenReturn(MockedCall.whoAmICall(tickerRequests, "i am discovery")); + Mockito.when(transportChannel.newCall(Mockito.eq(DiscoveryServiceGrpc.getWhoAmIMethod()), Mockito.any())) + .thenReturn(MockedCall.whoAmICall(tickerRequests, "i am node")); + + CompletableFuture isReady = new CompletableFuture<>(); + + @SuppressWarnings("deprecation") + GrpcTransport transport = GrpcTransport.forConnectionString("grpc://mocked:2136/local") + .withChannelFactoryBuilder(builder -> channelFactory) + .buildAsync(() -> isReady.complete(null)); + + Assert.assertNotNull(transport); + Assert.assertFalse(isReady.isDone()); + + // before discovery completed we send requests to discovery endpoint + tickerRequests.noTasks(); + CompletableFuture> f1 = whoAmI(transport); + Assert.assertFalse(f1.isDone()); + + tickerRequests.runNextTask().noTasks(); + Assert.assertTrue(f1.isDone()); + Assert.assertEquals("i am discovery", f1.join().getValue().getUser()); + + // Complete discovery + Assert.assertFalse(isReady.isDone()); + tickerDiscovery.runNextTask().noTasks(); + + CompletableFuture> f2 = whoAmI(transport); + Assert.assertFalse(f2.isDone()); + tickerRequests.runNextTask().noTasks(); + Assert.assertTrue(f2.isDone()); + Assert.assertEquals("i am node", f2.join().getValue().getUser()); + + Assert.assertTrue(isReady.isDone()); + } + + public class Ticker implements Executor { + private final Queue tasks = new ConcurrentLinkedQueue<>(); + + @Override + public void execute(Runnable command) { + tasks.offer(command); + } + + public Ticker runNextTask() { + Runnable next = tasks.poll(); + Assert.assertNotNull("Executor's queue is empty", next); + next.run(); + return this; + } + + public Ticker noTasks() { + Assert.assertTrue("Executor have extra tasks", tasks.isEmpty()); + return this; + } + } +} diff --git a/core/src/test/java/tech/ydb/core/impl/pool/ManagedChannelMock.java b/core/src/test/java/tech/ydb/core/impl/pool/ManagedChannelMock.java index 267431028..22a59e407 100644 --- a/core/src/test/java/tech/ydb/core/impl/pool/ManagedChannelMock.java +++ b/core/src/test/java/tech/ydb/core/impl/pool/ManagedChannelMock.java @@ -15,6 +15,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import tech.ydb.core.grpc.GrpcTransportBuilder; + /** * * @author Aleksandr Gorshenin @@ -140,4 +142,16 @@ public boolean awaitTermination(long timeout, TimeUnit unit) throws InterruptedE } }; } + + public static ManagedChannelFactory.Builder MOCKED = (GrpcTransportBuilder builder) -> new ManagedChannelFactory() { + @Override + public ManagedChannel newManagedChannel(String host, int port) { + return good(); + } + + @Override + public long getConnectTimeoutMs() { + return builder.getConnectTimeoutMillis(); + } + }; } From c46e115b6b736669ab3b66212c37a6ec02d99296 Mon Sep 17 00:00:00 2001 From: Alexandr Gorshenin Date: Tue, 23 Apr 2024 12:15:34 +0100 Subject: [PATCH 10/15] Fixed javadoc --- .../main/java/tech/ydb/core/grpc/GrpcTransportBuilder.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/tech/ydb/core/grpc/GrpcTransportBuilder.java b/core/src/main/java/tech/ydb/core/grpc/GrpcTransportBuilder.java index a47f5b3f5..ef1d8f7ac 100644 --- a/core/src/main/java/tech/ydb/core/grpc/GrpcTransportBuilder.java +++ b/core/src/main/java/tech/ydb/core/grpc/GrpcTransportBuilder.java @@ -234,9 +234,9 @@ public GrpcTransportBuilder withBalancingSettings(BalancingSettings balancingSet /** * Set GrpcTransport's init mode. - * See {@link tech.ydb.core.grpc.GrpcTransport.InitMode } for details + * See {@link tech.ydb.core.grpc.GrpcTransportBuilder.InitMode } for details * - * @param initMode mode of tranport initialization + * @param initMode mode of transport initialization * @return GrpcTransportBuilder with the given initMode */ public GrpcTransportBuilder withInitMode(InitMode initMode) { From 5bd0d9388e59b11c25187fa28ab639010a66e687 Mon Sep 17 00:00:00 2001 From: Alexandr Gorshenin Date: Tue, 23 Apr 2024 12:17:58 +0100 Subject: [PATCH 11/15] Fixed test race --- core/src/test/java/tech/ydb/core/impl/YdbTransportImplTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/core/src/test/java/tech/ydb/core/impl/YdbTransportImplTest.java b/core/src/test/java/tech/ydb/core/impl/YdbTransportImplTest.java index 0f308cab6..3c12de02e 100644 --- a/core/src/test/java/tech/ydb/core/impl/YdbTransportImplTest.java +++ b/core/src/test/java/tech/ydb/core/impl/YdbTransportImplTest.java @@ -167,6 +167,7 @@ public void asyncBuildGoodTest() { Assert.assertTrue(f2.isDone()); Assert.assertEquals("i am node", f2.join().getValue().getUser()); + transport.close(); Assert.assertTrue(isReady.isDone()); } From 77f199ee464fcd100f73a331753d04fb745d801e Mon Sep 17 00:00:00 2001 From: Alexandr Gorshenin Date: Wed, 24 Apr 2024 20:40:24 +0100 Subject: [PATCH 12/15] Fixed typos --- .../ydb/core/grpc/GrpcTransportBuilder.java | 23 ++++++++++--------- .../java/tech/ydb/core/impl/YdbDiscovery.java | 4 ++-- .../tech/ydb/core/impl/YdbTransportImpl.java | 2 +- .../tech/ydb/core/impl/YdbDiscoveryTest.java | 2 +- 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/core/src/main/java/tech/ydb/core/grpc/GrpcTransportBuilder.java b/core/src/main/java/tech/ydb/core/grpc/GrpcTransportBuilder.java index ef1d8f7ac..04f2c4bc9 100644 --- a/core/src/main/java/tech/ydb/core/grpc/GrpcTransportBuilder.java +++ b/core/src/main/java/tech/ydb/core/grpc/GrpcTransportBuilder.java @@ -38,27 +38,28 @@ public class GrpcTransportBuilder { */ public enum InitMode { /** - * In synchronous mode, transport creation will wait for successful discovery of current the database nodes. Any + * In synchronous mode, transport creation will wait for successful discovery of current database nodes. Any * of discovery execution like an authentication error or a network issue will be thrown as RuntimeException. * It allows to catch configuration problems and stops the transport creating. */ SYNC, /** - * In asynchronous mode, transport creation will not block by the discovery waiting and will not throw any - * exceptions in the case of configuration problems. But any request with the transport will wait for the - * discovery and may throw an exception if it will not be completed. This mode allows to application not to be - * blocked during the transport initialization but guarantees that requests will be created only after - * complete initialization + * In asynchronous mode, transport creation will not be blocked while waiting for discovery response and will + * not throw any exceptions in case of configuration problems. But any request with the transport will wait for + * the discovery and may throw an exception if it will not be completed. This mode allows the application not + * to be blocked during the transport initialization any user request on this transport will wait for + * initialization completion before being sent to the server */ ASYNC, /** - * In fallback asynchronous mode, neither initialization the transport nor making the requests will block by - * the discovery waiting. In this case if the discovery is not completed - all requests will be sent to - * the discovery endpoint. Any discovery problems will be ignored. This mode allows to start working with - * the database without waiting for discovery to complete, but after its completion, existing connections (like - * grpc streams) can be interrupted to switch to new endpoint. + * In fallback asynchronous mode, neither transport creation nor user request execution will be blocked while + * initial discovery is in progress. In this case if the discovery is not completed, all requests will be sent + * to the discovery endpoint. Any discovery problems will be ignored. This mode allows to start working with the + * database without waiting for discovery to complete, but after its completion, existing long-running + * operations (like grpc streams) will be interrupted. + * Thus this mode is not recommended for long-running streams such as topic reading/writing. */ ASYNC_FALLBACK } diff --git a/core/src/main/java/tech/ydb/core/impl/YdbDiscovery.java b/core/src/main/java/tech/ydb/core/impl/YdbDiscovery.java index 61e1ddca1..2ba12eeec 100644 --- a/core/src/main/java/tech/ydb/core/impl/YdbDiscovery.java +++ b/core/src/main/java/tech/ydb/core/impl/YdbDiscovery.java @@ -42,7 +42,7 @@ public class YdbDiscovery { public interface Handler { Instant instant(); GrpcTransport createDiscoveryTransport(); - boolean forceDiscovery(); + boolean needToForceDiscovery(); void handleEndpoints(List endpoints, String selfLocation); } @@ -122,7 +122,7 @@ private void tick() { return; } - if (handler.forceDiscovery()) { + if (handler.needToForceDiscovery()) { logger.debug("launching discovery by endpoint pessimization"); runDiscovery(); } else { diff --git a/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java b/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java index 44d47d45b..0a768d6f0 100644 --- a/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java +++ b/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java @@ -178,7 +178,7 @@ public Instant instant() { } @Override - public boolean forceDiscovery() { + public boolean needToForceDiscovery() { return endpointPool.needToRunDiscovery(); } diff --git a/core/src/test/java/tech/ydb/core/impl/YdbDiscoveryTest.java b/core/src/test/java/tech/ydb/core/impl/YdbDiscoveryTest.java index 49ec1cf7a..93c439d4e 100644 --- a/core/src/test/java/tech/ydb/core/impl/YdbDiscoveryTest.java +++ b/core/src/test/java/tech/ydb/core/impl/YdbDiscoveryTest.java @@ -258,7 +258,7 @@ public GrpcTransport createDiscoveryTransport() { } @Override - public boolean forceDiscovery() { + public boolean needToForceDiscovery() { return force; } From 16d8598860f1530ce884120fdeb1321fa529be86 Mon Sep 17 00:00:00 2001 From: Alexandr Gorshenin Date: Wed, 24 Apr 2024 20:55:51 +0100 Subject: [PATCH 13/15] Fixed data race in tests --- .../tech/ydb/core/impl/MockedScheduler.java | 30 +++++++++++++++++-- .../ydb/core/impl/YdbTransportImplTest.java | 9 +++--- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/core/src/test/java/tech/ydb/core/impl/MockedScheduler.java b/core/src/test/java/tech/ydb/core/impl/MockedScheduler.java index 3b8ff6746..7adea2ebb 100644 --- a/core/src/test/java/tech/ydb/core/impl/MockedScheduler.java +++ b/core/src/test/java/tech/ydb/core/impl/MockedScheduler.java @@ -6,6 +6,7 @@ import java.util.List; import java.util.Queue; import java.util.concurrent.Callable; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.Delayed; import java.util.concurrent.ExecutionException; @@ -27,7 +28,9 @@ public class MockedScheduler implements ScheduledExecutorService { private final MockedClock clock; private final Queue> tasks = new ConcurrentLinkedQueue<>(); - private volatile boolean stopped; + + private volatile boolean queueIsBlocked = false; + private volatile boolean stopped = false; public MockedScheduler(MockedClock clock) { this.clock = clock; @@ -43,7 +46,8 @@ public MockedScheduler hasTasksCount(int count) { return this; } - public MockedScheduler runNextTask() { + public synchronized MockedScheduler runNextTask() { + queueIsBlocked = true; MockedTask next = tasks.poll(); Assert.assertNotNull("Scheduler's queue is empty", next); clock.goToFuture(next.time); @@ -52,6 +56,7 @@ public MockedScheduler runNextTask() { tasks.add(next); } + queueIsBlocked = false; return this; } @@ -115,16 +120,33 @@ public boolean awaitTermination(long timeout, TimeUnit unit) throws InterruptedE @Override public Future submit(Runnable task) { + if (queueIsBlocked) { + task.run(); + return CompletableFuture.completedFuture(null); + } return schedule(task, 0, TimeUnit.MILLISECONDS); } @Override public Future submit(Runnable task, T result) { + if (queueIsBlocked) { + task.run(); + return CompletableFuture.completedFuture(result); + } return schedule(Executors.callable(task, result), 0, TimeUnit.MILLISECONDS); } @Override public Future submit(Callable task) { + if (queueIsBlocked) { + CompletableFuture future = new CompletableFuture<>(); + try { + future.complete(task.call()); + } catch (Exception ex) { + future.completeExceptionally(ex); + } + return future; + } return schedule(task, 0, TimeUnit.MILLISECONDS); } @@ -150,6 +172,10 @@ public T invokeAny(Collection> tasks, long timeout, Ti @Override public void execute(Runnable command) { + if (queueIsBlocked) { + command.run(); + return; + } schedule(command, 0, TimeUnit.MILLISECONDS); } diff --git a/core/src/test/java/tech/ydb/core/impl/YdbTransportImplTest.java b/core/src/test/java/tech/ydb/core/impl/YdbTransportImplTest.java index 3c12de02e..2b134372b 100644 --- a/core/src/test/java/tech/ydb/core/impl/YdbTransportImplTest.java +++ b/core/src/test/java/tech/ydb/core/impl/YdbTransportImplTest.java @@ -2,6 +2,7 @@ import java.time.Duration; +import java.time.ZoneId; import java.util.Queue; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentLinkedQueue; @@ -127,11 +128,11 @@ public void defaultBuildNotReadyTest() { @Test public void asyncBuildGoodTest() { - Ticker tickerDiscovery = new Ticker(); Ticker tickerRequests = new Ticker(); + MockedScheduler scheduler = new MockedScheduler(MockedClock.create(ZoneId.of("UTC"))); Mockito.when(discoveryChannel.newCall(Mockito.eq(DiscoveryServiceGrpc.getListEndpointsMethod()), Mockito.any())) - .thenReturn(MockedCall.discovery(tickerDiscovery, "self", new EndpointRecord("node", 2136))); + .thenReturn(MockedCall.discovery("self", new EndpointRecord("node", 2136))); Mockito.when(discoveryChannel.newCall(Mockito.eq(DiscoveryServiceGrpc.getWhoAmIMethod()), Mockito.any())) .thenReturn(MockedCall.whoAmICall(tickerRequests, "i am discovery")); @@ -142,6 +143,7 @@ public void asyncBuildGoodTest() { @SuppressWarnings("deprecation") GrpcTransport transport = GrpcTransport.forConnectionString("grpc://mocked:2136/local") + .withSchedulerFactory(() -> scheduler) .withChannelFactoryBuilder(builder -> channelFactory) .buildAsync(() -> isReady.complete(null)); @@ -159,7 +161,7 @@ public void asyncBuildGoodTest() { // Complete discovery Assert.assertFalse(isReady.isDone()); - tickerDiscovery.runNextTask().noTasks(); + scheduler.hasTasksCount(2).runNextTask().runNextTask(); CompletableFuture> f2 = whoAmI(transport); Assert.assertFalse(f2.isDone()); @@ -167,7 +169,6 @@ public void asyncBuildGoodTest() { Assert.assertTrue(f2.isDone()); Assert.assertEquals("i am node", f2.join().getValue().getUser()); - transport.close(); Assert.assertTrue(isReady.isDone()); } From 63c8f7e7fe1dad1d8a138a1f1887b4c66b830f82 Mon Sep 17 00:00:00 2001 From: Alexandr Gorshenin Date: Thu, 25 Apr 2024 14:44:47 +0100 Subject: [PATCH 14/15] Updated docs --- .../main/java/tech/ydb/core/grpc/GrpcTransportBuilder.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/tech/ydb/core/grpc/GrpcTransportBuilder.java b/core/src/main/java/tech/ydb/core/grpc/GrpcTransportBuilder.java index 04f2c4bc9..29e9e46d9 100644 --- a/core/src/main/java/tech/ydb/core/grpc/GrpcTransportBuilder.java +++ b/core/src/main/java/tech/ydb/core/grpc/GrpcTransportBuilder.java @@ -39,16 +39,15 @@ public class GrpcTransportBuilder { public enum InitMode { /** * In synchronous mode, transport creation will wait for successful discovery of current database nodes. Any - * of discovery execution like an authentication error or a network issue will be thrown as RuntimeException. - * It allows to catch configuration problems and stops the transport creating. + * errors on discovery execution like an authentication error or a network issue will be thrown as + * RuntimeException. It allows to catch configuration problems and stops the transport creating. */ SYNC, - /** * In asynchronous mode, transport creation will not be blocked while waiting for discovery response and will * not throw any exceptions in case of configuration problems. But any request with the transport will wait for * the discovery and may throw an exception if it will not be completed. This mode allows the application not - * to be blocked during the transport initialization any user request on this transport will wait for + * to be blocked during the transport initialization. Any user request on this transport will wait for * initialization completion before being sent to the server */ ASYNC, From b90e642bde2ac6d17be19f1faa05d6f010426a08 Mon Sep 17 00:00:00 2001 From: Alexandr Gorshenin Date: Thu, 25 Apr 2024 15:13:14 +0100 Subject: [PATCH 15/15] Added deadline check of discovery waitReady --- .../tech/ydb/core/impl/BaseGrpcTransport.java | 15 +++++++++++++-- .../java/tech/ydb/core/impl/YdbDiscovery.java | 5 +++-- .../java/tech/ydb/core/impl/YdbTransportImpl.java | 10 +++++++--- .../java/tech/ydb/core/impl/YdbDiscoveryTest.java | 9 +++++---- 4 files changed, 28 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/tech/ydb/core/impl/BaseGrpcTransport.java b/core/src/main/java/tech/ydb/core/impl/BaseGrpcTransport.java index 8dd790be4..d8dfae075 100644 --- a/core/src/main/java/tech/ydb/core/impl/BaseGrpcTransport.java +++ b/core/src/main/java/tech/ydb/core/impl/BaseGrpcTransport.java @@ -15,6 +15,7 @@ import tech.ydb.core.Result; import tech.ydb.core.Status; import tech.ydb.core.StatusCode; +import tech.ydb.core.UnexpectedResultException; import tech.ydb.core.grpc.GrpcReadStream; import tech.ydb.core.grpc.GrpcReadWriteStream; import tech.ydb.core.grpc.GrpcRequestSettings; @@ -28,7 +29,6 @@ import tech.ydb.core.impl.call.ReadWriteStreamCall; import tech.ydb.core.impl.call.UnaryCall; import tech.ydb.core.impl.pool.GrpcChannel; -import tech.ydb.core.utils.Async; /** * @@ -92,9 +92,12 @@ public CompletableFuture> unaryCall( } return new UnaryCall<>(call, handler).startCall(request, makeMetadataFromSettings(settings)); + } catch (UnexpectedResultException ex) { + logger.error("unary call with traceId {} unexprected status {}", settings.getTraceId(), ex.getStatus()); + return CompletableFuture.completedFuture(Result.fail(ex)); } catch (RuntimeException ex) { logger.error("unary call with traceId {} problem {}", settings.getTraceId(), ex.getMessage()); - return Async.failedFuture(ex); + return CompletableFuture.completedFuture(Result.error(ex.getMessage(), ex)); } } @@ -131,6 +134,10 @@ public GrpcReadStream readStreamCall( } return new ReadStreamCall<>(call, request, makeMetadataFromSettings(settings), handler); + } catch (UnexpectedResultException ex) { + logger.error("server stream call with traceId {} unexpected status {}", + settings.getTraceId(), ex.getStatus()); + return new EmptyStream<>(ex.getStatus()); } catch (RuntimeException ex) { logger.error("server stream call with traceId {} problem {}", settings.getTraceId(), ex.getMessage()); Issue issue = Issue.of(ex.getMessage(), Issue.Severity.ERROR); @@ -170,6 +177,10 @@ public GrpcReadWriteStream readWriteStreamCall( } return new ReadWriteStreamCall<>(call, makeMetadataFromSettings(settings), getAuthCallOptions(), handler); + } catch (UnexpectedResultException ex) { + logger.error("server bidirectional stream call with traceId {} unexpected status {}", + settings.getTraceId(), ex.getStatus()); + return new EmptyStream<>(ex.getStatus()); } catch (RuntimeException ex) { logger.error("server bidirectional stream call with traceId {} problem {}", settings.getTraceId(), ex.getMessage()); diff --git a/core/src/main/java/tech/ydb/core/impl/YdbDiscovery.java b/core/src/main/java/tech/ydb/core/impl/YdbDiscovery.java index 2ba12eeec..04aa67f0a 100644 --- a/core/src/main/java/tech/ydb/core/impl/YdbDiscovery.java +++ b/core/src/main/java/tech/ydb/core/impl/YdbDiscovery.java @@ -83,7 +83,7 @@ public void stop() { } } - public void waitReady() throws IllegalStateException { + public void waitReady(long millis) throws IllegalStateException { if (isStarted) { return; } @@ -94,7 +94,8 @@ public void waitReady() throws IllegalStateException { return; } - readyObj.wait(discoveryTimeout.toMillis()); + long timeout = millis > 0 ? millis : discoveryTimeout.toMillis(); + readyObj.wait(timeout); } catch (InterruptedException ex) { Thread.currentThread().interrupt(); lastException = new IllegalStateException("Discovery waiting interrupted", ex); diff --git a/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java b/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java index 0a768d6f0..3c57d88a3 100644 --- a/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java +++ b/core/src/main/java/tech/ydb/core/impl/YdbTransportImpl.java @@ -67,7 +67,7 @@ public void start(GrpcTransportBuilder.InitMode mode) { discovery.start(); if (mode == GrpcTransportBuilder.InitMode.SYNC) { - discovery.waitReady(); + discovery.waitReady(-1); } } @@ -82,7 +82,7 @@ public void startAsync(Runnable readyWatcher) { discovery.start(); if (readyWatcher != null) { scheduler.execute(() -> { - discovery.waitReady(); + discovery.waitReady(-1); readyWatcher.run(); }); } @@ -157,7 +157,11 @@ public AuthCallOptions getAuthCallOptions() { protected GrpcChannel getChannel(GrpcRequestSettings settings) { EndpointRecord endpoint = endpointPool.getEndpoint(settings.getPreferredNodeID()); if (endpoint == null) { - discovery.waitReady(); + long timeout = -1; + if (settings.getDeadlineAfter() != 0) { + timeout = settings.getDeadlineAfter() - System.nanoTime(); + } + discovery.waitReady(timeout); endpoint = endpointPool.getEndpoint(settings.getPreferredNodeID()); } return channelPool.getChannel(endpoint); diff --git a/core/src/test/java/tech/ydb/core/impl/YdbDiscoveryTest.java b/core/src/test/java/tech/ydb/core/impl/YdbDiscoveryTest.java index 93c439d4e..0343e4342 100644 --- a/core/src/test/java/tech/ydb/core/impl/YdbDiscoveryTest.java +++ b/core/src/test/java/tech/ydb/core/impl/YdbDiscoveryTest.java @@ -61,7 +61,7 @@ private T checkFutureException(CompletableFuture private CompletableFuture createWaitingFuture(YdbDiscovery discovery) { return CompletableFuture.supplyAsync(() -> { - discovery.waitReady(); + discovery.waitReady(100); return Boolean.TRUE; }); } @@ -87,7 +87,7 @@ public void baseTest() { scheduler.hasTasksCount(1).runNextTask(); verifyDiscoveryCount(1); - discovery.waitReady(); + discovery.waitReady(-1); discovery.stop(); // stop is imdepotent operation @@ -237,8 +237,9 @@ public void failedDiscoveryTest() { scheduler.hasTasksCount(1).runNextTask(); verifyDiscoveryCount(4); - RuntimeException ex5 = checkFutureException(req5, "Discovery failed", RuntimeException.class); - Assert.assertEquals("Test io problem", ex5.getMessage()); + UnexpectedResultException ex5 = checkFutureException(req5, "Discovery failed", UnexpectedResultException.class); + Assert.assertEquals(StatusCode.CLIENT_INTERNAL_ERROR, ex5.getStatus().getCode()); + Assert.assertEquals("Test io problem, code: CLIENT_INTERNAL_ERROR", ex5.getMessage()); discovery.stop(); }