STORM-2373: HDFS Spout should support multiple ignore extensions by pasalkarsachin1 · Pull Request #1957 · apache/storm

pasalkarsachin1 · 2017-02-22T06:25:42Z

This changes is just to support multiple ignore extension for below cases

Incase there is another distcp process which copies data to source directory & creates extension of _COPYING_
User have different file extensions which are not supported by reader

pasalkarsachin1 · 2017-02-23T03:13:45Z

Can someone take a look at this?

roshannaik · 2017-02-23T03:20:49Z

+ /**
+	* @deprecated use {@link #setIgnoreSuffix(List)} instead.
+	*/
 public HdfsSpout setIgnoreSuffix(String ignoreSuffix) {


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.

Done. Suffixes separated by ','

pasalkarsachin1 · 2017-02-24T12:29:23Z

@roshannaik Can you again review it please?

harshach · 2017-02-25T03:16:42Z

+ */
+ public static ArrayList<Path> listFilesByModificationTimeWithIgnoreSuffixes(FileSystem fs, Path directory,
+ long olderThan, List<String> ignoreSuffixes) throws IOException {
+


remove empty line.

harshach · 2017-02-25T03:16:49Z

+ * @return
+ */
+ private static boolean filterSufix(String name, List<String> ignoreSuffixes) {
+


harshach · 2017-02-25T03:19:09Z

@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

pasalkarsachin1 · 2017-02-25T04:36:24Z

@harshach Done. Thanks for review.

roshannaik

Have you been able to do any manual testing to validate this ? Would be nice to include a UT.

roshannaik · 2017-02-27T07:49:14Z

 // -- 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(","));


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.

roshannaik · 2017-02-27T07:55:17Z

 | **.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|


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"

pasalkarsachin1 · 2017-02-27T17:38:58Z

@roshannaik I have added test case too. We have running this code in our staging environment

roshannaik · 2017-02-27T20:05:40Z

 | **.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|


A minor correction ... it should say "Files with matching suffixes". Looks good otherwise.

pasalkarsachin1 · 2017-02-27T20:16:49Z

@roshannaik Done

roshannaik · 2017-02-27T20:47:52Z

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 ".

pasalkarsachin1 · 2017-02-28T03:43:17Z

@roshannaik My bad. I have corrected it.

roshannaik · 2017-02-28T06:39:22Z

+1
Thanks for the contribution.

pasalkarsachin1 · 2017-02-28T11:33:53Z

Can I squash my changes then?

roshannaik · 2017-02-28T19:46:20Z

sure

updated as per comment

…into STORM-2373

pasalkarsachin1 · 2017-03-01T18:52:05Z

@roshannaik Can you help me in squash this? I am not able to do it :(

roshannaik · 2017-03-02T00:58:48Z

https://ariejan.net/2011/07/05/git-squash-your-latests-commits-into-one/

This changes is just to support multiple ignore extension for below cases Incase there is another distcp process which copies data to source directory & creates extension of _COPYING_ User have different file extensions which are not supported by reader

…into STORM-2373

pasalkarsachin1 · 2017-03-02T02:40:30Z

@roshannaik Thanks for document. Can you merge 8da34ca? Then I will close PR.

pasalkarsachin1 · 2017-03-04T16:31:41Z

Can someone merge it?

harshach · 2017-03-07T00:09:58Z

@pasalkarsachin1 I am going to merge it in but before can you squash your commits into a single commit.

pasalkarsachin1 · 2017-03-07T04:31:37Z

@harshach I have already squash it in 8da34ca

harshach · 2017-03-07T21:04:51Z

@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.

roshannaik · 2017-03-07T22:01:56Z

@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.

pasalkarsachin1 · 2017-03-16T17:21:28Z

@roshannaik @harshach Can you help me in merging it. Unfortunately I am not able to squash it :(

We are closing stale Pull Requests to make the list more manageable. Please re-open any Pull Request that has been closed in error. Closesapache#608Closesapache#639Closesapache#640Closesapache#648Closesapache#662Closesapache#668Closesapache#692Closesapache#705Closesapache#724Closesapache#728Closesapache#730Closesapache#753Closesapache#803Closesapache#854Closesapache#922Closesapache#986Closesapache#992Closesapache#1019Closesapache#1040Closesapache#1041Closesapache#1043Closesapache#1046Closesapache#1051Closesapache#1078Closesapache#1146Closesapache#1164Closesapache#1165Closesapache#1178Closesapache#1213Closesapache#1225Closesapache#1258Closesapache#1259Closesapache#1268Closesapache#1272Closesapache#1277Closesapache#1278Closesapache#1288Closesapache#1296Closesapache#1328Closesapache#1342Closesapache#1353Closesapache#1370Closesapache#1376Closesapache#1391Closesapache#1395Closesapache#1399Closesapache#1406Closesapache#1410Closesapache#1422Closesapache#1427Closesapache#1443Closesapache#1462Closesapache#1468Closesapache#1483Closesapache#1506Closesapache#1509Closesapache#1515Closesapache#1520Closesapache#1521Closesapache#1525Closesapache#1527Closesapache#1544Closesapache#1550Closesapache#1566Closesapache#1569Closesapache#1570Closesapache#1575Closesapache#1580Closesapache#1584Closesapache#1591Closesapache#1600Closesapache#1611Closesapache#1613Closesapache#1639Closesapache#1703Closesapache#1711Closesapache#1719Closesapache#1737Closesapache#1760Closesapache#1767Closesapache#1768Closesapache#1785Closesapache#1799Closesapache#1822Closesapache#1824Closesapache#1844Closesapache#1874Closesapache#1918Closesapache#1928Closesapache#1937Closesapache#1942Closesapache#1951Closesapache#1957Closesapache#1963Closesapache#1964Closesapache#1965Closesapache#1967Closesapache#1968Closesapache#1971Closesapache#1985Closesapache#1986Closesapache#1998Closesapache#2031Closesapache#2032Closesapache#2071Closesapache#2076Closesapache#2108Closesapache#2119Closesapache#2128Closesapache#2142Closesapache#2174Closesapache#2206Closesapache#2297Closesapache#2322Closesapache#2332Closesapache#2341Closesapache#2377Closesapache#2414Closesapache#2469

Sachin Pasalkar added 2 commits February 22, 2017 11:54

STORM-2373: HDFS Spout should support multiple ignore extensions
c15c647

Updated code to support backward compatibility better
6372413

roshannaik requested changes Feb 23, 2017
View reviewed changes

Sachin Pasalkarand others added 2 commits February 23, 2017 08:55

Updated as per comments
f3515f9

Fixed indentation issue
df727df

harshach reviewed Feb 25, 2017
View reviewed changes

As per comments
f97a976

roshannaik requested changes Feb 27, 2017
View reviewed changes

Sachin Pasalkar added 2 commits February 27, 2017 15:40

Updated as per comments
4a87678

Added test case
6d5d728

roshannaik reviewed Feb 27, 2017
View reviewed changes

Updated as per comment
813bedf

updated as per comment
e007005

Sachin Pasalkar added 2 commits March 2, 2017 00:06

HDFS Spout should support multiple ignore extensions
09a4b08
updated as per comment

Merge branch 'STORM-2373' ofhttps://github.com/pasalkarsachin1/storm…
747af9b
…into STORM-2373

Merge branch 'STORM-2373' ofhttps://github.com/pasalkarsachin1/storm…
48a176e
…into STORM-2373

d2r mentioned this pull request Oct 16, 2018
STORM-3250: Closes Pull Requests unchanged in 2018 #2880
Merged

asfgit closed this in #2880 Oct 22, 2018

Uh oh!

Conversation

pasalkarsachin1 commented Feb 22, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pasalkarsachin1 commented Feb 23, 2017

Uh oh!

roshannaikFeb 23, 2017

Choose a reason for hiding this comment

Uh oh!

pasalkarsachin1Feb 23, 2017

Choose a reason for hiding this comment

Uh oh!

pasalkarsachin1 commented Feb 24, 2017

Uh oh!

harshachFeb 25, 2017

Choose a reason for hiding this comment

Uh oh!

harshachFeb 25, 2017

Choose a reason for hiding this comment

Uh oh!

harshach commented Feb 25, 2017

Uh oh!

pasalkarsachin1 commented Feb 25, 2017

Uh oh!

roshannaik left a comment

Choose a reason for hiding this comment

Uh oh!

roshannaikFeb 27, 2017

Choose a reason for hiding this comment

Uh oh!

roshannaikFeb 27, 2017

Choose a reason for hiding this comment

Uh oh!

pasalkarsachin1 commented Feb 27, 2017

Uh oh!

roshannaikFeb 27, 2017

Choose a reason for hiding this comment

Uh oh!

pasalkarsachin1 commented Feb 27, 2017

Uh oh!

roshannaik commented Feb 27, 2017

Uh oh!

pasalkarsachin1 commented Feb 28, 2017

Uh oh!

roshannaik commented Feb 28, 2017

Uh oh!

pasalkarsachin1 commented Feb 28, 2017

Uh oh!

roshannaik commented Feb 28, 2017

Uh oh!

pasalkarsachin1 commented Mar 1, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

roshannaik commented Mar 2, 2017

Uh oh!

pasalkarsachin1 commented Mar 2, 2017

Uh oh!

pasalkarsachin1 commented Mar 4, 2017

Uh oh!

harshach commented Mar 7, 2017

Uh oh!

pasalkarsachin1 commented Mar 7, 2017

Uh oh!

harshach commented Mar 7, 2017

Uh oh!

roshannaik commented Mar 7, 2017

Uh oh!

pasalkarsachin1 commented Mar 16, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pasalkarsachin1 commented Feb 22, 2017•
edited
Loading

pasalkarsachin1 commented Mar 1, 2017•
edited
Loading