From 0c8dee006c2163e46422453e2b0a17a1f4a3f081 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20B=C4=85czkowski?= Date: Wed, 18 Sep 2024 15:25:26 +0200 Subject: [PATCH 1/4] Add option to consider initial contact points during reconnection When control connection tries to reconnect usually it considers only nodes provided by load balancing policy. Usually those do not include what was initially passed to the driver but the recently seen alive nodes. In some setups the IPs can keep changing so it may be useful to have an option to try initial contact points as one of the options during reconnection. Mainly if the contact point is a hostname. This commit adds the option to the `QueryOptions` to control that behaviour and adds necessary logic to `ControlConnection` class. It is disabled by default, meaning that default behaviour remains unchanged. Add org.burningwave tools dependency This dependency has features that allow for easier host resolution mocking. --- driver-core/pom.xml | 7 ++ .../driver/core/ControlConnection.java | 10 +++ .../datastax/driver/core/QueryOptions.java | 22 +++++ .../com/datastax/driver/core/CCMBridge.java | 12 +++ .../core/HostResolutionReconnectionTest.java | 86 +++++++++++++++++++ .../resources/burningwave.static.properties | 4 + pom.xml | 7 ++ 7 files changed, 148 insertions(+) create mode 100644 driver-core/src/test/java/com/datastax/driver/core/HostResolutionReconnectionTest.java create mode 100644 driver-core/src/test/resources/burningwave.static.properties diff --git a/driver-core/pom.xml b/driver-core/pom.xml index f7d727012dd..03b427bd8e6 100644 --- a/driver-core/pom.xml +++ b/driver-core/pom.xml @@ -195,6 +195,13 @@ 1.78.1 + + + org.burningwave + tools + test + + diff --git a/driver-core/src/main/java/com/datastax/driver/core/ControlConnection.java b/driver-core/src/main/java/com/datastax/driver/core/ControlConnection.java index cb2f424df72..7f3c3bde636 100644 --- a/driver-core/src/main/java/com/datastax/driver/core/ControlConnection.java +++ b/driver-core/src/main/java/com/datastax/driver/core/ControlConnection.java @@ -34,6 +34,7 @@ import com.datastax.driver.core.utils.MoreFutures; import com.datastax.driver.core.utils.MoreObjects; import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.Iterators; import com.google.common.util.concurrent.FutureCallback; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.SettableFuture; @@ -160,6 +161,15 @@ protected Connection tryReconnect() throws ConnectionException { if (isShutdown) throw new ConnectionException(null, "Control connection was shut down"); try { + if (cluster + .configuration + .getQueryOptions() + .shouldAddOriginalContactsToReconnectionPlan()) { + List initialContacts = cluster.metadata.getContactPoints(); + Collections.shuffle(initialContacts); + return reconnectInternal( + Iterators.concat(queryPlan(), initialContacts.iterator()), false); + } return reconnectInternal(queryPlan(), false); } catch (NoHostAvailableException e) { throw new ConnectionException(null, e.getMessage()); diff --git a/driver-core/src/main/java/com/datastax/driver/core/QueryOptions.java b/driver-core/src/main/java/com/datastax/driver/core/QueryOptions.java index 9fcd6a437d4..137c56aa747 100644 --- a/driver-core/src/main/java/com/datastax/driver/core/QueryOptions.java +++ b/driver-core/src/main/java/com/datastax/driver/core/QueryOptions.java @@ -72,6 +72,8 @@ public class QueryOptions { private volatile boolean schemaQueriesPaged = true; + private volatile boolean addOriginalContactsToReconnectionPlan = false; + /** * Creates a new {@link QueryOptions} instance using the {@link #DEFAULT_CONSISTENCY_LEVEL}, * {@link #DEFAULT_SERIAL_CONSISTENCY_LEVEL} and {@link #DEFAULT_FETCH_SIZE}. @@ -499,6 +501,26 @@ public int getMaxPendingRefreshNodeRequests() { return maxPendingRefreshNodeRequests; } + /** + * Whether the driver should use original contact points when reconnecting to a control node. In + * practice this forces driver to manually add original contact points to the end of the query + * plan. It is possible that it may introduce duplicates (but under differnet Host class + * instances) in the query plan. If this is set to false it does not mean that original contact + * points will be excluded. + * + *

One use case of this feature is that if the original contact point is defined by hostname + * and its IP address changes then setting this to {@code true} allows trying reconnecting to the + * new IP if all connection was lost. + */ + public QueryOptions setAddOriginalContactsToReconnectionPlan(boolean enabled) { + this.addOriginalContactsToReconnectionPlan = enabled; + return this; + } + + public boolean shouldAddOriginalContactsToReconnectionPlan() { + return this.addOriginalContactsToReconnectionPlan; + } + @Override public boolean equals(Object that) { if (that == null || !(that instanceof QueryOptions)) { diff --git a/driver-core/src/test/java/com/datastax/driver/core/CCMBridge.java b/driver-core/src/test/java/com/datastax/driver/core/CCMBridge.java index 4c9ba61fc57..dc9b807c023 100644 --- a/driver-core/src/test/java/com/datastax/driver/core/CCMBridge.java +++ b/driver-core/src/test/java/com/datastax/driver/core/CCMBridge.java @@ -949,6 +949,7 @@ public static class Builder { private static final Pattern RANDOM_PORT_PATTERN = Pattern.compile(RANDOM_PORT); private String ipPrefix = TestUtils.IP_PREFIX; + private String providedClusterName = null; int[] nodes = {1}; private int[] jmxPorts = {}; private boolean start = true; @@ -991,6 +992,15 @@ public Builder withSniProxy() { return this; } + /** + * Builder takes care of naming and numbering clusters on its own. Use if you really need a + * specific name + */ + public Builder withClusterName(String clusterName) { + this.providedClusterName = clusterName; + return this; + } + /** Enables SSL encryption. */ public Builder withSSL() { cassandraConfiguration.put("client_encryption_options.enabled", "true"); @@ -1115,6 +1125,8 @@ public CCMBridge build() { // be careful NOT to alter internal state (hashCode/equals) during build! String clusterName = TestUtils.generateIdentifier("ccm_"); + if (providedClusterName != null) clusterName = providedClusterName; + VersionNumber dseVersion; VersionNumber cassandraVersion; boolean versionConfigured = this.version != null; diff --git a/driver-core/src/test/java/com/datastax/driver/core/HostResolutionReconnectionTest.java b/driver-core/src/test/java/com/datastax/driver/core/HostResolutionReconnectionTest.java new file mode 100644 index 00000000000..7e0e57ce0a3 --- /dev/null +++ b/driver-core/src/test/java/com/datastax/driver/core/HostResolutionReconnectionTest.java @@ -0,0 +1,86 @@ +package com.datastax.driver.core; + +import java.net.InetSocketAddress; +import java.util.LinkedHashMap; +import java.util.Map; +import org.burningwave.tools.net.DefaultHostResolver; +import org.burningwave.tools.net.HostResolutionRequestInterceptor; +import org.burningwave.tools.net.MappedHostResolver; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.testng.annotations.Test; + +public class HostResolutionReconnectionTest { + + private static final Logger logger = + LoggerFactory.getLogger(HostResolutionReconnectionTest.class); + + @Test(groups = "isolated") + public void should_reconnect_to_different_cluster() { + // Configure host resolution + Map hostAliasesA = new LinkedHashMap<>(); + hostAliasesA.put("control.reconnect.test", "127.1.1.1"); + HostResolutionRequestInterceptor.INSTANCE.install( + new MappedHostResolver(hostAliasesA), DefaultHostResolver.INSTANCE); + + Cluster cluster = null; + Session session = null; + CCMBridge bridgeA = null; + try { + bridgeA = + CCMBridge.builder() + .withNodes(1) + .withIpPrefix("127.1.1.") + .withBinaryPort(9042) + .withClusterName("same_name") + .build(); + bridgeA.start(); + + cluster = + Cluster.builder() + .addContactPointsWithPorts( + InetSocketAddress.createUnresolved("control.reconnect.test", 9042)) + .withPort(9042) + .withoutAdvancedShardAwareness() + .withQueryOptions(new QueryOptions().setAddOriginalContactsToReconnectionPlan(true)) + .build(); + session = cluster.connect(); + + ResultSet rs = session.execute("select * from system.local"); + Row row = rs.one(); + String address = row.getInet("broadcast_address").toString(); + logger.info("Queried node has broadcast_address: {}}", address); + System.out.flush(); + } finally { + assert bridgeA != null; + bridgeA.close(); + } + + CCMBridge bridgeB = null; + // Overwrite host resolution + Map hostAliasesB = new LinkedHashMap<>(); + hostAliasesB.put("control.reconnect.test", "127.2.2.1"); + HostResolutionRequestInterceptor.INSTANCE.install( + new MappedHostResolver(hostAliasesB), DefaultHostResolver.INSTANCE); + try { + bridgeB = + CCMBridge.builder() + .withNodes(1) + .withIpPrefix("127.2.2.") + .withBinaryPort(9042) + .withClusterName("same_name") + .build(); + bridgeB.start(); + Thread.sleep(1000 * 92); + ResultSet rs = session.execute("select * from system.local"); + Row row = rs.one(); + String address = row.getInet("broadcast_address").toString(); + logger.info("Queried node has broadcast_address: {}}", address); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } finally { + assert bridgeB != null; + bridgeB.close(); + } + } +} diff --git a/driver-core/src/test/resources/burningwave.static.properties b/driver-core/src/test/resources/burningwave.static.properties new file mode 100644 index 00000000000..7108b42c0fb --- /dev/null +++ b/driver-core/src/test/resources/burningwave.static.properties @@ -0,0 +1,4 @@ +managed-logger.repository=autodetect +managed-logger.repository.enabled=false +banner.hide=true +priority-of-this-configuration=1000 \ No newline at end of file diff --git a/pom.xml b/pom.xml index 51d15164fde..102cb5f6894 100644 --- a/pom.xml +++ b/pom.xml @@ -86,6 +86,7 @@ 1.1.2 1.2.13 3.0.8 + 0.26.2 127.0.1. unit @@ -398,6 +399,12 @@ ${groovy.version} + + org.burningwave + tools + ${burningwave.tools.version} + + From 38215397dbec80aa0db1e440383cc41d8a3370b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20B=C4=85czkowski?= Date: Mon, 23 Sep 2024 14:36:05 +0200 Subject: [PATCH 2/4] temporary logging settings --- driver-core/src/test/resources/log4j.properties | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/driver-core/src/test/resources/log4j.properties b/driver-core/src/test/resources/log4j.properties index 67195750ee6..a2d3e400775 100644 --- a/driver-core/src/test/resources/log4j.properties +++ b/driver-core/src/test/resources/log4j.properties @@ -31,9 +31,9 @@ log4j.logger.spray.can=ERROR #log4j.logger.org.scassandra.http.client=ERROR # These loggers can be quite verbose -log4j.logger.com.datastax.driver.core=INFO -log4j.logger.com.datastax.driver.core.Cluster=ERROR -log4j.logger.com.datastax.driver.core.policies.DCAwareRoundRobinPolicy=ERROR +log4j.logger.com.datastax.driver.core=DEBUG +#log4j.logger.com.datastax.driver.core.Cluster=ERROR +#log4j.logger.com.datastax.driver.core.policies.DCAwareRoundRobinPolicy=ERROR # Useful loggers when debugging core functionality #log4j.logger.com.datastax.driver.core.ControlConnection=ERROR @@ -44,7 +44,7 @@ log4j.logger.com.datastax.driver.core.policies.DCAwareRoundRobinPolicy=ERROR # Adjust log levels for CCM tests #log4j.logger.com.datastax.driver.core.CCMTestsSupport=DEBUG #log4j.logger.com.datastax.driver.core.CCMCache=DEBUG -#log4j.logger.com.datastax.driver.core.CCMBridge=DEBUG +log4j.logger.com.datastax.driver.core.CCMBridge=DEBUG # A1 is set to be a ConsoleAppender. log4j.appender.A1=org.apache.log4j.ConsoleAppender From 17fed64fb8270d8a0aee0858895f3c0a4e3b3bf9 Mon Sep 17 00:00:00 2001 From: Dmitry Kropachev Date: Tue, 24 Sep 2024 11:39:58 -0400 Subject: [PATCH 3/4] 1 --- .github/workflows/tests@v1.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests@v1.yml b/.github/workflows/tests@v1.yml index 4bdd37794fd..4947cfcbde5 100644 --- a/.github/workflows/tests@v1.yml +++ b/.github/workflows/tests@v1.yml @@ -176,6 +176,7 @@ jobs: strategy: matrix: + java-version: [8, 11] scylla-version: ${{ fromJson(needs.setup-integration-tests.outputs.scylla-integration-tests-versions) }} fail-fast: false @@ -183,10 +184,10 @@ jobs: - name: Checkout source uses: actions/checkout@v4 - - name: Set up JDK 8 + - name: Set up JDK ${{ matrix.java-version }} uses: actions/setup-java@v4 with: - java-version: '8' + java-version: ${{ matrix.java-version }} distribution: 'adopt' - name: Setup Python 3 From db0d0fea729cf637c30af470f0ffe02fc9aaa0b3 Mon Sep 17 00:00:00 2001 From: Dmitry Kropachev Date: Tue, 24 Sep 2024 13:26:01 -0400 Subject: [PATCH 4/4] 1 --- .github/workflows/tests@v1.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/tests@v1.yml b/.github/workflows/tests@v1.yml index 4947cfcbde5..4f234d5dae2 100644 --- a/.github/workflows/tests@v1.yml +++ b/.github/workflows/tests@v1.yml @@ -210,19 +210,19 @@ jobs: if: failure() run: | shopt -s globstar - mkdir scylla-${{ matrix.scylla-version }} - cp --parents ./**/target/*-reports/*.xml scylla-${{ matrix.scylla-version }}/ + mkdir scylla-${{ matrix.java-version }}-${{ matrix.scylla-version }} + cp --parents ./**/target/*-reports/*.xml scylla-${{ matrix.java-version }}-${{ matrix.scylla-version }}/ - name: Upload test results uses: actions/upload-artifact@v4 if: failure() with: - name: test-results-${{ matrix.scylla-version }} + name: test-results-${{ matrix.java-version }}-${{ matrix.scylla-version }} path: "*/**/target/*-reports/*.xml" - name: Upload CCM logs uses: actions/upload-artifact@v4 if: failure() with: - name: ccm-logs-scylla-${{ matrix.scylla-version }} + name: ccm-logs-scylla-${{ matrix.java-version }}-${{ matrix.scylla-version }} path: /tmp/*-0/ccm*/node*/logs/*