[
https://issues.apache.org/jira/browse/HADOOP-17665?focusedWorklogId=593449&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-593449
]
ASF GitHub Bot logged work on HADOOP-17665:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 07/May/21 18:08
Start Date: 07/May/21 18:08
Worklog Time Spent: 10m
Work Description: saintstack commented on a change in pull request #2979:
URL: https://github.com/apache/hadoop/pull/2979#discussion_r628409577
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java
##########
@@ -601,22 +603,30 @@ private ServerConnector createHttpsChannelConnector(
private Timer makeConfigurationChangeMonitor(long reloadInterval,
SslContextFactory.Server sslContextFactory) {
java.util.Timer timer = new
java.util.Timer(FileBasedKeyStoresFactory.SSL_MONITORING_THREAD_NAME, true);
+ ArrayList<Path> locations = new ArrayList<Path>();
Review comment:
Nit
Could you write:
List<Path> locations = new ArrayList<>();
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ssl/FileMonitoringTimerTask.java
##########
@@ -42,34 +45,59 @@
static final String PROCESS_ERROR_MESSAGE =
"Could not process file change : ";
- final private Path filePath;
+ final private List<Path> filePaths;
final private Consumer<Path> onFileChange;
final Consumer<Throwable> onChangeFailure;
- private long lastProcessed;
+ private List<Long> lastProcessed;
/**
- * Create file monitoring task to be scheduled using a standard Java {@link
java.util.Timer}
- * instance.
+ * See {@link #FileMonitoringTimerTask(List, Consumer, Consumer)}.
*
- * @param filePath The path to the file to monitor.
- * @param onFileChange The function to call when the file has changed.
- * @param onChangeFailure The function to call when an exception is thrown
during the
- * file change processing.
+ * @param filePath The file to monitor.
+ * @param onFileChange What to do when the file changes.
+ * @param onChangeFailure What to do when <code>onFileChange</code>
+ * throws an exception.
*/
public FileMonitoringTimerTask(Path filePath, Consumer<Path> onFileChange,
- Consumer<Throwable> onChangeFailure) {
- Preconditions.checkNotNull(filePath, "path to monitor disk file is not
set");
- Preconditions.checkNotNull(onFileChange, "action to monitor disk file is
not set");
+ Consumer<Throwable> onChangeFailure) {
+ this(Collections.singletonList(filePath), onFileChange, onChangeFailure);
+ }
- this.filePath = filePath;
- this.lastProcessed = filePath.toFile().lastModified();
+ /**
+ * Create file monitoring task to be scheduled using a standard
+ * Java {@link java.util.Timer} instance.
+ *
+ * @param filePaths The path to the file to monitor.
+ * @param onFileChange The function to call when the file has changed.
+ * @param onChangeFailure The function to call when an exception is
+ * thrown during the file change processing.
+ */
+ public FileMonitoringTimerTask(List<Path> filePaths,
+ Consumer<Path> onFileChange,
+ Consumer<Throwable> onChangeFailure) {
+ Preconditions.checkNotNull(filePaths,
+ "path to monitor disk file is not set");
+ Preconditions.checkNotNull(onFileChange,
+ "action to monitor disk file is not set");
+
+ this.filePaths = new ArrayList<Path>(filePaths);
Review comment:
nit
new ArrayList<> ? instead of new ArrayList<Path> and new ArrayList<Long>...
not important.
##########
File path: hadoop-yarn-project/hadoop-yarn/pom.xml
##########
@@ -246,4 +246,5 @@
<module>hadoop-yarn-ui</module>
<module>hadoop-yarn-csi</module>
</modules>
+ <!-- -->
Review comment:
nit: Next time, see if you can make a change that doesn't leave behind
detritus!
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java
##########
@@ -601,22 +603,30 @@ private ServerConnector createHttpsChannelConnector(
private Timer makeConfigurationChangeMonitor(long reloadInterval,
SslContextFactory.Server sslContextFactory) {
java.util.Timer timer = new
java.util.Timer(FileBasedKeyStoresFactory.SSL_MONITORING_THREAD_NAME, true);
+ ArrayList<Path> locations = new ArrayList<Path>();
+ if (keyStore != null) {
+ locations.add(Paths.get(keyStore));
+ }
+ if (trustStore != null) {
+ locations.add(Paths.get(trustStore));
+ }
//
// The Jetty SSLContextFactory provides a 'reload' method which will
reload both
// truststore and keystore certificates.
//
timer.schedule(new FileMonitoringTimerTask(
- Paths.get(keyStore),
- path -> {
- LOG.info("Reloading certificates from store keystore " +
keyStore);
- try {
- sslContextFactory.reload(factory -> { });
- } catch (Exception ex) {
- LOG.error("Failed to reload SSL keystore certificates", ex);
- }
- },null),
- reloadInterval,
- reloadInterval
+ locations,
+ path -> {
+ LOG.info("Reloading keystore and truststore certificates.");
+ try {
+ sslContextFactory.reload(factory -> { });
+ } catch (Exception ex) {
+ LOG.error("Failed to reload SSL keystore " +
+ "and truststore certificates", ex);
+ }
+ },null),
Review comment:
I was going to make comments on style in here but you are just moving
the code block... not changing it ... which is generally the better way to go:
i.e. don't change code not related to your PR concern.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 593449)
Remaining Estimate: 0h
Time Spent: 10m
> Ignore missing keystore configuration in reloading mechanism
> -------------------------------------------------------------
>
> Key: HADOOP-17665
> URL: https://issues.apache.org/jira/browse/HADOOP-17665
> Project: Hadoop Common
> Issue Type: Sub-task
> Reporter: Borislav Iordanov
> Assignee: Borislav Iordanov
> Priority: Major
> Time Spent: 10m
> Remaining Estimate: 0h
>
> When there is no configuration of keystore/truststore location, the reload
> mechanism should be disabled.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]