From 398c3a7d6aa178d49e0408e54b1876252fabb3b0 Mon Sep 17 00:00:00 2001 From: diego Dupin Date: Fri, 2 Sep 2022 11:11:02 +0200 Subject: [PATCH] [CONJ-1003] "mariadb:replication:" was always using first replica --- .travis.yml | 3 ++ .../java/org/mariadb/jdbc/Connection.java | 4 ++ .../java/org/mariadb/jdbc/export/HaMode.java | 40 +++++++++++------ .../jdbc/integration/MultiHostTest.java | 45 +++++++++++++++++++ .../jdbc/unit/util/constant/HaModeTest.java | 39 +++++++++++----- 5 files changed, 108 insertions(+), 23 deletions(-) diff --git a/.travis.yml b/.travis.yml index 9080da674..eed52ea4d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,6 +8,9 @@ jdk: openjdk11 addons: hosts: - mariadb.example.com + - mariadb1.example.com + - mariadb2.example.com + - mariadb3.example.com before_install: - git clone https://github.com/mariadb-corporation/connector-test-machine.git diff --git a/src/main/java/org/mariadb/jdbc/Connection.java b/src/main/java/org/mariadb/jdbc/Connection.java index 8e9848968..b6dbf0809 100644 --- a/src/main/java/org/mariadb/jdbc/Connection.java +++ b/src/main/java/org/mariadb/jdbc/Connection.java @@ -917,4 +917,8 @@ public void fireStatementClosed(PreparedStatement prep) { protected ExceptionFactory getExceptionFactory() { return exceptionFactory; } + + public String __test_host() { + return this.client.getHostAddress().toString(); + } } diff --git a/src/main/java/org/mariadb/jdbc/export/HaMode.java b/src/main/java/org/mariadb/jdbc/export/HaMode.java index e4af9fe1b..25f0de9f8 100644 --- a/src/main/java/org/mariadb/jdbc/export/HaMode.java +++ b/src/main/java/org/mariadb/jdbc/export/HaMode.java @@ -16,7 +16,7 @@ public Optional getAvailableHost( List hostAddresses, ConcurrentMap denyList, boolean primary) { - return HaMode.getAvailableHostInOrder(hostAddresses, denyList, primary); + return HaMode.getAvailableHostRandomOrder(hostAddresses, denyList, primary); } }, /** sequential: driver will always connect according to connection string order */ @@ -34,18 +34,7 @@ public Optional getAvailableHost( List hostAddresses, ConcurrentMap denyList, boolean primary) { - // use in order not blacklisted server - List loopAddress = new ArrayList<>(hostAddresses); - - // ensure denied server have not reached denied timeout - denyList.entrySet().stream() - .filter(e -> e.getValue() < System.currentTimeMillis()) - .forEach(e -> denyList.remove(e.getKey())); - - loopAddress.removeAll(denyList.keySet()); - - Collections.shuffle(loopAddress); - return loopAddress.stream().filter(e -> e.primary == primary).findFirst(); + return HaMode.getAvailableHostRandomOrder(hostAddresses, denyList, primary); } }, /** no ha-mode. Connect to first host only */ @@ -106,6 +95,31 @@ public static Optional getAvailableHostInOrder( return Optional.empty(); } + /** + * return hosts of corresponding type (primary or not) without blacklisted hosts. hosts in + * blacklist reaching blacklist timeout will be present. Order is random. + * + * @param hostAddresses hosts + * @param denyList blacklist + * @param primary returns primary hosts or replica + * @return list without denied hosts + */ + public static Optional getAvailableHostRandomOrder( + List hostAddresses, ConcurrentMap denyList, boolean primary) { + // use in order not blacklisted server + List loopAddress = new ArrayList<>(hostAddresses); + + // ensure denied server have not reached denied timeout + denyList.entrySet().stream() + .filter(e -> e.getValue() < System.currentTimeMillis()) + .forEach(e -> denyList.remove(e.getKey())); + + loopAddress.removeAll(denyList.keySet()); + + Collections.shuffle(loopAddress); + return loopAddress.stream().filter(e -> e.primary == primary).findFirst(); + } + /** * List of hosts without blacklist entries, ordered according to HA mode * diff --git a/src/test/java/org/mariadb/jdbc/integration/MultiHostTest.java b/src/test/java/org/mariadb/jdbc/integration/MultiHostTest.java index 18660ca36..0a35a2f2e 100644 --- a/src/test/java/org/mariadb/jdbc/integration/MultiHostTest.java +++ b/src/test/java/org/mariadb/jdbc/integration/MultiHostTest.java @@ -45,6 +45,51 @@ public void failoverReadonlyToMaster() throws Exception { } } + @Test + public void ensureReadOnlyOnReplica() throws Exception { + // mariadb1.example.com, mariadb2.example.com and mariadb3.example.com DNS alias must be defined + Configuration conf = Configuration.parse(mDefUrl); + HostAddress hostAddress = conf.addresses().get(0); + String url = + mDefUrl.replaceAll( + "//([^/]*)/", + String.format( + "//mariadb1.example.com:%s,mariadb2.example.com:%s,mariadb3.example.com:%s/", + hostAddress.port, hostAddress.port, hostAddress.port)); + url = url.replaceAll("jdbc:mariadb:", "jdbc:mariadb:replication:"); + if (conf.sslMode() == SslMode.VERIFY_FULL) { + url = url.replaceAll("sslMode=verify-full", "sslMode=verify-ca"); + } + try { + int replica1 = 0; + int replica2 = 0; + for (int i = 0; i < 100; i++) { + try (Connection con = + (Connection) + DriverManager.getConnection( + url + "&waitReconnectTimeout=30&deniedListTimeout=300")) { + assertTrue(con.__test_host().contains("primary")); + con.setReadOnly(true); + assertTrue(con.__test_host().contains("replica")); + if (con.__test_host().contains("mariadb2")) { + replica1++; + } + if (con.__test_host().contains("mariadb3")) { + replica2++; + } + } + } + + assertTrue( + replica1 > 35, "value replica1/replicat2 aren't right : " + replica1 + "/" + replica2); + assertTrue( + replica2 > 35, "value replica1/replicat2 aren't right : " + replica1 + "/" + replica2); + } catch (SQLNonTransientConnectionException e) { + fail( + "mariadb1.example.com, mariadb2.example.com and mariadb3.example.com DNS alias must be defined"); + } + } + @Test public void readOnly() throws SQLException { Assumptions.assumeTrue( diff --git a/src/test/java/org/mariadb/jdbc/unit/util/constant/HaModeTest.java b/src/test/java/org/mariadb/jdbc/unit/util/constant/HaModeTest.java index 67a6b8e57..f73ddaf74 100644 --- a/src/test/java/org/mariadb/jdbc/unit/util/constant/HaModeTest.java +++ b/src/test/java/org/mariadb/jdbc/unit/util/constant/HaModeTest.java @@ -26,6 +26,8 @@ import org.mariadb.jdbc.util.log.Loggers; import org.mariadb.jdbc.util.options.OptionAliases; +import static org.junit.jupiter.api.Assertions.assertTrue; + public class HaModeTest { @Test public void instantiateStaticOnlyClass() { @@ -67,19 +69,36 @@ public void replicationEndOfBlacklistTest() { Assertions.assertTrue(res.isPresent()); Assertions.assertEquals(host1, res.get()); - res = HaMode.REPLICATION.getAvailableHost(available, denyList, false); - Assertions.assertTrue(res.isPresent()); - Assertions.assertEquals(host2, res.get()); + int replica1 = 0; + int replica2 = 0; + for (int i = 0; i < 1000; i++) { + res = HaMode.REPLICATION.getAvailableHost(available, denyList, false); + Assertions.assertTrue(res.isPresent()); + if (host2.equals(res.get())) replica1++; + if (host3.equals(res.get())) replica2++; + } + assertTrue( + replica1 > 350 && replica2 > 350, "bad distribution :" + replica1 + "/" + replica2); + + replica1 = 0; + replica2 = 0; denyList.putIfAbsent(host2, System.currentTimeMillis() - 10); - res = HaMode.REPLICATION.getAvailableHost(available, denyList, false); - Assertions.assertTrue(res.isPresent()); - Assertions.assertEquals(host2, res.get()); + for (int i = 0; i < 1000; i++) { + res = HaMode.REPLICATION.getAvailableHost(available, denyList, false); + Assertions.assertTrue(res.isPresent()); + if (host2.equals(res.get())) replica1++; + if (host3.equals(res.get())) replica2++; + } + assertTrue( + replica1 > 350 && replica2 > 350, "bad distribution :" + replica1 + "/" + replica2); - denyList.putIfAbsent(host2, System.currentTimeMillis() + 1000); - res = HaMode.REPLICATION.getAvailableHost(available, denyList, false); - Assertions.assertTrue(res.isPresent()); - Assertions.assertEquals(host3, res.get()); + for (int i = 0; i < 1000; i++) { + denyList.putIfAbsent(host2, System.currentTimeMillis() + 1000); + res = HaMode.REPLICATION.getAvailableHost(available, denyList, false); + Assertions.assertTrue(res.isPresent()); + Assertions.assertEquals(host3, res.get()); + } } @Test