itachiunited commented on code in PR #14535:
URL: https://github.com/apache/pinot/pull/14535#discussion_r1857464765
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/tls/RenewableTlsUtils.java:
##########
@@ -286,4 +321,4 @@ static void registerFile(WatchService watchService,
Map<WatchKey, Set<Path>> key
keyPathMap.computeIfAbsent(key, k -> new HashSet<>());
keyPathMap.get(key).add(path.getFileName());
}
-}
+}
Review Comment:
Done
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/tls/RenewableTlsUtils.java:
##########
@@ -243,38 +264,52 @@ static void
reloadSslFactoryWhenFileStoreChanges(SSLFactory baseSslFactory,
LOGGER.info("Detected change in file: {}, try to renew SSLFactory {}
"
+ "(built from key store {} and truststore {})",
changedFile, baseSslFactory, keyStorePath, trustStorePath);
- try {
- // Need to retry a few times because when one file (key store or
trust store) is updated, the other file
- // (trust store or key store) may not have been fully written yet,
so we need to wait a bit and retry.
-
RetryPolicies.fixedDelayRetryPolicy(maxSslFactoryReloadingAttempts,
sslFactoryReloadingRetryDelayMs)
- .attempt(() -> {
- try {
- SSLFactory updatedSslFactory =
- createSSLFactory(keyStoreType, keyStorePath,
keyStorePassword, trustStoreType, trustStorePath,
- trustStorePassword, sslContextProtocol,
secureRandom, false, insecureModeSupplier.get());
- SSLFactoryUtils.reload(baseSslFactory, updatedSslFactory);
- LOGGER.info("Successfully renewed SSLFactory {} (built
from key store {} and truststore {}) on file"
- + " {} changes", baseSslFactory, keyStorePath,
trustStorePath, changedFile);
- return true;
- } catch (Exception e) {
- LOGGER.info(
- "Encountered issues when renewing SSLFactory {} (built
from key store {} and truststore {}) on "
- + "file {} changes", baseSslFactory, keyStorePath,
trustStorePath, changedFile, e);
- return false;
- }
- });
- } catch (Exception e) {
- LOGGER.error(
- "Failed to renew SSLFactory {} (built from key store {} and
truststore {}) on file {} changes after {} "
- + "retries", baseSslFactory, keyStorePath, trustStorePath,
changedFile,
- maxSslFactoryReloadingAttempts, e);
- }
+
+ reloadSslFactory(baseSslFactory, keyStoreType, keyStorePath,
keyStorePassword, trustStoreType, trustStorePath, trustStorePassword,
sslContextProtocol, secureRandom, insecureModeSupplier);
}
}
key.reset();
}
}
+ @VisibleForTesting
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
