You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flume.apache.org by Denes Arvay <de...@cloudera.com> on 2016/08/02 23:19:25 UTC
Re: Review Request 50232: FLUME-2619: Spooldir source does not log
channel exceptions
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50232/#review144556
-----------------------------------------------------------
flume-ng-core/src/main/java/org/apache/flume/source/SpoolDirectorySource.java (lines 257 - 281)
<https://reviews.apache.org/r/50232/#comment210586>
could you please somehow eliminate the copy-paste here? Either by extracting the `if (backoff) {...}` block to a method or by combining the two `catch` blocks to one `catch (ChannelException)` and doing `instanceof` checks inside it. (I'd prefer the former one)
flume-ng-core/src/main/java/org/apache/flume/source/SpoolDirectorySource.java (line 259)
<https://reviews.apache.org/r/50232/#comment210587>
Minor: `String.valueOf` is unnecessary here and it's easier to read without it.
flume-ng-core/src/test/java/org/apache/flume/source/TestSpoolDirectorySource.java (line 402)
<https://reviews.apache.org/r/50232/#comment210589>
This check cannot fail because the `while` loop in line 308 makes sure that `hitChannelFullException` is true.
- Denes Arvay
On July 22, 2016, 1:33 p.m., Bal�zs Don�t Bessenyei wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50232/
> -----------------------------------------------------------
>
> (Updated July 22, 2016, 1:33 p.m.)
>
>
> Review request for Flume, Denes Arvay and Attila Simon.
>
>
> Repository: flume-git
>
>
> Description
> -------
>
> Spooldir assumes that any ChannelException means that the channel is full and it does not log the exception message.
>
>
> Diffs
> -----
>
> flume-ng-core/src/main/java/org/apache/flume/channel/ChannelProcessor.java 1cce137
> flume-ng-core/src/main/java/org/apache/flume/source/SpoolDirectorySource.java d88cc1d
> flume-ng-core/src/test/java/org/apache/flume/source/TestSpoolDirectorySource.java 82c5351
>
> Diff: https://reviews.apache.org/r/50232/diff/
>
>
> Testing
> -------
>
> [INFO] Flume NG Core ...................................... SUCCESS [08:04 min]
>
>
> Thanks,
>
> Bal�zs Don�t Bessenyei
>
>