You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by pasalkarsachin1 <gi...@git.apache.org> on 2017/02/22 06:25:46 UTC
[GitHub] storm pull request #1957: STORM-2373: HDFS Spout should support multiple ign...
GitHub user pasalkarsachin1 opened a pull request:
https://github.com/apache/storm/pull/1957
STORM-2373: HDFS Spout should support multiple ignore extensions
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/pasalkarsachin1/storm STORM-2373
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/1957.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 #1957
----
commit c15c647b9cc001b03f030762a13b4e64e2e5cbf3
Author: Sachin Pasalkar <sa...@symantec.com>
Date: 2017-02-22T06:24:18Z
STORM-2373: HDFS Spout should support multiple ignore extensions
----
---
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] storm pull request #1957: STORM-2373: HDFS Spout should support multiple ign...
Posted by roshannaik <gi...@git.apache.org>.
Github user roshannaik commented on a diff in the pull request:
https://github.com/apache/storm/pull/1957#discussion_r103150479
--- Diff: external/storm-hdfs/README.md ---
@@ -547,7 +548,7 @@ Only methods mentioned in **bold** are required.
| **.setArchiveDir()** |~~hdfsspout.archive.dir~~ | | After a file is processed completely it will be moved to this HDFS directory. If this directory does not exist it will be created. E.g. /data/done|
| **.setBadFilesDir()** |~~hdfsspout.badfiles.dir~~ | | if there is an error parsing a file's contents, the file is moved to this location. If this directory does not exist it will be created. E.g. /data/badfiles |
| .setLockDir() |~~hdfsspout.lock.dir~~ | '.lock' subdirectory under hdfsspout.source.dir | Dir in which lock files will be created. Concurrent HDFS spout instances synchronize using *lock* files. Before processing a file the spout instance creates a lock file in this directory with same name as input file and deletes this lock file after processing the file. Spouts also periodically makes a note of their progress (wrt reading the input file) in the lock file so that another spout instance can resume progress on the same file if the spout dies for any reason.|
-| .setIgnoreSuffix() |~~hdfsspout.ignore.suffix~~ | .ignore | File names with this suffix in the in the hdfsspout.source.dir location will not be processed|
+| .setIgnoreSuffix() |~~hdfsspout.ignore.suffix~~ | .ignore | You can provide comma separated suffixes. File names with this suffixes in the hdfsspout.source.dir location will not be processed|
--- End diff --
Suggest rephrasing the description to
"Comma separated list of file name suffixes. File with matching suffixes in the hdfsspout.source.dir location will not be processed"
---
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] storm issue #1957: STORM-2373: HDFS Spout should support multiple ignore ext...
Posted by roshannaik <gi...@git.apache.org>.
Github user roshannaik commented on the issue:
https://github.com/apache/storm/pull/1957
sure
---
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] storm pull request #1957: STORM-2373: HDFS Spout should support multiple ign...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/1957
---
[GitHub] storm issue #1957: STORM-2373: HDFS Spout should support multiple ignore ext...
Posted by roshannaik <gi...@git.apache.org>.
Github user roshannaik commented on the issue:
https://github.com/apache/storm/pull/1957
https://ariejan.net/2011/07/05/git-squash-your-latests-commits-into-one/
---
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] storm issue #1957: STORM-2373: HDFS Spout should support multiple ignore ext...
Posted by pasalkarsachin1 <gi...@git.apache.org>.
Github user pasalkarsachin1 commented on the issue:
https://github.com/apache/storm/pull/1957
Can I squash my changes then?
---
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] storm issue #1957: STORM-2373: HDFS Spout should support multiple ignore ext...
Posted by pasalkarsachin1 <gi...@git.apache.org>.
Github user pasalkarsachin1 commented on the issue:
https://github.com/apache/storm/pull/1957
@roshannaik Thanks for document. Can you merge 8da34caf4938e8a8cc5ac9c745000ed5c6f82b2a? Then I will close PR.
---
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] storm pull request #1957: STORM-2373: HDFS Spout should support multiple ign...
Posted by pasalkarsachin1 <gi...@git.apache.org>.
Github user pasalkarsachin1 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1957#discussion_r102632667
--- Diff: external/storm-hdfs/src/main/java/org/apache/storm/hdfs/spout/HdfsSpout.java ---
@@ -163,12 +164,19 @@ public HdfsSpout setClocksInSync(boolean clocksInSync) {
return this;
}
-
+ /**
+ * @deprecated use {@link #setIgnoreSuffix(List)} instead.
+ */
public HdfsSpout setIgnoreSuffix(String ignoreSuffix) {
--- End diff --
Done. Suffixes separated by ','
---
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] storm issue #1957: STORM-2373: HDFS Spout should support multiple ignore ext...
Posted by pasalkarsachin1 <gi...@git.apache.org>.
Github user pasalkarsachin1 commented on the issue:
https://github.com/apache/storm/pull/1957
@roshannaik @harshach Can you help me in merging it. Unfortunately I am not able to squash it :(
---
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] storm issue #1957: STORM-2373: HDFS Spout should support multiple ignore ext...
Posted by pasalkarsachin1 <gi...@git.apache.org>.
Github user pasalkarsachin1 commented on the issue:
https://github.com/apache/storm/pull/1957
@roshannaik My bad. I have corrected it.
---
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] storm issue #1957: STORM-2373: HDFS Spout should support multiple ignore ext...
Posted by pasalkarsachin1 <gi...@git.apache.org>.
Github user pasalkarsachin1 commented on the issue:
https://github.com/apache/storm/pull/1957
@roshannaik Can you again review it please?
---
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] storm pull request #1957: STORM-2373: HDFS Spout should support multiple ign...
Posted by roshannaik <gi...@git.apache.org>.
Github user roshannaik commented on a diff in the pull request:
https://github.com/apache/storm/pull/1957#discussion_r103299080
--- Diff: external/storm-hdfs/README.md ---
@@ -547,7 +548,7 @@ Only methods mentioned in **bold** are required.
| **.setArchiveDir()** |~~hdfsspout.archive.dir~~ | | After a file is processed completely it will be moved to this HDFS directory. If this directory does not exist it will be created. E.g. /data/done|
| **.setBadFilesDir()** |~~hdfsspout.badfiles.dir~~ | | if there is an error parsing a file's contents, the file is moved to this location. If this directory does not exist it will be created. E.g. /data/badfiles |
| .setLockDir() |~~hdfsspout.lock.dir~~ | '.lock' subdirectory under hdfsspout.source.dir | Dir in which lock files will be created. Concurrent HDFS spout instances synchronize using *lock* files. Before processing a file the spout instance creates a lock file in this directory with same name as input file and deletes this lock file after processing the file. Spouts also periodically makes a note of their progress (wrt reading the input file) in the lock file so that another spout instance can resume progress on the same file if the spout dies for any reason.|
-| .setIgnoreSuffix() |~~hdfsspout.ignore.suffix~~ | .ignore | File names with this suffix in the in the hdfsspout.source.dir location will not be processed|
+| .setIgnoreSuffix() |~~hdfsspout.ignore.suffix~~ | .ignore | Comma separated list of file name suffixes. File with matching suffixes in the hdfsspout.source.dir location will not be processed |
--- End diff --
A minor correction ... it should say "File**s** with matching suffixes". Looks good otherwise.
---
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] storm issue #1957: STORM-2373: HDFS Spout should support multiple ignore ext...
Posted by pasalkarsachin1 <gi...@git.apache.org>.
Github user pasalkarsachin1 commented on the issue:
https://github.com/apache/storm/pull/1957
@roshannaik Done
---
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] storm issue #1957: STORM-2373: HDFS Spout should support multiple ignore ext...
Posted by roshannaik <gi...@git.apache.org>.
Github user roshannaik commented on the issue:
https://github.com/apache/storm/pull/1957
@pasalkarsachin1 its unclear what you have done with the latest (13th) commit.
What you need is the first 12 commits squashed into 1. That will leave you with 1 commit only (and not 13 like you have now). I think you may need to get rid of this latest 13th commit before you squash the remaining 12 ... to get this right.
---
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] storm issue #1957: STORM-2373: HDFS Spout should support multiple ignore ext...
Posted by pasalkarsachin1 <gi...@git.apache.org>.
Github user pasalkarsachin1 commented on the issue:
https://github.com/apache/storm/pull/1957
Can someone take a look at this?
---
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] storm pull request #1957: STORM-2373: HDFS Spout should support multiple ign...
Posted by roshannaik <gi...@git.apache.org>.
Github user roshannaik commented on a diff in the pull request:
https://github.com/apache/storm/pull/1957#discussion_r103149784
--- Diff: external/storm-hdfs/src/main/java/org/apache/storm/hdfs/spout/HdfsSpout.java ---
@@ -447,7 +449,12 @@ public void open(Map conf, TopologyContext context, SpoutOutputCollector collect
// -- ignore file names config
if ( conf.containsKey(Configs.IGNORE_SUFFIX) ) {
- this.ignoreSuffix = conf.get(Configs.IGNORE_SUFFIX).toString();
+ ignoreSuffixes = Arrays.asList(conf.get(Configs.IGNORE_SUFFIX).toString().split(","));
--- End diff --
This wont handle inputs like: ".ignore1, .ignore2 ,.ignore3" where user may put one or more spaces before/after the comma. We should allow for that. Also would be nice to trim off any spaces at the start and end of the entire string.
---
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] storm issue #1957: STORM-2373: HDFS Spout should support multiple ignore ext...
Posted by pasalkarsachin1 <gi...@git.apache.org>.
Github user pasalkarsachin1 commented on the issue:
https://github.com/apache/storm/pull/1957
@harshach I have already squash it in 8da34ca
---
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] storm pull request #1957: STORM-2373: HDFS Spout should support multiple ign...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on a diff in the pull request:
https://github.com/apache/storm/pull/1957#discussion_r103069440
--- Diff: external/storm-hdfs/src/main/java/org/apache/storm/hdfs/common/HdfsUtils.java ---
@@ -76,6 +76,51 @@ public static FSDataOutputStream tryCreateFile(FileSystem fs, Path file) throws
}
}
}
+ /**
+ * list files sorted by modification time that have not been modified since
+ * 'olderThan'. if 'olderThan' is <= 0 then the filtering is disabled
+ *
+ * @param fs
+ * - {@link FileSystem}
+ * @param directory
+ * - Directory in which it will look for
+ * @param olderThan
+ * - Files updated olderthan this time
+ * @param ignoreSuffixes
+ * - List of ignoreSuffixes
+ * @return - List file path satisfied by criteria
+ * @throws IOException
+ */
+ public static ArrayList<Path> listFilesByModificationTimeWithIgnoreSuffixes(FileSystem fs, Path directory,
+ long olderThan, List<String> ignoreSuffixes) throws IOException {
+
+ ArrayList<Path> list = listFilesByModificationTime(fs, directory, olderThan);
+ ArrayList<Path> result = new ArrayList<>(list.size());
+ for (Path path : list) {
+ if (!filterSufix(path.getName(), ignoreSuffixes)) {
+ result.add(path);
+ }
+ }
+ return result;
+ }
+
+ /**
+ *
+ * @param name
+ * -name of file
+ * @param ignoreSuffixes
+ * - List of suffixes to be ignored
+ * @return
+ */
+ private static boolean filterSufix(String name, List<String> ignoreSuffixes) {
+
--- End diff --
same here.
---
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] storm issue #1957: STORM-2373: HDFS Spout should support multiple ignore ext...
Posted by pasalkarsachin1 <gi...@git.apache.org>.
Github user pasalkarsachin1 commented on the issue:
https://github.com/apache/storm/pull/1957
@roshannaik Can you help m in squash this? I am not able to do it :(
---
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] storm issue #1957: STORM-2373: HDFS Spout should support multiple ignore ext...
Posted by roshannaik <gi...@git.apache.org>.
Github user roshannaik commented on the issue:
https://github.com/apache/storm/pull/1957
You made the fix in the wrong place. The sentence should read
"Comma separated list of file name suffixes. Files with matching suffixes in the hdfsspout.source.dir location will not be processed ".
---
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] storm issue #1957: STORM-2373: HDFS Spout should support multiple ignore ext...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/1957
@pasalkarsachin1 I am going to merge it in but before can you squash your commits into a single commit.
---
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] storm pull request #1957: STORM-2373: HDFS Spout should support multiple ign...
Posted by roshannaik <gi...@git.apache.org>.
Github user roshannaik commented on a diff in the pull request:
https://github.com/apache/storm/pull/1957#discussion_r102632179
--- Diff: external/storm-hdfs/src/main/java/org/apache/storm/hdfs/spout/HdfsSpout.java ---
@@ -163,12 +164,19 @@ public HdfsSpout setClocksInSync(boolean clocksInSync) {
return this;
}
-
+ /**
+ * @deprecated use {@link #setIgnoreSuffix(List)} instead.
+ */
public HdfsSpout setIgnoreSuffix(String ignoreSuffix) {
--- End diff --
No needed to deprecate old method and add new one. You just extend the existing method to accept a space separate list of file extensions. That would preserve compatibility and extend it at the same time. conversion to an array can be done internally.
---
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] storm issue #1957: STORM-2373: HDFS Spout should support multiple ignore ext...
Posted by roshannaik <gi...@git.apache.org>.
Github user roshannaik commented on the issue:
https://github.com/apache/storm/pull/1957
+1
Thanks for the contribution.
---
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] storm issue #1957: STORM-2373: HDFS Spout should support multiple ignore ext...
Posted by pasalkarsachin1 <gi...@git.apache.org>.
Github user pasalkarsachin1 commented on the issue:
https://github.com/apache/storm/pull/1957
Can someone merge it?
---
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] storm issue #1957: STORM-2373: HDFS Spout should support multiple ignore ext...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/1957
@pasalkarsachin1 this PR still shows commits as 13. you can do git rebase -i HEAD~13 on this branch and squash commits and do a force push to same branch. I don't want to cherry-pick single commit. It will cleaner if we do this via PR.
---
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] storm issue #1957: STORM-2373: HDFS Spout should support multiple ignore ext...
Posted by pasalkarsachin1 <gi...@git.apache.org>.
Github user pasalkarsachin1 commented on the issue:
https://github.com/apache/storm/pull/1957
@roshannaik I have added test case too. We have running this code in our staging environment
---
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] storm issue #1957: STORM-2373: HDFS Spout should support multiple ignore ext...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/1957
@pasalkarsachin1 looks good. Minor nits. Also update the doc here with new config changes https://github.com/apache/storm/tree/master/external/storm-hdfs#hdfs-spout
---
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] storm pull request #1957: STORM-2373: HDFS Spout should support multiple ign...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on a diff in the pull request:
https://github.com/apache/storm/pull/1957#discussion_r103069438
--- Diff: external/storm-hdfs/src/main/java/org/apache/storm/hdfs/common/HdfsUtils.java ---
@@ -76,6 +76,51 @@ public static FSDataOutputStream tryCreateFile(FileSystem fs, Path file) throws
}
}
}
+ /**
+ * list files sorted by modification time that have not been modified since
+ * 'olderThan'. if 'olderThan' is <= 0 then the filtering is disabled
+ *
+ * @param fs
+ * - {@link FileSystem}
+ * @param directory
+ * - Directory in which it will look for
+ * @param olderThan
+ * - Files updated olderthan this time
+ * @param ignoreSuffixes
+ * - List of ignoreSuffixes
+ * @return - List file path satisfied by criteria
+ * @throws IOException
+ */
+ public static ArrayList<Path> listFilesByModificationTimeWithIgnoreSuffixes(FileSystem fs, Path directory,
+ long olderThan, List<String> ignoreSuffixes) throws IOException {
+
--- End diff --
remove empty line.
---
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] storm issue #1957: STORM-2373: HDFS Spout should support multiple ignore ext...
Posted by pasalkarsachin1 <gi...@git.apache.org>.
Github user pasalkarsachin1 commented on the issue:
https://github.com/apache/storm/pull/1957
@harshach Done. Thanks for review.
---
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.
---