[CONJ-1003] "mariadb:replication:" was always using first replica

mariadb-corporation · Sep 2, 2022 · 398c3a7 · 398c3a7
1 parent 78bc75e
commit 398c3a7
Show file tree

Hide file tree

Showing 5 changed files with 108 additions and 23 deletions.
diff --git 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
@@ -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
@@ -16,7 +16,7 @@ public Optional<HostAddress> getAvailableHost(
         List<HostAddress> hostAddresses,
         ConcurrentMap<HostAddress, Long> 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<HostAddress> getAvailableHost(
         List<HostAddress> hostAddresses,
         ConcurrentMap<HostAddress, Long> denyList,
         boolean primary) {
-      // use in order not blacklisted server
-      List<HostAddress> 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<HostAddress> 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<HostAddress> getAvailableHostRandomOrder(
+      List<HostAddress> hostAddresses, ConcurrentMap<HostAddress, Long> denyList, boolean primary) {
+    // use in order not blacklisted server
+    List<HostAddress> 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
@@ -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
@@ -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