You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@nifi.apache.org by apiri <gi...@git.apache.org> on 2016/04/15 20:29:22 UTC
[GitHub] nifi-minifi pull request: MINIFI-14 Incorporating Listener/Notifie...
GitHub user apiri opened a pull request:
https://github.com/apache/nifi-minifi/pull/10
MINIFI-14 Incorporating Listener/Notifier Logic
Incorporating Listener/Notifier logic to RunMiNiFi and performing some refactoring of how the FileChangeNotifier is handled.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apiri/nifi-minifi MINIFI-14
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi-minifi/pull/10.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #10
----
commit e25682a5b61cd354c06e47a2d18ee5cc6d3935bc
Author: Aldrin Piri <al...@apache.org>
Date: 2016-04-13T20:43:31Z
MINIFI-14 Incorporating Listener/Notifier Logic to RunMiNiFi and performing some refactoring of how the FileChangeNotifier is handled.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi-minifi pull request: MINIFI-14 Incorporating Listener/Notifie...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/nifi-minifi/pull/10
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi-minifi pull request: MINIFI-14 Incorporating Listener/Notifie...
Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/10#discussion_r59985963
--- Diff: minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-resources/src/main/resources/conf/bootstrap.conf ---
@@ -28,6 +28,16 @@ conf.dir=./conf
# How long to wait after telling MiNiFi to shutdown before explicitly killing the Process
graceful.shutdown.seconds=20
+# The location for the configuration file
+nifi.minifi.config=./conf/config.yml
+
+# Notifiers to use for the associated agent, comma separated list of class names
+#nifi.minifi.notifier.components=org.apache.nifi.minifi.bootstrap.configuration.FileChangeNotifier
+
+# File change notifier configuration for path of the file to monitor and the associated polling period
+#nifi.minifi.notifier.file.config.path=
--- End diff --
Yep, we can adjust. It's just a matter of how we convey to the listener that there is not an input stream/file such that the listener shouldn't save. Let me refactor a bit and push another commit out.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi-minifi pull request: MINIFI-14 Incorporating Listener/Notifie...
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/10#discussion_r59949864
--- Diff: minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-resources/src/main/resources/conf/bootstrap.conf ---
@@ -28,6 +28,16 @@ conf.dir=./conf
# How long to wait after telling MiNiFi to shutdown before explicitly killing the Process
graceful.shutdown.seconds=20
+# The location for the configuration file
+nifi.minifi.config=./conf/config.yml
+
+# Notifiers to use for the associated agent, comma separated list of class names
+#nifi.minifi.notifier.components=org.apache.nifi.minifi.bootstrap.configuration.FileChangeNotifier
+
+# File change notifier configuration for path of the file to monitor and the associated polling period
+#nifi.minifi.notifier.file.config.path=
--- End diff --
This should have a sample value matching the default config location (`./conf/config.yml`)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi-minifi pull request: MINIFI-14 Incorporating Listener/Notifie...
Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/10#discussion_r60062452
--- Diff: minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-resources/src/main/resources/conf/bootstrap.conf ---
@@ -28,6 +28,16 @@ conf.dir=./conf
# How long to wait after telling MiNiFi to shutdown before explicitly killing the Process
graceful.shutdown.seconds=20
+# The location for the configuration file
+nifi.minifi.config=./conf/config.yml
+
+# Notifiers to use for the associated agent, comma separated list of class names
+#nifi.minifi.notifier.components=org.apache.nifi.minifi.bootstrap.configuration.FileChangeNotifier
+
+# File change notifier configuration for path of the file to monitor and the associated polling period
+#nifi.minifi.notifier.file.config.path=
--- End diff --
This has turned out to be not quite as straightforward as hoped and think for now will punt until we see what needs there are. I did create a new issue to track this, however: https://issues.apache.org/jira/browse/MINIFI-16
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi-minifi pull request: MINIFI-14 Incorporating Listener/Notifie...
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/10#discussion_r59978081
--- Diff: minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-resources/src/main/resources/conf/bootstrap.conf ---
@@ -28,6 +28,16 @@ conf.dir=./conf
# How long to wait after telling MiNiFi to shutdown before explicitly killing the Process
graceful.shutdown.seconds=20
+# The location for the configuration file
+nifi.minifi.config=./conf/config.yml
+
+# Notifiers to use for the associated agent, comma separated list of class names
+#nifi.minifi.notifier.components=org.apache.nifi.minifi.bootstrap.configuration.FileChangeNotifier
+
+# File change notifier configuration for path of the file to monitor and the associated polling period
+#nifi.minifi.notifier.file.config.path=
--- End diff --
I agree that there may be use-cases where there will be an separate directory to watch but as a user that has access to the MINIFI directory directly, my natural thinking is to modify the file itself. If it's not too much trouble to optimize now I would lean towards that though if it would be a lot of needless work we can just properly document the limitations.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi-minifi pull request: MINIFI-14 Incorporating Listener/Notifie...
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/10#discussion_r59930579
--- Diff: minifi-bootstrap/src/main/java/org/apache/nifi/minifi/bootstrap/RunMiNiFi.java ---
@@ -567,6 +586,104 @@ public void dump(final File dumpFile) throws IOException {
}
}
+ public void reload() throws IOException {
+ final Logger logger = cmdLogger;
+ final Integer port = getCurrentPort(logger);
+ if (port == null) {
+ logger.info("Apache MiNiFi is not currently running");
+ return;
+ }
+
+ // indicate that a reload command is in progress
+ final File reloadLockFile = getReloadFile(logger);
+ if (!reloadLockFile.exists()) {
+ reloadLockFile.createNewFile();
+ }
+
+ final Properties nifiProps = loadProperties(logger);
+ final String secretKey = nifiProps.getProperty("secret.key");
+ final String pid = nifiProps.getProperty("pid");
+
+ try (final Socket socket = new Socket()) {
+ logger.debug("Connecting to MiNiFi instance");
+ socket.setSoTimeout(10000);
+ socket.connect(new InetSocketAddress("localhost", port));
+ logger.debug("Established connection to MiNiFi instance.");
+ socket.setSoTimeout(10000);
+
+ logger.debug("Sending RELOAD Command to port {}", port);
+ final OutputStream out = socket.getOutputStream();
+ out.write((RELOAD_CMD + " " + secretKey + "\n").getBytes(StandardCharsets.UTF_8));
+ out.flush();
+ socket.shutdownOutput();
+
+ final InputStream in = socket.getInputStream();
+ int lastChar;
+ final StringBuilder sb = new StringBuilder();
+ while ((lastChar = in.read()) > -1) {
+ sb.append((char) lastChar);
+ }
+ final String response = sb.toString().trim();
+
+ logger.debug("Received response to RELOAD command: {}", response);
+
+ if (RELOAD_CMD.equals(response)) {
+ logger.info("Apache MiNiFi has accepted the Reload Command and is reloading");
+
+ if (pid != null) {
+ final Properties bootstrapProperties = getBootstrapProperties();
+
+ String gracefulShutdown = bootstrapProperties.getProperty(GRACEFUL_SHUTDOWN_PROP, DEFAULT_GRACEFUL_SHUTDOWN_VALUE);
+ int gracefulShutdownSeconds;
+ try {
+ gracefulShutdownSeconds = Integer.parseInt(gracefulShutdown);
+ } catch (final NumberFormatException nfe) {
+ gracefulShutdownSeconds = Integer.parseInt(DEFAULT_GRACEFUL_SHUTDOWN_VALUE);
+ }
+
+ final long startWait = System.nanoTime();
+ while (isProcessRunning(pid, logger)) {
+ logger.info("Waiting for Apache MiNiFi to finish shutting down...");
+ final long waitNanos = System.nanoTime() - startWait;
+ final long waitSeconds = TimeUnit.NANOSECONDS.toSeconds(waitNanos);
+ if (waitSeconds >= gracefulShutdownSeconds && gracefulShutdownSeconds > 0) {
+ if (isProcessRunning(pid, logger)) {
+ logger.warn("MiNiFi has not finished shutting down after {} seconds. Killing process.", gracefulShutdownSeconds);
--- End diff --
Yeah, just adding something about it shutting down in order to reload would be fine.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi-minifi pull request: MINIFI-14 Incorporating Listener/Notifie...
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/10#discussion_r60062578
--- Diff: minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-resources/src/main/resources/conf/bootstrap.conf ---
@@ -28,6 +28,16 @@ conf.dir=./conf
# How long to wait after telling MiNiFi to shutdown before explicitly killing the Process
graceful.shutdown.seconds=20
+# The location for the configuration file
+nifi.minifi.config=./conf/config.yml
+
+# Notifiers to use for the associated agent, comma separated list of class names
+#nifi.minifi.notifier.components=org.apache.nifi.minifi.bootstrap.configuration.FileChangeNotifier
+
+# File change notifier configuration for path of the file to monitor and the associated polling period
+#nifi.minifi.notifier.file.config.path=
--- End diff --
Sounds good
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi-minifi pull request: MINIFI-14 Incorporating Listener/Notifie...
Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/10#discussion_r59926319
--- Diff: minifi-bootstrap/src/main/java/org/apache/nifi/minifi/bootstrap/RunMiNiFi.java ---
@@ -567,6 +586,104 @@ public void dump(final File dumpFile) throws IOException {
}
}
+ public void reload() throws IOException {
+ final Logger logger = cmdLogger;
+ final Integer port = getCurrentPort(logger);
+ if (port == null) {
+ logger.info("Apache MiNiFi is not currently running");
+ return;
+ }
+
+ // indicate that a reload command is in progress
+ final File reloadLockFile = getReloadFile(logger);
+ if (!reloadLockFile.exists()) {
+ reloadLockFile.createNewFile();
+ }
+
+ final Properties nifiProps = loadProperties(logger);
+ final String secretKey = nifiProps.getProperty("secret.key");
+ final String pid = nifiProps.getProperty("pid");
+
+ try (final Socket socket = new Socket()) {
+ logger.debug("Connecting to MiNiFi instance");
+ socket.setSoTimeout(10000);
+ socket.connect(new InetSocketAddress("localhost", port));
+ logger.debug("Established connection to MiNiFi instance.");
+ socket.setSoTimeout(10000);
+
+ logger.debug("Sending RELOAD Command to port {}", port);
+ final OutputStream out = socket.getOutputStream();
+ out.write((RELOAD_CMD + " " + secretKey + "\n").getBytes(StandardCharsets.UTF_8));
+ out.flush();
+ socket.shutdownOutput();
+
+ final InputStream in = socket.getInputStream();
+ int lastChar;
+ final StringBuilder sb = new StringBuilder();
+ while ((lastChar = in.read()) > -1) {
+ sb.append((char) lastChar);
+ }
+ final String response = sb.toString().trim();
+
+ logger.debug("Received response to RELOAD command: {}", response);
+
+ if (RELOAD_CMD.equals(response)) {
+ logger.info("Apache MiNiFi has accepted the Reload Command and is reloading");
+
+ if (pid != null) {
+ final Properties bootstrapProperties = getBootstrapProperties();
+
+ String gracefulShutdown = bootstrapProperties.getProperty(GRACEFUL_SHUTDOWN_PROP, DEFAULT_GRACEFUL_SHUTDOWN_VALUE);
+ int gracefulShutdownSeconds;
+ try {
+ gracefulShutdownSeconds = Integer.parseInt(gracefulShutdown);
+ } catch (final NumberFormatException nfe) {
+ gracefulShutdownSeconds = Integer.parseInt(DEFAULT_GRACEFUL_SHUTDOWN_VALUE);
+ }
+
+ final long startWait = System.nanoTime();
+ while (isProcessRunning(pid, logger)) {
+ logger.info("Waiting for Apache MiNiFi to finish shutting down...");
+ final long waitNanos = System.nanoTime() - startWait;
+ final long waitSeconds = TimeUnit.NANOSECONDS.toSeconds(waitNanos);
+ if (waitSeconds >= gracefulShutdownSeconds && gracefulShutdownSeconds > 0) {
+ if (isProcessRunning(pid, logger)) {
+ logger.warn("MiNiFi has not finished shutting down after {} seconds. Killing process.", gracefulShutdownSeconds);
--- End diff --
Sufficient to note that the reload is occurring in these statements?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi-minifi pull request: MINIFI-14 Incorporating Listener/Notifie...
Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/10#discussion_r59961712
--- Diff: minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-resources/src/main/resources/conf/bootstrap.conf ---
@@ -28,6 +28,16 @@ conf.dir=./conf
# How long to wait after telling MiNiFi to shutdown before explicitly killing the Process
graceful.shutdown.seconds=20
+# The location for the configuration file
+nifi.minifi.config=./conf/config.yml
+
+# Notifiers to use for the associated agent, comma separated list of class names
+#nifi.minifi.notifier.components=org.apache.nifi.minifi.bootstrap.configuration.FileChangeNotifier
+
+# File change notifier configuration for path of the file to monitor and the associated polling period
+#nifi.minifi.notifier.file.config.path=
--- End diff --
Was a bit unsure about how the mechanics of this would work from a general interface standpoint. What I was thinking was that new configurations could be pushed in file form by dropping them in a given watch directory and then the rest would behave as currently specified. If we are watching the file that is the config itself, this could be a bit messy.
I think watching the file itself could also be fine, but would think that there would need to be some indicator within the handleChange to a) not ingest the file as an InputStream and b) not actually write the file out.
Good either way, just want to ensure the various implementations do not have to cater to that only of the file.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi-minifi pull request: MINIFI-14 Incorporating Listener/Notifie...
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/10#discussion_r59926141
--- Diff: minifi-bootstrap/src/main/java/org/apache/nifi/minifi/bootstrap/RunMiNiFi.java ---
@@ -567,6 +586,104 @@ public void dump(final File dumpFile) throws IOException {
}
}
+ public void reload() throws IOException {
+ final Logger logger = cmdLogger;
+ final Integer port = getCurrentPort(logger);
+ if (port == null) {
+ logger.info("Apache MiNiFi is not currently running");
+ return;
+ }
+
+ // indicate that a reload command is in progress
+ final File reloadLockFile = getReloadFile(logger);
+ if (!reloadLockFile.exists()) {
+ reloadLockFile.createNewFile();
+ }
+
+ final Properties nifiProps = loadProperties(logger);
+ final String secretKey = nifiProps.getProperty("secret.key");
+ final String pid = nifiProps.getProperty("pid");
+
+ try (final Socket socket = new Socket()) {
+ logger.debug("Connecting to MiNiFi instance");
+ socket.setSoTimeout(10000);
+ socket.connect(new InetSocketAddress("localhost", port));
+ logger.debug("Established connection to MiNiFi instance.");
+ socket.setSoTimeout(10000);
+
+ logger.debug("Sending RELOAD Command to port {}", port);
+ final OutputStream out = socket.getOutputStream();
+ out.write((RELOAD_CMD + " " + secretKey + "\n").getBytes(StandardCharsets.UTF_8));
+ out.flush();
+ socket.shutdownOutput();
+
+ final InputStream in = socket.getInputStream();
+ int lastChar;
+ final StringBuilder sb = new StringBuilder();
+ while ((lastChar = in.read()) > -1) {
+ sb.append((char) lastChar);
+ }
+ final String response = sb.toString().trim();
+
+ logger.debug("Received response to RELOAD command: {}", response);
+
+ if (RELOAD_CMD.equals(response)) {
+ logger.info("Apache MiNiFi has accepted the Reload Command and is reloading");
+
+ if (pid != null) {
+ final Properties bootstrapProperties = getBootstrapProperties();
+
+ String gracefulShutdown = bootstrapProperties.getProperty(GRACEFUL_SHUTDOWN_PROP, DEFAULT_GRACEFUL_SHUTDOWN_VALUE);
+ int gracefulShutdownSeconds;
+ try {
+ gracefulShutdownSeconds = Integer.parseInt(gracefulShutdown);
+ } catch (final NumberFormatException nfe) {
+ gracefulShutdownSeconds = Integer.parseInt(DEFAULT_GRACEFUL_SHUTDOWN_VALUE);
+ }
+
+ final long startWait = System.nanoTime();
+ while (isProcessRunning(pid, logger)) {
+ logger.info("Waiting for Apache MiNiFi to finish shutting down...");
+ final long waitNanos = System.nanoTime() - startWait;
+ final long waitSeconds = TimeUnit.NANOSECONDS.toSeconds(waitNanos);
+ if (waitSeconds >= gracefulShutdownSeconds && gracefulShutdownSeconds > 0) {
+ if (isProcessRunning(pid, logger)) {
+ logger.warn("MiNiFi has not finished shutting down after {} seconds. Killing process.", gracefulShutdownSeconds);
--- End diff --
While I understand that underneath the bootstrap is waiting for the MiNiFi instance to shutdown before it starts it again, these 2 log messages (646 an 651) could be confusing when the user is just expecting it to reload.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi-minifi pull request: MINIFI-14 Incorporating Listener/Notifie...
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/10#discussion_r59949432
--- Diff: minifi-bootstrap/src/main/java/org/apache/nifi/minifi/bootstrap/configuration/FileChangeNotifier.java ---
@@ -92,6 +93,7 @@ protected boolean targetChanged() {
final WatchEvent<Path> pathEvent = (WatchEvent<Path>) watchEvt;
final Path changedFile = pathEvent.context();
+ pathEvent.count();
--- End diff --
I don't think method call has any side effects and the return value ignored.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi-minifi pull request: MINIFI-14 Incorporating Listener/Notifie...
Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/10#discussion_r59961376
--- Diff: minifi-bootstrap/src/main/java/org/apache/nifi/minifi/bootstrap/configuration/FileChangeNotifier.java ---
@@ -92,6 +93,7 @@ protected boolean targetChanged() {
final WatchEvent<Path> pathEvent = (WatchEvent<Path>) watchEvt;
final Path changedFile = pathEvent.context();
+ pathEvent.count();
--- End diff --
Good catch, definitely a relic developing.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---