You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by pvillard31 <gi...@git.apache.org> on 2016/08/31 21:53:36 UTC
[GitHub] nifi pull request #980: NIFI-1170 - Improved TailFile processor to support m...
GitHub user pvillard31 opened a pull request:
https://github.com/apache/nifi/pull/980
NIFI-1170 - Improved TailFile processor to support multiple files tailing
Modifications should not break backward compatibility.
Two modes: single file (default) or multi files.
In case of multi files mode, a base directory property is required.
This will be the base directory to recursively look for the files to
tail. Since this lookup can be consuming, I assumed that files to tail
must exist when starting the processor.
The rolling file pattern now accepts the \u2018${filename}\u2019 keyword to reference
the original filename (without the extension). This is useful when
defining the rolling file pattern for multiple files in the same
directory. It obviously supposes that all the files to tail have a
similar rolling pattern. Besides the rolling file must be plain text
(no compression supported so far, this is wip).
This PR includes the modifications proposed in PR #490.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/pvillard31/nifi tailfile-regex
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi/pull/980.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 #980
----
commit c7e4ba609697727c188e1f7db1658d6d9c75abc1
Author: Pierre Villard <pi...@gmail.com>
Date: 2016-08-31T16:53:26Z
NIFI-1170 - Improved TailFile processor to support multiple files tailing
Modifications should not break backward compatibility.
Two modes: single file (default) or multi files.
In case of multi files mode, a base directory property is required.
This will be the base directory to recursively look for the files to
tail. Since this lookup can be consuming, I assumed that files to tail
must exist when starting the processor.
The rolling file pattern accepts the \u2018${filename}\u2019 keyword to reference
the original filename (without the extension). This is useful when
defining the rolling file pattern for multiple files in the same
directory. It obviously supposes that all the files to tail have a
similar rolling pattern. Besides the rolling file must be plain text
(no compression supported so far).
This PR includes the modifications proposed in PR #490.
----
---
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 issue #980: NIFI-1170 - Improved TailFile processor to support multiple...
Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on the issue:
https://github.com/apache/nifi/pull/980
@trixpan I modified the documentation as you suggest even though I do have use cases where the appended file is static and rolling files contain the date. I think you can give it a try... it may need a bit a polishing but it should work. I tried to stay as close as possible to the original implementation, let me know how it behaves when dealing with huge files :)
---
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 pull request #980: NIFI-1170 - Improved TailFile processor to support m...
Posted by olegz <gi...@git.apache.org>.
Github user olegz commented on a diff in the pull request:
https://github.com/apache/nifi/pull/980#discussion_r78600733
--- Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TailFile.java ---
@@ -117,31 +173,78 @@
.allowableValues(LOCATION_LOCAL, LOCATION_REMOTE)
.defaultValue(LOCATION_LOCAL.getValue())
.build();
+
static final PropertyDescriptor START_POSITION = new PropertyDescriptor.Builder()
.name("Initial Start Position")
- .description("When the Processor first begins to tail data, this property specifies where the Processor should begin reading data. Once data has been ingested from the file, "
+ .description("When the Processor first begins to tail data, this property specifies where the Processor should begin reading data. Once data has been ingested from a file, "
+ "the Processor will continue from the last point from which it has received data.")
.allowableValues(START_BEGINNING_OF_TIME, START_CURRENT_FILE, START_CURRENT_TIME)
.defaultValue(START_CURRENT_FILE.getValue())
.required(true)
.build();
+ static final PropertyDescriptor RECURSIVE = new PropertyDescriptor.Builder()
+ .name("tailfile-recursive-lookup")
+ .displayName("Recursive lookup")
+ .description("When using Multiple files mode, this property defines if files must be listed recursively or not"
+ + " in the base directory.")
+ .allowableValues("true", "false")
+ .defaultValue("true")
+ .required(true)
+ .build();
+
+ static final PropertyDescriptor ROLLING_STRATEGY = new PropertyDescriptor.Builder()
+ .name("tailfile-rolling-strategy")
+ .displayName("Rolling Strategy")
+ .description("Specifies if the files to tail have a fixed name or not.")
+ .required(true)
+ .allowableValues(FIXED_NAME, CHANGING_NAME)
+ .defaultValue(FIXED_NAME.getValue())
+ .build();
+
+ static final PropertyDescriptor LOOKUP_FREQUENCY = new PropertyDescriptor.Builder()
+ .name("tailfile-lookup-frequency")
+ .displayName("Lookup frequency")
+ .description("Only used in Multiple files mode and Changing name rolling strategy, it specifies the minimum "
+ + "duration the processor will wait before listing again the files to tail.")
+ .required(false)
+ .addValidator(StandardValidators.TIME_PERIOD_VALIDATOR)
+ .defaultValue("10 minutes")
--- End diff --
Pierre, I am actually wondering if MAX Long is the appropriate default here. If I understand it correctly you do NOT want to resist them at all unless some time has passed and if so the best "some time" IMHO is infinity while letting the user override
---
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 pull request #980: NIFI-1170 - Improved TailFile processor to support m...
Posted by olegz <gi...@git.apache.org>.
Github user olegz commented on a diff in the pull request:
https://github.com/apache/nifi/pull/980#discussion_r78183927
--- Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TailFile.java ---
@@ -117,31 +173,78 @@
.allowableValues(LOCATION_LOCAL, LOCATION_REMOTE)
.defaultValue(LOCATION_LOCAL.getValue())
.build();
+
static final PropertyDescriptor START_POSITION = new PropertyDescriptor.Builder()
.name("Initial Start Position")
- .description("When the Processor first begins to tail data, this property specifies where the Processor should begin reading data. Once data has been ingested from the file, "
+ .description("When the Processor first begins to tail data, this property specifies where the Processor should begin reading data. Once data has been ingested from a file, "
+ "the Processor will continue from the last point from which it has received data.")
.allowableValues(START_BEGINNING_OF_TIME, START_CURRENT_FILE, START_CURRENT_TIME)
.defaultValue(START_CURRENT_FILE.getValue())
.required(true)
.build();
+ static final PropertyDescriptor RECURSIVE = new PropertyDescriptor.Builder()
+ .name("tailfile-recursive-lookup")
+ .displayName("Recursive lookup")
+ .description("When using Multiple files mode, this property defines if files must be listed recursively or not"
+ + " in the base directory.")
+ .allowableValues("true", "false")
+ .defaultValue("true")
+ .required(true)
+ .build();
+
+ static final PropertyDescriptor ROLLING_STRATEGY = new PropertyDescriptor.Builder()
+ .name("tailfile-rolling-strategy")
+ .displayName("Rolling Strategy")
+ .description("Specifies if the files to tail have a fixed name or not.")
+ .required(true)
+ .allowableValues(FIXED_NAME, CHANGING_NAME)
+ .defaultValue(FIXED_NAME.getValue())
+ .build();
+
+ static final PropertyDescriptor LOOKUP_FREQUENCY = new PropertyDescriptor.Builder()
+ .name("tailfile-lookup-frequency")
+ .displayName("Lookup frequency")
+ .description("Only used in Multiple files mode and Changing name rolling strategy, it specifies the minimum "
+ + "duration the processor will wait before listing again the files to tail.")
+ .required(false)
+ .addValidator(StandardValidators.TIME_PERIOD_VALIDATOR)
+ .defaultValue("10 minutes")
--- End diff --
Hmm, arbitrary timeouts always makes me worried unless there is some reasonable justification. For example, why 10 and not 20 min? In any event it seems like you want to give it a rather long time, unless overriden by user. If so wouldn't making it "infinite" accomplish same/similar goal - _will not re-list for 'n' time' where 'n' can be set by the user, otherwise its something like Long.MAX_VAL.
This is by no means a directive, just a personal opinion, so would be curious to hear your thoughts.
---
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 pull request #980: NIFI-1170 - Improved TailFile processor to support m...
Posted by olegz <gi...@git.apache.org>.
Github user olegz commented on a diff in the pull request:
https://github.com/apache/nifi/pull/980#discussion_r78185433
--- Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TailFile.java ---
@@ -245,58 +490,82 @@ private void recoverState(final ProcessContext context, final Map<String, String
+ "this indicates that the file has rotated. Will begin tailing current file from beginning.", new Object[]{existingTailFile.length(), position});
}
- state = new TailFileState(currentFilename, tailFile, reader, position, timestamp, checksum, ByteBuffer.allocate(65536));
+ states.get(filePath).setState(new TailFileState(filePath, tailFile, reader, position, timestamp, length, checksum, ByteBuffer.allocate(65536)));
} else {
- resetState(currentFilename);
+ resetState(filePath);
}
- getLogger().debug("Recovered state {}", new Object[]{state});
+ getLogger().debug("Recovered state {}", new Object[]{states.get(filePath).getState()});
}
- private void resetState(final String currentFilename) {
- expectedRecoveryChecksum = null;
- state = new TailFileState(currentFilename, null, null, 0L, 0L, null, ByteBuffer.allocate(65536));
+ private void resetState(final String filePath) {
+ states.get(filePath).setExpectedRecoveryChecksum(null);
+ states.get(filePath).setState(new TailFileState(filePath, null, null, 0L, 0L, 0L, null, ByteBuffer.allocate(65536)));
}
@OnStopped
public void cleanup() {
- final TailFileState state = this.state;
- if (state == null) {
+ for (TailFileObject tfo : states.values()) {
+ cleanReader(tfo);
+ final TailFileState state = tfo.getState();
+ tfo.setState(new TailFileState(state.getFilename(), state.getFile(), null, state.getPosition(), state.getTimestamp(), state.getLength(), state.getChecksum(), state.getBuffer()));
+ }
+ }
+
+ private void cleanReader(TailFileObject tfo) {
+ if (tfo.getState() == null) {
return;
}
- final FileChannel reader = state.getReader();
+ final FileChannel reader = tfo.getState().getReader();
if (reader == null) {
return;
}
try {
reader.close();
+ getLogger().debug("Closed FileChannel {}", new Object[]{reader});
} catch (final IOException ioe) {
getLogger().warn("Failed to close file handle during cleanup");
}
-
- getLogger().debug("Closed FileChannel {}", new Object[]{reader});
-
- this.state = new TailFileState(state.getFilename(), state.getFile(), null, state.getPosition(), state.getTimestamp(), state.getChecksum(), state.getBuffer());
}
@Override
public void onTrigger(final ProcessContext context, final ProcessSession session) throws ProcessException {
+ if(isMultiChanging.get()) {
+ long timeSinceLastLookup = new Date().getTime() - lastLookup.get();
+ if(timeSinceLastLookup > context.getProperty(LOOKUP_FREQUENCY).asTimePeriod(TimeUnit.MILLISECONDS)) {
+ try {
+ recoverState(context);
+ } catch (IOException e) {
+ getLogger().warn("Exception raised while looking up for new files", e);
--- End diff --
Did you mean ```getLogger().error(..)```?
---
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 pull request #980: NIFI-1170 - Improved TailFile processor to support m...
Posted by olegz <gi...@git.apache.org>.
Github user olegz commented on a diff in the pull request:
https://github.com/apache/nifi/pull/980#discussion_r78752139
--- Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TailFile.java ---
@@ -117,31 +173,78 @@
.allowableValues(LOCATION_LOCAL, LOCATION_REMOTE)
.defaultValue(LOCATION_LOCAL.getValue())
.build();
+
static final PropertyDescriptor START_POSITION = new PropertyDescriptor.Builder()
.name("Initial Start Position")
- .description("When the Processor first begins to tail data, this property specifies where the Processor should begin reading data. Once data has been ingested from the file, "
+ .description("When the Processor first begins to tail data, this property specifies where the Processor should begin reading data. Once data has been ingested from a file, "
+ "the Processor will continue from the last point from which it has received data.")
.allowableValues(START_BEGINNING_OF_TIME, START_CURRENT_FILE, START_CURRENT_TIME)
.defaultValue(START_CURRENT_FILE.getValue())
.required(true)
.build();
+ static final PropertyDescriptor RECURSIVE = new PropertyDescriptor.Builder()
+ .name("tailfile-recursive-lookup")
+ .displayName("Recursive lookup")
+ .description("When using Multiple files mode, this property defines if files must be listed recursively or not"
+ + " in the base directory.")
+ .allowableValues("true", "false")
+ .defaultValue("true")
+ .required(true)
+ .build();
+
+ static final PropertyDescriptor ROLLING_STRATEGY = new PropertyDescriptor.Builder()
+ .name("tailfile-rolling-strategy")
+ .displayName("Rolling Strategy")
+ .description("Specifies if the files to tail have a fixed name or not.")
+ .required(true)
+ .allowableValues(FIXED_NAME, CHANGING_NAME)
+ .defaultValue(FIXED_NAME.getValue())
+ .build();
+
+ static final PropertyDescriptor LOOKUP_FREQUENCY = new PropertyDescriptor.Builder()
+ .name("tailfile-lookup-frequency")
+ .displayName("Lookup frequency")
+ .description("Only used in Multiple files mode and Changing name rolling strategy, it specifies the minimum "
+ + "duration the processor will wait before listing again the files to tail.")
+ .required(false)
+ .addValidator(StandardValidators.TIME_PERIOD_VALIDATOR)
+ .defaultValue("10 minutes")
--- End diff --
@joewitt that is what we are trying to determine "finite value you think is reasonable" and based on Pierre's explanation 10 minutes seems to be anything but. . .
@pvillard31 that makes more sense now, so given that in my "practical" experience a typical rollovers are 24hrs, do you think setting the default value to 1hr or 24hr would be more appropriate?
---
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 pull request #980: NIFI-1170 - Improved TailFile processor to support m...
Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on a diff in the pull request:
https://github.com/apache/nifi/pull/980#discussion_r78438361
--- Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TailFile.java ---
@@ -117,31 +173,78 @@
.allowableValues(LOCATION_LOCAL, LOCATION_REMOTE)
.defaultValue(LOCATION_LOCAL.getValue())
.build();
+
static final PropertyDescriptor START_POSITION = new PropertyDescriptor.Builder()
.name("Initial Start Position")
- .description("When the Processor first begins to tail data, this property specifies where the Processor should begin reading data. Once data has been ingested from the file, "
+ .description("When the Processor first begins to tail data, this property specifies where the Processor should begin reading data. Once data has been ingested from a file, "
+ "the Processor will continue from the last point from which it has received data.")
.allowableValues(START_BEGINNING_OF_TIME, START_CURRENT_FILE, START_CURRENT_TIME)
.defaultValue(START_CURRENT_FILE.getValue())
.required(true)
.build();
+ static final PropertyDescriptor RECURSIVE = new PropertyDescriptor.Builder()
+ .name("tailfile-recursive-lookup")
+ .displayName("Recursive lookup")
+ .description("When using Multiple files mode, this property defines if files must be listed recursively or not"
+ + " in the base directory.")
+ .allowableValues("true", "false")
+ .defaultValue("true")
+ .required(true)
+ .build();
+
+ static final PropertyDescriptor ROLLING_STRATEGY = new PropertyDescriptor.Builder()
+ .name("tailfile-rolling-strategy")
+ .displayName("Rolling Strategy")
+ .description("Specifies if the files to tail have a fixed name or not.")
+ .required(true)
+ .allowableValues(FIXED_NAME, CHANGING_NAME)
+ .defaultValue(FIXED_NAME.getValue())
+ .build();
+
+ static final PropertyDescriptor LOOKUP_FREQUENCY = new PropertyDescriptor.Builder()
+ .name("tailfile-lookup-frequency")
+ .displayName("Lookup frequency")
+ .description("Only used in Multiple files mode and Changing name rolling strategy, it specifies the minimum "
+ + "duration the processor will wait before listing again the files to tail.")
+ .required(false)
+ .addValidator(StandardValidators.TIME_PERIOD_VALIDATOR)
+ .defaultValue("10 minutes")
--- End diff --
You are definitely right, it really depends of the use case and I am in no position to assume that those values are a good fit in most situations. I removed default values and I'll assume that additional details in the documentation will give a good sense of what is the role of those values.
---
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 issue #980: NIFI-1170 - Improved TailFile processor to support multiple...
Posted by trixpan <gi...@git.apache.org>.
Github user trixpan commented on the issue:
https://github.com/apache/nifi/pull/980
Let me know when this is ready to test and I will give it a go
---
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 pull request #980: NIFI-1170 - Improved TailFile processor to support m...
Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on a diff in the pull request:
https://github.com/apache/nifi/pull/980#discussion_r78954211
--- Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TailFile.java ---
@@ -117,31 +173,78 @@
.allowableValues(LOCATION_LOCAL, LOCATION_REMOTE)
.defaultValue(LOCATION_LOCAL.getValue())
.build();
+
static final PropertyDescriptor START_POSITION = new PropertyDescriptor.Builder()
.name("Initial Start Position")
- .description("When the Processor first begins to tail data, this property specifies where the Processor should begin reading data. Once data has been ingested from the file, "
+ .description("When the Processor first begins to tail data, this property specifies where the Processor should begin reading data. Once data has been ingested from a file, "
+ "the Processor will continue from the last point from which it has received data.")
.allowableValues(START_BEGINNING_OF_TIME, START_CURRENT_FILE, START_CURRENT_TIME)
.defaultValue(START_CURRENT_FILE.getValue())
.required(true)
.build();
+ static final PropertyDescriptor RECURSIVE = new PropertyDescriptor.Builder()
+ .name("tailfile-recursive-lookup")
+ .displayName("Recursive lookup")
+ .description("When using Multiple files mode, this property defines if files must be listed recursively or not"
+ + " in the base directory.")
+ .allowableValues("true", "false")
+ .defaultValue("true")
+ .required(true)
+ .build();
+
+ static final PropertyDescriptor ROLLING_STRATEGY = new PropertyDescriptor.Builder()
+ .name("tailfile-rolling-strategy")
+ .displayName("Rolling Strategy")
+ .description("Specifies if the files to tail have a fixed name or not.")
+ .required(true)
+ .allowableValues(FIXED_NAME, CHANGING_NAME)
+ .defaultValue(FIXED_NAME.getValue())
+ .build();
+
+ static final PropertyDescriptor LOOKUP_FREQUENCY = new PropertyDescriptor.Builder()
+ .name("tailfile-lookup-frequency")
+ .displayName("Lookup frequency")
+ .description("Only used in Multiple files mode and Changing name rolling strategy, it specifies the minimum "
+ + "duration the processor will wait before listing again the files to tail.")
+ .required(false)
+ .addValidator(StandardValidators.TIME_PERIOD_VALIDATOR)
+ .defaultValue("10 minutes")
--- End diff --
@olegz @joewitt
Just updated the PR with a default of 10 minutes for refresh frequency, and 24 hours for maximum age as a daily rollover pattern seems to be indeed the most common case. I updated the property descriptions and additional details page.
I also changed a little bit the approach on the regex matching while listing files to tail. Previously I was only matching the regular expression against file name, now I am trying to match the regular expression against the path. It gives more flexibility to the users. I added an example in the additional details.
And I squashed my commits.
Let me know your thoughts.
@trixpan, I understand your point but it sounds like an edge case, no? It would require to tail a very large number of files to get in such a situation. I'd be inclined to see this as an additional improvement in a separate PR if you agree.
---
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 pull request #980: NIFI-1170 - Improved TailFile processor to support m...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/nifi/pull/980
---
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 pull request #980: NIFI-1170 - Improved TailFile processor to support m...
Posted by trixpan <gi...@git.apache.org>.
Github user trixpan commented on a diff in the pull request:
https://github.com/apache/nifi/pull/980#discussion_r78770355
--- Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TailFile.java ---
@@ -117,31 +173,78 @@
.allowableValues(LOCATION_LOCAL, LOCATION_REMOTE)
.defaultValue(LOCATION_LOCAL.getValue())
.build();
+
static final PropertyDescriptor START_POSITION = new PropertyDescriptor.Builder()
.name("Initial Start Position")
- .description("When the Processor first begins to tail data, this property specifies where the Processor should begin reading data. Once data has been ingested from the file, "
+ .description("When the Processor first begins to tail data, this property specifies where the Processor should begin reading data. Once data has been ingested from a file, "
+ "the Processor will continue from the last point from which it has received data.")
.allowableValues(START_BEGINNING_OF_TIME, START_CURRENT_FILE, START_CURRENT_TIME)
.defaultValue(START_CURRENT_FILE.getValue())
.required(true)
.build();
+ static final PropertyDescriptor RECURSIVE = new PropertyDescriptor.Builder()
+ .name("tailfile-recursive-lookup")
+ .displayName("Recursive lookup")
+ .description("When using Multiple files mode, this property defines if files must be listed recursively or not"
+ + " in the base directory.")
+ .allowableValues("true", "false")
+ .defaultValue("true")
+ .required(true)
+ .build();
+
+ static final PropertyDescriptor ROLLING_STRATEGY = new PropertyDescriptor.Builder()
+ .name("tailfile-rolling-strategy")
+ .displayName("Rolling Strategy")
+ .description("Specifies if the files to tail have a fixed name or not.")
+ .required(true)
+ .allowableValues(FIXED_NAME, CHANGING_NAME)
+ .defaultValue(FIXED_NAME.getValue())
+ .build();
+
+ static final PropertyDescriptor LOOKUP_FREQUENCY = new PropertyDescriptor.Builder()
+ .name("tailfile-lookup-frequency")
+ .displayName("Lookup frequency")
+ .description("Only used in Multiple files mode and Changing name rolling strategy, it specifies the minimum "
+ + "duration the processor will wait before listing again the files to tail.")
+ .required(false)
+ .addValidator(StandardValidators.TIME_PERIOD_VALIDATOR)
+ .defaultValue("10 minutes")
--- End diff --
@joewitt
I think @pvillard31 has a point when he says the status of a file should ALWAYS be tracked unless:
1. overwritten / reset by the user (causing data duplication).
2. too old to be relevant (removed automatically)
Under this arrangement, the two timers make sense:
1 - Maximum age of file - if file is older than this date it won't be tailed. (_this happens to be very similar to Heka's approach as well_)
2 - How frequently to harvest for new files - self explanatory
2b - if new file is found tail. If file is pre-existent and is older than max age remove status;
In addition, we could consider what flume-ng taildir called an idle timeout,
`idleTimeout - Time (ms) to close inactive files. If the closed file is appended new lines to, this source will automatically re-open it.`
These are files that are younger than maximum age, but largely stagnated. We would keep their status (until expiry) but they would be closed and only re-opened if the file size increased (or other tail conditions were be triggered).
flume-ng tried to deal with resource waste by using and increasing delay to poll the idle files. The higher the number of polls without new data, the longer it would take before a new retry. Not sure if this is something we would like to do but would also help.
Inevitably, most teams using date based naming conventions do that to prevent performing truncation of a file when logrotate runs and I suspect we should simply let the user know that having too many files in the same folder, matching the same URL would cause impact to performance and that compressing them so they don't match the file regex, or moving them to other directories in order to minimise resource waste.
Hope this makes sense
---
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 issue #980: NIFI-1170 - Improved TailFile processor to support multiple...
Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on the issue:
https://github.com/apache/nifi/pull/980
FYI: this is not ready for review. After a talk with @trixpan, I'll put some additional work into it. I'll let you know when I think this is ready to take into account the use case described in the JIRA.
---
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 pull request #980: NIFI-1170 - Improved TailFile processor to support m...
Posted by joewitt <gi...@git.apache.org>.
Github user joewitt commented on a diff in the pull request:
https://github.com/apache/nifi/pull/980#discussion_r78673963
--- Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TailFile.java ---
@@ -117,31 +173,78 @@
.allowableValues(LOCATION_LOCAL, LOCATION_REMOTE)
.defaultValue(LOCATION_LOCAL.getValue())
.build();
+
static final PropertyDescriptor START_POSITION = new PropertyDescriptor.Builder()
.name("Initial Start Position")
- .description("When the Processor first begins to tail data, this property specifies where the Processor should begin reading data. Once data has been ingested from the file, "
+ .description("When the Processor first begins to tail data, this property specifies where the Processor should begin reading data. Once data has been ingested from a file, "
+ "the Processor will continue from the last point from which it has received data.")
.allowableValues(START_BEGINNING_OF_TIME, START_CURRENT_FILE, START_CURRENT_TIME)
.defaultValue(START_CURRENT_FILE.getValue())
.required(true)
.build();
+ static final PropertyDescriptor RECURSIVE = new PropertyDescriptor.Builder()
+ .name("tailfile-recursive-lookup")
+ .displayName("Recursive lookup")
+ .description("When using Multiple files mode, this property defines if files must be listed recursively or not"
+ + " in the base directory.")
+ .allowableValues("true", "false")
+ .defaultValue("true")
+ .required(true)
+ .build();
+
+ static final PropertyDescriptor ROLLING_STRATEGY = new PropertyDescriptor.Builder()
+ .name("tailfile-rolling-strategy")
+ .displayName("Rolling Strategy")
+ .description("Specifies if the files to tail have a fixed name or not.")
+ .required(true)
+ .allowableValues(FIXED_NAME, CHANGING_NAME)
+ .defaultValue(FIXED_NAME.getValue())
+ .build();
+
+ static final PropertyDescriptor LOOKUP_FREQUENCY = new PropertyDescriptor.Builder()
+ .name("tailfile-lookup-frequency")
+ .displayName("Lookup frequency")
+ .description("Only used in Multiple files mode and Changing name rolling strategy, it specifies the minimum "
+ + "duration the processor will wait before listing again the files to tail.")
+ .required(false)
+ .addValidator(StandardValidators.TIME_PERIOD_VALIDATOR)
+ .defaultValue("10 minutes")
--- End diff --
some finite value you think is reasonable is great. 10 mins 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 pull request #980: NIFI-1170 - Improved TailFile processor to support m...
Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on a diff in the pull request:
https://github.com/apache/nifi/pull/980#discussion_r78652271
--- Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TailFile.java ---
@@ -117,31 +173,78 @@
.allowableValues(LOCATION_LOCAL, LOCATION_REMOTE)
.defaultValue(LOCATION_LOCAL.getValue())
.build();
+
static final PropertyDescriptor START_POSITION = new PropertyDescriptor.Builder()
.name("Initial Start Position")
- .description("When the Processor first begins to tail data, this property specifies where the Processor should begin reading data. Once data has been ingested from the file, "
+ .description("When the Processor first begins to tail data, this property specifies where the Processor should begin reading data. Once data has been ingested from a file, "
+ "the Processor will continue from the last point from which it has received data.")
.allowableValues(START_BEGINNING_OF_TIME, START_CURRENT_FILE, START_CURRENT_TIME)
.defaultValue(START_CURRENT_FILE.getValue())
.required(true)
.build();
+ static final PropertyDescriptor RECURSIVE = new PropertyDescriptor.Builder()
+ .name("tailfile-recursive-lookup")
+ .displayName("Recursive lookup")
+ .description("When using Multiple files mode, this property defines if files must be listed recursively or not"
+ + " in the base directory.")
+ .allowableValues("true", "false")
+ .defaultValue("true")
+ .required(true)
+ .build();
+
+ static final PropertyDescriptor ROLLING_STRATEGY = new PropertyDescriptor.Builder()
+ .name("tailfile-rolling-strategy")
+ .displayName("Rolling Strategy")
+ .description("Specifies if the files to tail have a fixed name or not.")
+ .required(true)
+ .allowableValues(FIXED_NAME, CHANGING_NAME)
+ .defaultValue(FIXED_NAME.getValue())
+ .build();
+
+ static final PropertyDescriptor LOOKUP_FREQUENCY = new PropertyDescriptor.Builder()
+ .name("tailfile-lookup-frequency")
+ .displayName("Lookup frequency")
+ .description("Only used in Multiple files mode and Changing name rolling strategy, it specifies the minimum "
+ + "duration the processor will wait before listing again the files to tail.")
+ .required(false)
+ .addValidator(StandardValidators.TIME_PERIOD_VALIDATOR)
+ .defaultValue("10 minutes")
--- End diff --
In my mind the lookup frequency would depend on the rolling configuration of the log files. Let's say there is a daily rollover, then the lookup frequency could be something like one hour (in fact the maximum duration the user is ready to wait before receiving log messages from a fresh new file). On the other side, the maximum age duration is the time we need to wait after the last received message in a log file before considering there won't be any more message in this file and that we can "forget" about its state (and if it happens that we do receive new messages, then we will duplicate data). With a daily rollover, it could be 24h.
If we set the lookup frequency to infinity, it means that only the files that exist when the processor is started will be considered. I don't know if it's really what we want for the user to be the default, don't you think?
On the other hand, I could agree that for the maximum age we set it to infinity by default to ensure there is no data duplication unless the user is overriding the property. But in any case, how would you set a default value to infinity in the property descriptor configuration? ``.defaultValue()`` is expecting a String, so I'm not sure to see how you would use Max Long in combination with the time period validator. Otherwise, I could use the standard '0' to represent this case. Let me know what you think.
---
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 issue #980: NIFI-1170 - Improved TailFile processor to support multiple...
Posted by olegz <gi...@git.apache.org>.
Github user olegz commented on the issue:
https://github.com/apache/nifi/pull/980
@pvillard31 Looked thru changes, tested. Left one more comment, otherwise looks good. Will merge once the comment is addressed (not necessarily through code, just waned to get your opinion).
---
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 pull request #980: NIFI-1170 - Improved TailFile processor to support m...
Posted by joewitt <gi...@git.apache.org>.
Github user joewitt commented on a diff in the pull request:
https://github.com/apache/nifi/pull/980#discussion_r78762052
--- Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TailFile.java ---
@@ -117,31 +173,78 @@
.allowableValues(LOCATION_LOCAL, LOCATION_REMOTE)
.defaultValue(LOCATION_LOCAL.getValue())
.build();
+
static final PropertyDescriptor START_POSITION = new PropertyDescriptor.Builder()
.name("Initial Start Position")
- .description("When the Processor first begins to tail data, this property specifies where the Processor should begin reading data. Once data has been ingested from the file, "
+ .description("When the Processor first begins to tail data, this property specifies where the Processor should begin reading data. Once data has been ingested from a file, "
+ "the Processor will continue from the last point from which it has received data.")
.allowableValues(START_BEGINNING_OF_TIME, START_CURRENT_FILE, START_CURRENT_TIME)
.defaultValue(START_CURRENT_FILE.getValue())
.required(true)
.build();
+ static final PropertyDescriptor RECURSIVE = new PropertyDescriptor.Builder()
+ .name("tailfile-recursive-lookup")
+ .displayName("Recursive lookup")
+ .description("When using Multiple files mode, this property defines if files must be listed recursively or not"
+ + " in the base directory.")
+ .allowableValues("true", "false")
+ .defaultValue("true")
+ .required(true)
+ .build();
+
+ static final PropertyDescriptor ROLLING_STRATEGY = new PropertyDescriptor.Builder()
+ .name("tailfile-rolling-strategy")
+ .displayName("Rolling Strategy")
+ .description("Specifies if the files to tail have a fixed name or not.")
+ .required(true)
+ .allowableValues(FIXED_NAME, CHANGING_NAME)
+ .defaultValue(FIXED_NAME.getValue())
+ .build();
+
+ static final PropertyDescriptor LOOKUP_FREQUENCY = new PropertyDescriptor.Builder()
+ .name("tailfile-lookup-frequency")
+ .displayName("Lookup frequency")
+ .description("Only used in Multiple files mode and Changing name rolling strategy, it specifies the minimum "
+ + "duration the processor will wait before listing again the files to tail.")
+ .required(false)
+ .addValidator(StandardValidators.TIME_PERIOD_VALIDATOR)
+ .defaultValue("10 minutes")
--- End diff --
There appear to be two concerns here.
1) How often to look for new (not currently watched/tailed files)
2) At what point to consider a file fully consumed and no longer needing to be actively watched/tailed.
There should be a property for each concern then. For (1) a rather low value on the order of seconds to minutes as a default sounds reasonable. For (2) a higher default value on the order of minutes to hours sounds reasonable. In either case, the description of the property should clearly call out what it means and the impact of the settings being too low or too high for a given situation so users can decide whether they should specify an alternative for their case or not.
In no case should either of these be 'infinite' and we must ensure we limit how many things we track at once as it becomes a resource concern. If this is already accounted for then great.
---
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 pull request #980: NIFI-1170 - Improved TailFile processor to support m...
Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on a diff in the pull request:
https://github.com/apache/nifi/pull/980#discussion_r78437746
--- Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TailFile.java ---
@@ -245,58 +490,82 @@ private void recoverState(final ProcessContext context, final Map<String, String
+ "this indicates that the file has rotated. Will begin tailing current file from beginning.", new Object[]{existingTailFile.length(), position});
}
- state = new TailFileState(currentFilename, tailFile, reader, position, timestamp, checksum, ByteBuffer.allocate(65536));
+ states.get(filePath).setState(new TailFileState(filePath, tailFile, reader, position, timestamp, length, checksum, ByteBuffer.allocate(65536)));
} else {
- resetState(currentFilename);
+ resetState(filePath);
}
- getLogger().debug("Recovered state {}", new Object[]{state});
+ getLogger().debug("Recovered state {}", new Object[]{states.get(filePath).getState()});
}
- private void resetState(final String currentFilename) {
- expectedRecoveryChecksum = null;
- state = new TailFileState(currentFilename, null, null, 0L, 0L, null, ByteBuffer.allocate(65536));
+ private void resetState(final String filePath) {
+ states.get(filePath).setExpectedRecoveryChecksum(null);
+ states.get(filePath).setState(new TailFileState(filePath, null, null, 0L, 0L, 0L, null, ByteBuffer.allocate(65536)));
}
@OnStopped
public void cleanup() {
- final TailFileState state = this.state;
- if (state == null) {
+ for (TailFileObject tfo : states.values()) {
+ cleanReader(tfo);
+ final TailFileState state = tfo.getState();
+ tfo.setState(new TailFileState(state.getFilename(), state.getFile(), null, state.getPosition(), state.getTimestamp(), state.getLength(), state.getChecksum(), state.getBuffer()));
+ }
+ }
+
+ private void cleanReader(TailFileObject tfo) {
+ if (tfo.getState() == null) {
return;
}
- final FileChannel reader = state.getReader();
+ final FileChannel reader = tfo.getState().getReader();
if (reader == null) {
return;
}
try {
reader.close();
+ getLogger().debug("Closed FileChannel {}", new Object[]{reader});
} catch (final IOException ioe) {
getLogger().warn("Failed to close file handle during cleanup");
}
-
- getLogger().debug("Closed FileChannel {}", new Object[]{reader});
-
- this.state = new TailFileState(state.getFilename(), state.getFile(), null, state.getPosition(), state.getTimestamp(), state.getChecksum(), state.getBuffer());
}
@Override
public void onTrigger(final ProcessContext context, final ProcessSession session) throws ProcessException {
+ if(isMultiChanging.get()) {
+ long timeSinceLastLookup = new Date().getTime() - lastLookup.get();
+ if(timeSinceLastLookup > context.getProperty(LOOKUP_FREQUENCY).asTimePeriod(TimeUnit.MILLISECONDS)) {
+ try {
+ recoverState(context);
+ } catch (IOException e) {
+ getLogger().warn("Exception raised while looking up for new files", e);
--- End diff --
Good catch.
---
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.
---