[
https://issues.apache.org/jira/browse/FLUME-857?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13156842#comment-13156842
]
[email protected] commented on FLUME-857:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2929/#review3505
-----------------------------------------------------------
Ship it!
Prasad, lgtm.
Nice tests.
Please fix some nits, and go ahead and commit.
flume-core/src/main/java/com/cloudera/flume/handlers/debug/StubbornAppendSink.java
<https://reviews.apache.org/r/2929/#comment7794>
nit: Capitalization error
flume-core/src/main/java/com/cloudera/flume/handlers/debug/StubbornAppendSink.java
<https://reviews.apache.org/r/2929/#comment7802>
Maybe ERROR level log?
flume-core/src/test/java/com/cloudera/flume/collector/TestCollectorSink.java
<https://reviews.apache.org/r/2929/#comment7795>
nit: extra line
flume-core/src/test/java/com/cloudera/flume/collector/TestCollectorSink.java
<https://reviews.apache.org/r/2929/#comment7796>
nit: sp
flume-core/src/test/java/com/cloudera/flume/collector/TestCollectorSink.java
<https://reviews.apache.org/r/2929/#comment7801>
The goal here is that this doesn't dead lock right? maybe make test time
out?
@Test(timeout=5000)
flume-core/src/test/java/com/cloudera/flume/collector/TestCollectorSink.java
<https://reviews.apache.org/r/2929/#comment7798>
nice.
nit: Can you indent this to make clear that it is a chain instead of
separate statements?
Maybe something like this?
doThrow(...)
.doAnswer(...{
}).
.when(...).append(..);
Alternately, create the exceptions and answer first so that you just see
the chain?
IOException ioe = ...
Answer ans = ...
doThrow(ioe).answer(ans).when(snk).append((Event) anyObject);
flume-core/src/test/java/com/cloudera/flume/collector/TestCollectorSink.java
<https://reviews.apache.org/r/2929/#comment7799>
nit: what does rte stand for?
flume-core/src/test/java/com/cloudera/flume/collector/TestCollectorSink.java
<https://reviews.apache.org/r/2929/#comment7803>
similar comment about indents.
flume-core/src/test/java/com/cloudera/flume/collector/TestCollectorSink.java
<https://reviews.apache.org/r/2929/#comment7804>
similar.
flume-core/src/test/java/com/cloudera/flume/collector/TestCollectorSink.java
<https://reviews.apache.org/r/2929/#comment7805>
This pattern exists in many tests. maybe we should make a
FlumeSinkTestUtil helper class to do this for us.
flume-core/src/test/java/com/cloudera/flume/collector/TestCollectorSink.java
<https://reviews.apache.org/r/2929/#comment7806>
rte?
- jmhsieh
On 2011-11-24 04:44:38, Prasad Mujumdar wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2929/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-11-24 04:44:38)
bq.
bq.
bq. Review request for jmhsieh.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. The CollectionSink create a chain of sinks on top of the underlying sink
to perform roll and retries upon failures. The stubborn append reopens the
downstream sink and retries the append once. The insistent append keeps
retrying the append.
bq. Now if any errors (like network or HDFS datanodes shutdown) cause the
downstream sink to throw IOException on append, the stubborn append will try to
close that sink and reopen. If the close also throws an IOException, then sink
chain doesn't get properly closed. This results into the collectorsink skipping
the open and perpetually failing on append/close.
bq. The fix is to ignore IOException in when closing downstream sink from the
stubborn append. Also set the close status correctly for decorators and roller.
bq.
bq.
bq. This addresses bug FLUME-857.
bq. https://issues.apache.org/jira/browse/FLUME-857
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. flume-core/src/main/java/com/cloudera/flume/core/EventSinkDecorator.java
6ab39ba
bq.
flume-core/src/main/java/com/cloudera/flume/handlers/debug/StubbornAppendSink.java
9436eb4
bq.
flume-core/src/main/java/com/cloudera/flume/handlers/rolling/RollSink.java
27302b1
bq.
flume-core/src/test/java/com/cloudera/flume/collector/TestCollectorSink.java
4f25f58
bq.
bq. Diff: https://reviews.apache.org/r/2929/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. Added new test cases in TestCollectorSink. will run full regression test
suite.
bq.
bq.
bq. Thanks,
bq.
bq. Prasad
bq.
bq.
> CollectorSink can get stuck into perpetual close() if the underlying sinks
> throws IOExceptions for both append and close.
> -------------------------------------------------------------------------------------------------------------------------
>
> Key: FLUME-857
> URL: https://issues.apache.org/jira/browse/FLUME-857
> Project: Flume
> Issue Type: Bug
> Affects Versions: v0.9.3, v0.9.4
> Reporter: Prasad Mujumdar
> Assignee: Prasad Mujumdar
> Fix For: v0.9.5
>
>
> The CollectionSink create a chain of sinks on top of the underlying sink to
> perform roll and retries upon failures. The stubborn append reopens the
> downstream sink and retries the append once. The insistent append keeps
> retrying the append.
> Now if any errors (like network or HDFS datanodes shutdown) cause the
> downstream sink to throw IOException on append, the stubborn append will try
> to close that sink and reopen. If the close also throws an IOException, then
> sink chain doesn't get properly closed. This results into the collectorsink
> skipping the open and perpetually failing on append/close.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
