pgyori commented on a change in pull request #4748:
URL: https://github.com/apache/nifi/pull/4748#discussion_r568806370
##########
File path: nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java
##########
@@ -711,6 +716,25 @@ public void dump(final File dumpFile) throws IOException {
makeRequest(DUMP_CMD, null, dumpFile, "thread dump");
}
+ private boolean isNiFiFullyLoaded() throws IOException {
+ final Logger logger = defaultLogger;
+ final Integer port = getCurrentPort(logger);
+ if (port == null) {
+ logger.info("Apache NiFi is not currently running");
Review comment:
In this particular case, I would prefer to leave it like that. There are
many log messages in this class that would need to be extracted as constants,
and the part of the class where the constants are, would be 'littered' with the
crowd of log messages. In this case, I think extracting the messages would
decrease readability.
##########
File path: nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java
##########
@@ -763,6 +773,27 @@ private void makeRequest(final String request, final
String arguments, final Fil
}
}
+ private void sendRequest(Socket socket, Integer port, String request,
String arguments, Logger logger) throws IOException {
+ logger.debug("Connecting to NiFi instance");
+ socket.setSoTimeout(60000);
+ socket.connect(new InetSocketAddress("localhost", port));
+ logger.debug("Established connection to NiFi instance.");
+ socket.setSoTimeout(60000);
+
+ logger.debug("Sending " + request + " Command to port {}", port);
Review comment:
That's right. The fix will be in my next commit.
##########
File path: nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java
##########
@@ -763,6 +773,27 @@ private void makeRequest(final String request, final
String arguments, final Fil
}
}
+ private void sendRequest(Socket socket, Integer port, String request,
String arguments, Logger logger) throws IOException {
+ logger.debug("Connecting to NiFi instance");
+ socket.setSoTimeout(60000);
+ socket.connect(new InetSocketAddress("localhost", port));
+ logger.debug("Established connection to NiFi instance.");
+ socket.setSoTimeout(60000);
+
+ logger.debug("Sending " + request + " Command to port {}", port);
+ final OutputStream socketOut = socket.getOutputStream();
+
+ final Properties nifiProps = loadProperties(logger);
+ final String secretKey = nifiProps.getProperty("secret.key");
Review comment:
No. Each running NiFi instance has 1 secret key. We should not rely on
the instance variable, as the bootstrap process can (and in this wait-for-init
feature it IS) started over and over again 'independently' from the running
NiFi instance, so the value of the secret key needs to be loaded from the
property file.
##########
File path: nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java
##########
@@ -1467,4 +1498,11 @@ public boolean isProcessRunning() {
return Boolean.TRUE.equals(processRunning);
}
}
+
+ private static class NiFiNotRunningException extends Exception {
+ @Override
+ public Throwable fillInStackTrace() {
Review comment:
Yes, thanks!
##########
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-runtime/src/main/java/org/apache/nifi/BootstrapListener.java
##########
@@ -46,6 +46,7 @@
private volatile Listener listener;
private volatile ServerSocket serverSocket;
Review comment:
As far as I know, in the cases above, it actually is the reference that
needs to be volatile since the value of the reference is changed from multiple
threads and we need to make sure that no matter which thread reads the object
behind this reference, it gets to the current object and not an outdated
previous instance.
----------------------------------------------------------------
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]
