PR feedback: avoid extra exception wrapping, provide thread naming, improve error messages, etc.

Author: aratno

core/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.java

@@ -255,12 +255,6 @@ public enum DefaultDriverOption implements DriverOption {
    * <p>Value-type: {@link String}
    */
   SSL_KEYSTORE_PASSWORD("advanced.ssl-engine-factory.keystore-password"),
-  /**
-   * The duration between attempts to reload the keystore.
-   *
-   * <p>Value-type: {@link java.time.Duration}
-   */
-  SSL_KEYSTORE_RELOAD_INTERVAL("advanced.ssl-engine-factory.keystore-reload-interval"),
   /**
    * The location of the truststore file.
    *
@@ -982,6 +976,12 @@ public enum DefaultDriverOption implements DriverOption {
    * <p>Value-type: boolean
    */
   METRICS_GENERATE_AGGREGABLE_HISTOGRAMS("advanced.metrics.histograms.generate-aggregable"),
+  /**
+   * The duration between attempts to reload the keystore.
+   *
+   * <p>Value-type: {@link java.time.Duration}
+   */
+  SSL_KEYSTORE_RELOAD_INTERVAL("advanced.ssl-engine-factory.keystore-reload-interval"),
   ;
 
   private final String path;

core/src/main/java/com/datastax/oss/driver/internal/core/ssl/DefaultSslEngineFactory.java

@@ -150,8 +150,8 @@ protected SSLContext buildContext(DriverExecutionProfile config) throws Exceptio
     }
   }
 
-  private ReloadingKeyManagerFactory buildReloadingKeyManagerFactory(
-      DriverExecutionProfile config) {
+  private ReloadingKeyManagerFactory buildReloadingKeyManagerFactory(DriverExecutionProfile config)
+      throws Exception {
     Path keystorePath = Paths.get(config.getString(DefaultDriverOption.SSL_KEYSTORE_PATH));
     String password =
         config.isDefined(DefaultDriverOption.SSL_KEYSTORE_PASSWORD)

core/src/main/java/com/datastax/oss/driver/internal/core/ssl/ReloadingKeyManagerFactory.java

@@ -73,26 +73,17 @@ public class ReloadingKeyManagerFactory extends KeyManagerFactory implements Aut
    * @return
    */
   public static ReloadingKeyManagerFactory create(
-      Path keystorePath, String keystorePassword, Duration reloadInterval) {
-    KeyManagerFactory kmf;
-    try {
-      kmf = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
-    } catch (NoSuchAlgorithmException e) {
-      throw new RuntimeException(e);
-    }
+      Path keystorePath, String keystorePassword, Duration reloadInterval)
+      throws UnrecoverableKeyException, KeyStoreException, NoSuchAlgorithmException,
+          CertificateException, IOException {
+    KeyManagerFactory kmf = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
 
     KeyStore ks;
     try (InputStream ksf = Files.newInputStream(keystorePath)) {
       ks = KeyStore.getInstance(KEYSTORE_TYPE);
       ks.load(ksf, keystorePassword.toCharArray());
-    } catch (IOException | CertificateException | KeyStoreException | NoSuchAlgorithmException e) {
-      throw new RuntimeException(e);
-    }
-    try {
-      kmf.init(ks, keystorePassword.toCharArray());
-    } catch (KeyStoreException | NoSuchAlgorithmException | UnrecoverableKeyException e) {
-      throw new RuntimeException(e);
     }
+    kmf.init(ks, keystorePassword.toCharArray());
 
     ReloadingKeyManagerFactory reloadingKeyManagerFactory = new ReloadingKeyManagerFactory(kmf);
     reloadingKeyManagerFactory.start(keystorePath, keystorePassword, reloadInterval);
@@ -115,32 +106,37 @@ private ReloadingKeyManagerFactory(Spi spi, Provider provider, String algorithm)
   private void start(Path keystorePath, String keystorePassword, Duration reloadInterval) {
     this.keystorePath = keystorePath;
     this.keystorePassword = keystorePassword;
-    this.executor =
-        Executors.newScheduledThreadPool(
-            1,
-            runnable -> {
-              Thread t = Executors.defaultThreadFactory().newThread(runnable);
-              t.setDaemon(true);
-              return t;
-            });
 
     // Ensure that reload is called once synchronously, to make sure the file exists etc.
     reload();
 
-    if (!reloadInterval.isZero())
+    if (!reloadInterval.isZero()) {
+      this.executor =
+          Executors.newScheduledThreadPool(
+              1,
+              runnable -> {
+                Thread t = Executors.defaultThreadFactory().newThread(runnable);
+                t.setName(String.format("%s-%%d", this.getClass().getSimpleName()));
+                t.setDaemon(true);
+                return t;
+              });
       this.executor.scheduleWithFixedDelay(
           this::reload,
           reloadInterval.toMillis(),
           reloadInterval.toMillis(),
           TimeUnit.MILLISECONDS);
+    }
   }
 
   @VisibleForTesting
   void reload() {
     try {
       reload0();
     } catch (Exception e) {
-      logger.warn("Failed to reload", e);
+      String msg =
+          "Failed to reload KeyStore. If this continues to happen, your client may use stale identity"
+              + "certificates and fail to re-establish connections to Cassandra hosts.";
+      logger.warn(msg, e);
     }
   }