You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flume.apache.org by hun shenshi <28...@qq.com> on 2017/06/02 10:09:16 UTC

Review Request 59743: FLUME-3101. When TaildirSource read the file written by high frequency, would not break out to read another file.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59743/
-----------------------------------------------------------

Review request for Flume.


Bugs: FLUME-3101
    https://issues.apache.org/jira/browse/FLUME-3101


Repository: flume-git


Description
-------

Add a threshold in TaildirSource. Use the threshold to control the number of reads when a file is written by high frequency.


Diffs
-----

  flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSource.java a107a01 


Diff: https://reviews.apache.org/r/59743/diff/1/


Testing
-------

mvn clean install -DskipTests -> built


Thanks,

hun shenshi


Re: Review Request 59743: FLUME-3101. When TaildirSource read the file written by high frequency, would not break out to read another file.

Posted by hun shenshi <28...@qq.com>.

> On 六月 2, 2017, 1:41 p.m., Miklos Csanady wrote:
> > flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSource.java
> > Line 276 (original), 278 (patched)
> > <https://reviews.apache.org/r/59743/diff/1/?file=1740249#file1740249line278>
> >
> >     If the readCount reaches numTreshold we could log a warning. 
> >     Or it can trigger a metrics counter.
> 
> hun shenshi wrote:
>     ok, I will log a warning.

I think log a warning is better


- hun


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59743/#review176765
-----------------------------------------------------------


On 六月 2, 2017, 10:09 a.m., hun shenshi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59743/
> -----------------------------------------------------------
> 
> (Updated 六月 2, 2017, 10:09 a.m.)
> 
> 
> Review request for Flume.
> 
> 
> Bugs: FLUME-3101
>     https://issues.apache.org/jira/browse/FLUME-3101
> 
> 
> Repository: flume-git
> 
> 
> Description
> -------
> 
> Add a threshold in TaildirSource. Use the threshold to control the number of reads when a file is written by high frequency.
> 
> 
> Diffs
> -----
> 
>   flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSource.java a107a01 
> 
> 
> Diff: https://reviews.apache.org/r/59743/diff/1/
> 
> 
> Testing
> -------
> 
> mvn clean install -DskipTests -> built
> 
> 
> Thanks,
> 
> hun shenshi
> 
>


Re: Review Request 59743: FLUME-3101. When TaildirSource read the file written by high frequency, would not break out to read another file.

Posted by hun shenshi <28...@qq.com>.

> On 六月 2, 2017, 1:41 p.m., Miklos Csanady wrote:
> > flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSource.java
> > Line 273 (original), 275 (patched)
> > <https://reviews.apache.org/r/59743/diff/1/?file=1740249#file1740249line275>
> >
> >     I would prefer to make it configurable.
> >     1000 can be a default.

It should be numThreshold?not retryInterval. Right?


> On 六月 2, 2017, 1:41 p.m., Miklos Csanady wrote:
> > flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSource.java
> > Line 276 (original), 278 (patched)
> > <https://reviews.apache.org/r/59743/diff/1/?file=1740249#file1740249line278>
> >
> >     If the readCount reaches numTreshold we could log a warning. 
> >     Or it can trigger a metrics counter.

ok, I will log a warning.


On 六月 2, 2017, 1:41 p.m., hun shenshi wrote:
> > Do you have an idea how to cover this case with a test?
> > 
> > Thank you for this nice contribution.

ok. I will think about it.


- hun


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59743/#review176765
-----------------------------------------------------------


On 六月 2, 2017, 10:09 a.m., hun shenshi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59743/
> -----------------------------------------------------------
> 
> (Updated 六月 2, 2017, 10:09 a.m.)
> 
> 
> Review request for Flume.
> 
> 
> Bugs: FLUME-3101
>     https://issues.apache.org/jira/browse/FLUME-3101
> 
> 
> Repository: flume-git
> 
> 
> Description
> -------
> 
> Add a threshold in TaildirSource. Use the threshold to control the number of reads when a file is written by high frequency.
> 
> 
> Diffs
> -----
> 
>   flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSource.java a107a01 
> 
> 
> Diff: https://reviews.apache.org/r/59743/diff/1/
> 
> 
> Testing
> -------
> 
> mvn clean install -DskipTests -> built
> 
> 
> Thanks,
> 
> hun shenshi
> 
>


Re: Review Request 59743: FLUME-3101. When TaildirSource read the file written by high frequency, would not break out to read another file.

Posted by Miklos Csanady <mi...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59743/#review176765
-----------------------------------------------------------



Thank you for this bugfix. 
I understand that this change can ease the blocking nature of one "over used" file in this taildir-source.
This cannot solve the overload, that's why we should somehow give a feedback of this lack of resources. 
If the reshold is high enough, the log will not triggered too often I think.


flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSource.java
Line 273 (original), 275 (patched)
<https://reviews.apache.org/r/59743/#comment250233>

    I would prefer to make it configurable.
    1000 can be a default.



flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSource.java
Line 276 (original), 278 (patched)
<https://reviews.apache.org/r/59743/#comment250235>

    If the readCount reaches numTreshold we could log a warning. 
    Or it can trigger a metrics counter.


Do you have an idea how to cover this case with a test?

Thank you for this nice contribution.

- Miklos Csanady


On June 2, 2017, 10:09 a.m., hun shenshi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59743/
> -----------------------------------------------------------
> 
> (Updated June 2, 2017, 10:09 a.m.)
> 
> 
> Review request for Flume.
> 
> 
> Bugs: FLUME-3101
>     https://issues.apache.org/jira/browse/FLUME-3101
> 
> 
> Repository: flume-git
> 
> 
> Description
> -------
> 
> Add a threshold in TaildirSource. Use the threshold to control the number of reads when a file is written by high frequency.
> 
> 
> Diffs
> -----
> 
>   flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSource.java a107a01 
> 
> 
> Diff: https://reviews.apache.org/r/59743/diff/1/
> 
> 
> Testing
> -------
> 
> mvn clean install -DskipTests -> built
> 
> 
> Thanks,
> 
> hun shenshi
> 
>


Re: Review Request 59743: FLUME-3101. When TaildirSource read the file written by high frequency, would not break out to read another file.

Posted by Peter Turcsanyi <tu...@gmail.com>.

> On Nov. 21, 2018, 5:21 p.m., Peter Turcsanyi wrote:
> >

As there was no answer to the previous comments, I took over the issue and opened a PR (based on this patch + applying the review changes): 
https://github.com/apache/flume/pull/240


- Peter


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59743/#review210769
-----------------------------------------------------------


On June 8, 2017, 4:16 p.m., hun shenshi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59743/
> -----------------------------------------------------------
> 
> (Updated June 8, 2017, 4:16 p.m.)
> 
> 
> Review request for Flume.
> 
> 
> Bugs: FLUME-3101
>     https://issues.apache.org/jira/browse/FLUME-3101
> 
> 
> Repository: flume-git
> 
> 
> Description
> -------
> 
> Add a threshold in TaildirSource. Use the threshold to control the number of reads when a file is written by high frequency.
> 
> 
> Diffs
> -----
> 
>   flume-ng-doc/sphinx/FlumeUserGuide.rst a5d64f0 
>   flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSource.java a107a01 
>   flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSourceConfigurationConstants.java f2347f3 
>   flume-ng-sources/flume-taildir-source/src/test/java/org/apache/flume/source/taildir/TestTaildirSource.java 097ee0b 
> 
> 
> Diff: https://reviews.apache.org/r/59743/diff/5/
> 
> 
> Testing
> -------
> 
> mvn clean install -DskipTests -> built
> junit tests for flume-taildir-source module -> passed
> 
> 
> Thanks,
> 
> hun shenshi
> 
>


Re: Review Request 59743: FLUME-3101. When TaildirSource read the file written by high frequency, would not break out to read another file.

Posted by Peter Turcsanyi <tu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59743/#review210769
-----------------------------------------------------------




flume-ng-sources/flume-taildir-source/src/test/java/org/apache/flume/source/taildir/TestTaildirSource.java
Lines 361 (patched)
<https://reviews.apache.org/r/59743/#comment295526>

    The processing order of the files is not deterministic, so the test fails if the order is [file2, file1].


- Peter Turcsanyi


On June 8, 2017, 4:16 p.m., hun shenshi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59743/
> -----------------------------------------------------------
> 
> (Updated June 8, 2017, 4:16 p.m.)
> 
> 
> Review request for Flume.
> 
> 
> Bugs: FLUME-3101
>     https://issues.apache.org/jira/browse/FLUME-3101
> 
> 
> Repository: flume-git
> 
> 
> Description
> -------
> 
> Add a threshold in TaildirSource. Use the threshold to control the number of reads when a file is written by high frequency.
> 
> 
> Diffs
> -----
> 
>   flume-ng-doc/sphinx/FlumeUserGuide.rst a5d64f0 
>   flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSource.java a107a01 
>   flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSourceConfigurationConstants.java f2347f3 
>   flume-ng-sources/flume-taildir-source/src/test/java/org/apache/flume/source/taildir/TestTaildirSource.java 097ee0b 
> 
> 
> Diff: https://reviews.apache.org/r/59743/diff/5/
> 
> 
> Testing
> -------
> 
> mvn clean install -DskipTests -> built
> junit tests for flume-taildir-source module -> passed
> 
> 
> Thanks,
> 
> hun shenshi
> 
>


Re: Review Request 59743: FLUME-3101. When TaildirSource read the file written by high frequency, would not break out to read another file.

Posted by Peter Turcsanyi <tu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59743/#review210523
-----------------------------------------------------------



Thank you for the contribution!
This is quite a useful feature, because a file which is being written by high frequency can cause a problem at shutdown too: when Taildir source is continouosly reading a file, it can not answer the shutdown request, so clean shutdown is not possible in this case.

Please find my review comments below that mostly affect the usability (config parameter name / description, logging).


flume-ng-doc/sphinx/FlumeUserGuide.rst
Lines 1189 (patched)
<https://reviews.apache.org/r/59743/#comment295232>

    As Taildir Source reads the files line-by-line, "number of reads" would rather mean the _number of lines_ for me, but actually it controls the _number or batches_ being read consecutively. To make it clear, I would modify the description something like this: "...the number of consecutive batch reads...".
    
    The name of the parameter does not describe what threshold it is about. "maxBatchReads" or similar would be better.



flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSource.java
Lines 261 (patched)
<https://reviews.apache.org/r/59743/#comment295233>

    According to the suggested `maxBatchReads` parameter name: `bacthReadCount`



flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSource.java
Line 276 (original), 284 (patched)
<https://reviews.apache.org/r/59743/#comment295230>

    I would handle these two cases separately. In that case the error message would be more straightforward.
    As both cases are normal, info log level is fair enough.


- Peter Turcsanyi


On June 8, 2017, 4:16 p.m., hun shenshi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59743/
> -----------------------------------------------------------
> 
> (Updated June 8, 2017, 4:16 p.m.)
> 
> 
> Review request for Flume.
> 
> 
> Bugs: FLUME-3101
>     https://issues.apache.org/jira/browse/FLUME-3101
> 
> 
> Repository: flume-git
> 
> 
> Description
> -------
> 
> Add a threshold in TaildirSource. Use the threshold to control the number of reads when a file is written by high frequency.
> 
> 
> Diffs
> -----
> 
>   flume-ng-doc/sphinx/FlumeUserGuide.rst a5d64f0 
>   flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSource.java a107a01 
>   flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSourceConfigurationConstants.java f2347f3 
>   flume-ng-sources/flume-taildir-source/src/test/java/org/apache/flume/source/taildir/TestTaildirSource.java 097ee0b 
> 
> 
> Diff: https://reviews.apache.org/r/59743/diff/5/
> 
> 
> Testing
> -------
> 
> mvn clean install -DskipTests -> built
> junit tests for flume-taildir-source module -> passed
> 
> 
> Thanks,
> 
> hun shenshi
> 
>


Re: Review Request 59743: FLUME-3101. When TaildirSource read the file written by high frequency, would not break out to read another file.

Posted by hun shenshi <28...@qq.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59743/
-----------------------------------------------------------

(Updated 六月 8, 2017, 2:16 p.m.)


Review request for Flume.


Bugs: FLUME-3101
    https://issues.apache.org/jira/browse/FLUME-3101


Repository: flume-git


Description
-------

Add a threshold in TaildirSource. Use the threshold to control the number of reads when a file is written by high frequency.


Diffs (updated)
-----

  flume-ng-doc/sphinx/FlumeUserGuide.rst a5d64f0 
  flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSource.java a107a01 
  flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSourceConfigurationConstants.java f2347f3 
  flume-ng-sources/flume-taildir-source/src/test/java/org/apache/flume/source/taildir/TestTaildirSource.java 097ee0b 


Diff: https://reviews.apache.org/r/59743/diff/5/

Changes: https://reviews.apache.org/r/59743/diff/4-5/


Testing
-------

mvn clean install -DskipTests -> built
junit tests for flume-taildir-source module -> passed


Thanks,

hun shenshi


Re: Review Request 59743: FLUME-3101. When TaildirSource read the file written by high frequency, would not break out to read another file.

Posted by hun shenshi <28...@qq.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59743/
-----------------------------------------------------------

(Updated 六月 8, 2017, 3:33 a.m.)


Review request for Flume.


Bugs: FLUME-3101
    https://issues.apache.org/jira/browse/FLUME-3101


Repository: flume-git


Description
-------

Add a threshold in TaildirSource. Use the threshold to control the number of reads when a file is written by high frequency.


Diffs (updated)
-----

  flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSource.java a107a01 
  flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSourceConfigurationConstants.java f2347f3 
  flume-ng-sources/flume-taildir-source/src/test/java/org/apache/flume/source/taildir/TestTaildirSource.java 097ee0b 


Diff: https://reviews.apache.org/r/59743/diff/4/

Changes: https://reviews.apache.org/r/59743/diff/3-4/


Testing
-------

mvn clean install -DskipTests -> built
junit tests for flume-taildir-source module -> passed


Thanks,

hun shenshi


Re: Review Request 59743: FLUME-3101. When TaildirSource read the file written by high frequency, would not break out to read another file.

Posted by hun shenshi <28...@qq.com>.

> On 六月 7, 2017, 4:41 p.m., Denes Arvay wrote:
> > Thank you for the patch.
> > In general I think if this problem occurs then the load is higher what the actual Flume setup can handle, so the files won't be fully read unless the load decreases. On the other hand it might be good for some use cases to process other files instead of working on only one, so all in all I agree with the change.
> > 
> > I had some comments, could you please fix those issues?
> > Plus it'd be great if you could add this new configuration parameter to the User Guide as well. (`flume-ng-doc/sphinx/FlumeUserGuide.rst`, to check your changes you can generate the html files with the following commands:
> > ```
> > $ export LC_ALL=C.UTF-8 # Required to build the javadocs on some platforms and in some locales
> > $ mvn clean install -Psite -DskipTests
> > ```
> > The output will be placed in the `target/site` directory.
> > 
> > Let me know if you have any concerns regarding to my comments.
> 
> hun shenshi wrote:
>     ok, I will fix those issues

Thank you for your comments.
I have fix those issues.
Please review.
Thank you.


- hun


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59743/#review177175
-----------------------------------------------------------


On 六月 8, 2017, 2:16 p.m., hun shenshi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59743/
> -----------------------------------------------------------
> 
> (Updated 六月 8, 2017, 2:16 p.m.)
> 
> 
> Review request for Flume.
> 
> 
> Bugs: FLUME-3101
>     https://issues.apache.org/jira/browse/FLUME-3101
> 
> 
> Repository: flume-git
> 
> 
> Description
> -------
> 
> Add a threshold in TaildirSource. Use the threshold to control the number of reads when a file is written by high frequency.
> 
> 
> Diffs
> -----
> 
>   flume-ng-doc/sphinx/FlumeUserGuide.rst a5d64f0 
>   flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSource.java a107a01 
>   flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSourceConfigurationConstants.java f2347f3 
>   flume-ng-sources/flume-taildir-source/src/test/java/org/apache/flume/source/taildir/TestTaildirSource.java 097ee0b 
> 
> 
> Diff: https://reviews.apache.org/r/59743/diff/5/
> 
> 
> Testing
> -------
> 
> mvn clean install -DskipTests -> built
> junit tests for flume-taildir-source module -> passed
> 
> 
> Thanks,
> 
> hun shenshi
> 
>


Re: Review Request 59743: FLUME-3101. When TaildirSource read the file written by high frequency, would not break out to read another file.

Posted by hun shenshi <28...@qq.com>.

> On 六月 7, 2017, 4:41 p.m., Denes Arvay wrote:
> > Thank you for the patch.
> > In general I think if this problem occurs then the load is higher what the actual Flume setup can handle, so the files won't be fully read unless the load decreases. On the other hand it might be good for some use cases to process other files instead of working on only one, so all in all I agree with the change.
> > 
> > I had some comments, could you please fix those issues?
> > Plus it'd be great if you could add this new configuration parameter to the User Guide as well. (`flume-ng-doc/sphinx/FlumeUserGuide.rst`, to check your changes you can generate the html files with the following commands:
> > ```
> > $ export LC_ALL=C.UTF-8 # Required to build the javadocs on some platforms and in some locales
> > $ mvn clean install -Psite -DskipTests
> > ```
> > The output will be placed in the `target/site` directory.
> > 
> > Let me know if you have any concerns regarding to my comments.

ok, I will fix those issues


- hun


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59743/#review177175
-----------------------------------------------------------


On 六月 4, 2017, 2:23 p.m., hun shenshi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59743/
> -----------------------------------------------------------
> 
> (Updated 六月 4, 2017, 2:23 p.m.)
> 
> 
> Review request for Flume.
> 
> 
> Bugs: FLUME-3101
>     https://issues.apache.org/jira/browse/FLUME-3101
> 
> 
> Repository: flume-git
> 
> 
> Description
> -------
> 
> Add a threshold in TaildirSource. Use the threshold to control the number of reads when a file is written by high frequency.
> 
> 
> Diffs
> -----
> 
>   flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSource.java a107a01 
>   flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSourceConfigurationConstants.java f2347f3 
>   flume-ng-sources/flume-taildir-source/src/test/java/org/apache/flume/source/taildir/TestTaildirSource.java 097ee0b 
> 
> 
> Diff: https://reviews.apache.org/r/59743/diff/3/
> 
> 
> Testing
> -------
> 
> mvn clean install -DskipTests -> built
> junit tests for flume-taildir-source module -> passed
> 
> 
> Thanks,
> 
> hun shenshi
> 
>


Re: Review Request 59743: FLUME-3101. When TaildirSource read the file written by high frequency, would not break out to read another file.

Posted by Denes Arvay <de...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59743/#review177175
-----------------------------------------------------------



Thank you for the patch.
In general I think if this problem occurs then the load is higher what the actual Flume setup can handle, so the files won't be fully read unless the load decreases. On the other hand it might be good for some use cases to process other files instead of working on only one, so all in all I agree with the change.

I had some comments, could you please fix those issues?
Plus it'd be great if you could add this new configuration parameter to the User Guide as well. (`flume-ng-doc/sphinx/FlumeUserGuide.rst`, to check your changes you can generate the html files with the following commands:
```
$ export LC_ALL=C.UTF-8 # Required to build the javadocs on some platforms and in some locales
$ mvn clean install -Psite -DskipTests
```
The output will be placed in the `target/site` directory.

Let me know if you have any concerns regarding to my comments.


flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSource.java
Lines 188 (patched)
<https://reviews.apache.org/r/59743/#comment250719>

    Could you please and a check to handle the invalid (i.e. <= 0) value for the `numThreshold`?
    Graceful fallback to the default value with some logging could be a good solution, as in the FileChannel: https://github.com/apache/flume/blob/trunk/flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java#L165



flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSource.java
Lines 256 (patched)
<https://reviews.apache.org/r/59743/#comment250715>

    I think it'd be better to use `long` to avoid overflow.



flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSourceConfigurationConstants.java
Lines 69 (patched)
<https://reviews.apache.org/r/59743/#comment250717>

    The default value should provide a behaviour which is the same as without this change. Could you please set this to `Integer.MAX_VALUE` (or `Long.MAX_VALUE` according to my comment on line 256)?
    Or `0` and handle that as special value in the `if` in line 279?



flume-ng-sources/flume-taildir-source/src/test/java/org/apache/flume/source/taildir/TestTaildirSource.java
Lines 342 (patched)
<https://reviews.apache.org/r/59743/#comment250713>

    Could you please add spaces around the = and < signs? Checkstyle failes on this line.



flume-ng-sources/flume-taildir-source/src/test/java/org/apache/flume/source/taildir/TestTaildirSource.java
Lines 351 (patched)
<https://reviews.apache.org/r/59743/#comment250714>

    same as in line 342


- Denes Arvay


On June 4, 2017, 4:23 p.m., hun shenshi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59743/
> -----------------------------------------------------------
> 
> (Updated June 4, 2017, 4:23 p.m.)
> 
> 
> Review request for Flume.
> 
> 
> Bugs: FLUME-3101
>     https://issues.apache.org/jira/browse/FLUME-3101
> 
> 
> Repository: flume-git
> 
> 
> Description
> -------
> 
> Add a threshold in TaildirSource. Use the threshold to control the number of reads when a file is written by high frequency.
> 
> 
> Diffs
> -----
> 
>   flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSource.java a107a01 
>   flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSourceConfigurationConstants.java f2347f3 
>   flume-ng-sources/flume-taildir-source/src/test/java/org/apache/flume/source/taildir/TestTaildirSource.java 097ee0b 
> 
> 
> Diff: https://reviews.apache.org/r/59743/diff/3/
> 
> 
> Testing
> -------
> 
> mvn clean install -DskipTests -> built
> junit tests for flume-taildir-source module -> passed
> 
> 
> Thanks,
> 
> hun shenshi
> 
>


Re: Review Request 59743: FLUME-3101. When TaildirSource read the file written by high frequency, would not break out to read another file.

Posted by hun shenshi <28...@qq.com>.

> On 六月 5, 2017, 12:40 p.m., Miklos Csanady wrote:
> > Nice job.
> > Thank you!

Will it merge in master fork?


- hun


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59743/#review176908
-----------------------------------------------------------


On 六月 4, 2017, 2:23 p.m., hun shenshi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59743/
> -----------------------------------------------------------
> 
> (Updated 六月 4, 2017, 2:23 p.m.)
> 
> 
> Review request for Flume.
> 
> 
> Bugs: FLUME-3101
>     https://issues.apache.org/jira/browse/FLUME-3101
> 
> 
> Repository: flume-git
> 
> 
> Description
> -------
> 
> Add a threshold in TaildirSource. Use the threshold to control the number of reads when a file is written by high frequency.
> 
> 
> Diffs
> -----
> 
>   flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSource.java a107a01 
>   flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSourceConfigurationConstants.java f2347f3 
>   flume-ng-sources/flume-taildir-source/src/test/java/org/apache/flume/source/taildir/TestTaildirSource.java 097ee0b 
> 
> 
> Diff: https://reviews.apache.org/r/59743/diff/3/
> 
> 
> Testing
> -------
> 
> mvn clean install -DskipTests -> built
> junit tests for flume-taildir-source module -> passed
> 
> 
> Thanks,
> 
> hun shenshi
> 
>


Re: Review Request 59743: FLUME-3101. When TaildirSource read the file written by high frequency, would not break out to read another file.

Posted by Miklos Csanady <mi...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59743/#review176908
-----------------------------------------------------------


Ship it!




Nice job.
Thank you!

- Miklos Csanady


On June 4, 2017, 2:23 p.m., hun shenshi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59743/
> -----------------------------------------------------------
> 
> (Updated June 4, 2017, 2:23 p.m.)
> 
> 
> Review request for Flume.
> 
> 
> Bugs: FLUME-3101
>     https://issues.apache.org/jira/browse/FLUME-3101
> 
> 
> Repository: flume-git
> 
> 
> Description
> -------
> 
> Add a threshold in TaildirSource. Use the threshold to control the number of reads when a file is written by high frequency.
> 
> 
> Diffs
> -----
> 
>   flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSource.java a107a01 
>   flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSourceConfigurationConstants.java f2347f3 
>   flume-ng-sources/flume-taildir-source/src/test/java/org/apache/flume/source/taildir/TestTaildirSource.java 097ee0b 
> 
> 
> Diff: https://reviews.apache.org/r/59743/diff/3/
> 
> 
> Testing
> -------
> 
> mvn clean install -DskipTests -> built
> junit tests for flume-taildir-source module -> passed
> 
> 
> Thanks,
> 
> hun shenshi
> 
>


Re: Review Request 59743: FLUME-3101. When TaildirSource read the file written by high frequency, would not break out to read another file.

Posted by hun shenshi <28...@qq.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59743/
-----------------------------------------------------------

(Updated 六月 4, 2017, 2:23 p.m.)


Review request for Flume.


Changes
-------

add a test


Bugs: FLUME-3101
    https://issues.apache.org/jira/browse/FLUME-3101


Repository: flume-git


Description
-------

Add a threshold in TaildirSource. Use the threshold to control the number of reads when a file is written by high frequency.


Diffs (updated)
-----

  flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSource.java a107a01 
  flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSourceConfigurationConstants.java f2347f3 
  flume-ng-sources/flume-taildir-source/src/test/java/org/apache/flume/source/taildir/TestTaildirSource.java 097ee0b 


Diff: https://reviews.apache.org/r/59743/diff/3/

Changes: https://reviews.apache.org/r/59743/diff/2-3/


Testing (updated)
-------

mvn clean install -DskipTests -> built
junit tests for flume-taildir-source module -> passed


Thanks,

hun shenshi


Re: Review Request 59743: FLUME-3101. When TaildirSource read the file written by high frequency, would not break out to read another file.

Posted by hun shenshi <28...@qq.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59743/
-----------------------------------------------------------

(Updated 六月 4, 2017, 10:10 a.m.)


Review request for Flume.


Changes
-------

make numThreshold configurable. 1000 can be a default.
log a warning when readCount larger than numThreshold


Bugs: FLUME-3101
    https://issues.apache.org/jira/browse/FLUME-3101


Repository: flume-git


Description
-------

Add a threshold in TaildirSource. Use the threshold to control the number of reads when a file is written by high frequency.


Diffs (updated)
-----

  flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSource.java a107a01 
  flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSourceConfigurationConstants.java f2347f3 


Diff: https://reviews.apache.org/r/59743/diff/2/

Changes: https://reviews.apache.org/r/59743/diff/1-2/


Testing
-------

mvn clean install -DskipTests -> built


Thanks,

hun shenshi