You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flume.apache.org by Hari Shreedharan <hs...@cloudera.com> on 2012/07/28 04:34:42 UTC
Review Request: FLUME-1401: Async hbase sink should support timing out.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6183/
-----------------------------------------------------------
Review request for Flume.
Description
-------
Timeout the sink if all callbacks are not received within the configurable timeout.
This addresses bug FLUME-1401.
https://issues.apache.org/jira/browse/FLUME-1401
Diffs
-----
flume-ng-doc/sphinx/FlumeUserGuide.rst 56011e8
flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/AsyncHBaseSink.java 6df1f33
flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/HBaseSinkConfigurationConstants.java a16cda8
Diff: https://reviews.apache.org/r/6183/diff/
Testing
-------
All unit tests pass. Did functional testing.
Thanks,
Hari Shreedharan
Re: Review Request: FLUME-1401: Async hbase sink should support timing out.
Posted by Hari Shreedharan <hs...@cloudera.com>.
> On July 30, 2012, 2:35 a.m., Brock Noland wrote:
> > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/AsyncHBaseSink.java, line 184
> > <https://reviews.apache.org/r/6183/diff/3/?file=130184#file130184line184>
> >
> > If this is an Error or RuntimeException I think we should re-throw.
Agreed. Will update the patch.
> On July 30, 2012, 2:35 a.m., Brock Noland wrote:
> > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/AsyncHBaseSink.java, line 201
> > <https://reviews.apache.org/r/6183/diff/3/?file=130184#file130184line201>
> >
> > Any reason we are using an exception to log this timeout message?
> >
> > Also, we have combined the interrupt and timeout into one log message.
Yes, you are right. I have modified this to simply set txnFail if condition.await returns false, and the failure processing code should take care of it.
- Hari
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6183/#review9580
-----------------------------------------------------------
On July 28, 2012, 7:19 p.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6183/
> -----------------------------------------------------------
>
> (Updated July 28, 2012, 7:19 p.m.)
>
>
> Review request for Flume.
>
>
> Description
> -------
>
> Timeout the sink if all callbacks are not received within the configurable timeout.
>
>
> This addresses bug FLUME-1401.
> https://issues.apache.org/jira/browse/FLUME-1401
>
>
> Diffs
> -----
>
> flume-ng-doc/sphinx/FlumeUserGuide.rst 56011e8
> flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/AsyncHBaseSink.java 6df1f33
> flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/HBaseSinkConfigurationConstants.java a16cda8
>
> Diff: https://reviews.apache.org/r/6183/diff/
>
>
> Testing
> -------
>
> All unit tests pass. Did functional testing.
>
>
> Thanks,
>
> Hari Shreedharan
>
>
Re: Review Request: FLUME-1401: Async hbase sink should support timing out.
Posted by Brock Noland <br...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6183/#review9580
-----------------------------------------------------------
flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/AsyncHBaseSink.java
<https://reviews.apache.org/r/6183/#comment20454>
If this is an Error or RuntimeException I think we should re-throw.
flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/AsyncHBaseSink.java
<https://reviews.apache.org/r/6183/#comment20455>
Any reason we are using an exception to log this timeout message?
Also, we have combined the interrupt and timeout into one log message.
- Brock Noland
On July 28, 2012, 7:19 p.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6183/
> -----------------------------------------------------------
>
> (Updated July 28, 2012, 7:19 p.m.)
>
>
> Review request for Flume.
>
>
> Description
> -------
>
> Timeout the sink if all callbacks are not received within the configurable timeout.
>
>
> This addresses bug FLUME-1401.
> https://issues.apache.org/jira/browse/FLUME-1401
>
>
> Diffs
> -----
>
> flume-ng-doc/sphinx/FlumeUserGuide.rst 56011e8
> flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/AsyncHBaseSink.java 6df1f33
> flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/HBaseSinkConfigurationConstants.java a16cda8
>
> Diff: https://reviews.apache.org/r/6183/diff/
>
>
> Testing
> -------
>
> All unit tests pass. Did functional testing.
>
>
> Thanks,
>
> Hari Shreedharan
>
>
Re: Review Request: FLUME-1401: Async hbase sink should support timing out.
Posted by Hari Shreedharan <hs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6183/#review9586
-----------------------------------------------------------
Please ignore the last patch...it is failing a unit test.
- Hari Shreedharan
On July 30, 2012, 6:03 a.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6183/
> -----------------------------------------------------------
>
> (Updated July 30, 2012, 6:03 a.m.)
>
>
> Review request for Flume.
>
>
> Description
> -------
>
> Timeout the sink if all callbacks are not received within the configurable timeout.
>
>
> This addresses bug FLUME-1401.
> https://issues.apache.org/jira/browse/FLUME-1401
>
>
> Diffs
> -----
>
> flume-ng-doc/sphinx/FlumeUserGuide.rst 56011e8
> flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/AsyncHBaseSink.java 6df1f33
> flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/HBaseSinkConfigurationConstants.java a16cda8
>
> Diff: https://reviews.apache.org/r/6183/diff/
>
>
> Testing
> -------
>
> All unit tests pass. Did functional testing.
>
>
> Thanks,
>
> Hari Shreedharan
>
>
Re: Review Request: FLUME-1401: Async hbase sink should support timing out.
Posted by Brock Noland <br...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6183/#review9589
-----------------------------------------------------------
Ship it!
Ship It!
- Brock Noland
On July 30, 2012, 6:51 a.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6183/
> -----------------------------------------------------------
>
> (Updated July 30, 2012, 6:51 a.m.)
>
>
> Review request for Flume.
>
>
> Description
> -------
>
> Timeout the sink if all callbacks are not received within the configurable timeout.
>
>
> This addresses bug FLUME-1401.
> https://issues.apache.org/jira/browse/FLUME-1401
>
>
> Diffs
> -----
>
> flume-ng-doc/sphinx/FlumeUserGuide.rst 56011e8
> flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/AsyncHBaseSink.java 6df1f33
> flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/HBaseSinkConfigurationConstants.java a16cda8
>
> Diff: https://reviews.apache.org/r/6183/diff/
>
>
> Testing
> -------
>
> All unit tests pass. Did functional testing.
>
>
> Thanks,
>
> Hari Shreedharan
>
>
Re: Review Request: FLUME-1401: Async hbase sink should support timing out.
Posted by Hari Shreedharan <hs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6183/
-----------------------------------------------------------
(Updated July 30, 2012, 6:51 a.m.)
Review request for Flume.
Changes
-------
Updating based on Brock's feedback. Moved error processing to another method. Added txn.close() now called after txn.commit(), since txn.close() is called on failure from handleTxnFailure method.
Description
-------
Timeout the sink if all callbacks are not received within the configurable timeout.
This addresses bug FLUME-1401.
https://issues.apache.org/jira/browse/FLUME-1401
Diffs (updated)
-----
flume-ng-doc/sphinx/FlumeUserGuide.rst 56011e8
flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/AsyncHBaseSink.java 6df1f33
flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/HBaseSinkConfigurationConstants.java a16cda8
Diff: https://reviews.apache.org/r/6183/diff/
Testing
-------
All unit tests pass. Did functional testing.
Thanks,
Hari Shreedharan
Re: Review Request: FLUME-1401: Async hbase sink should support timing out.
Posted by Hari Shreedharan <hs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6183/
-----------------------------------------------------------
(Updated July 30, 2012, 6:03 a.m.)
Review request for Flume.
Changes
-------
Thanks for your review, Brock. I updated the patch to handle the cases you mentioned. I also changed some other exception processing code, to use the existing method instead of repeated code.
Description
-------
Timeout the sink if all callbacks are not received within the configurable timeout.
This addresses bug FLUME-1401.
https://issues.apache.org/jira/browse/FLUME-1401
Diffs (updated)
-----
flume-ng-doc/sphinx/FlumeUserGuide.rst 56011e8
flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/AsyncHBaseSink.java 6df1f33
flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/HBaseSinkConfigurationConstants.java a16cda8
Diff: https://reviews.apache.org/r/6183/diff/
Testing
-------
All unit tests pass. Did functional testing.
Thanks,
Hari Shreedharan
Re: Review Request: FLUME-1401: Async hbase sink should support timing out.
Posted by Hari Shreedharan <hs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6183/
-----------------------------------------------------------
(Updated July 28, 2012, 7:19 p.m.)
Review request for Flume.
Changes
-------
Making a minor change in javadoc.
Description
-------
Timeout the sink if all callbacks are not received within the configurable timeout.
This addresses bug FLUME-1401.
https://issues.apache.org/jira/browse/FLUME-1401
Diffs (updated)
-----
flume-ng-doc/sphinx/FlumeUserGuide.rst 56011e8
flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/AsyncHBaseSink.java 6df1f33
flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/HBaseSinkConfigurationConstants.java a16cda8
Diff: https://reviews.apache.org/r/6183/diff/
Testing
-------
All unit tests pass. Did functional testing.
Thanks,
Hari Shreedharan
Re: Review Request: FLUME-1401: Async hbase sink should support timing out.
Posted by Hari Shreedharan <hs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6183/
-----------------------------------------------------------
(Updated July 28, 2012, 7:11 p.m.)
Review request for Flume.
Changes
-------
Adding a try-catch where channel.getTransaction and channel.take are called, as I ran into FLUME-1403 while testing.
Description
-------
Timeout the sink if all callbacks are not received within the configurable timeout.
This addresses bug FLUME-1401.
https://issues.apache.org/jira/browse/FLUME-1401
Diffs (updated)
-----
flume-ng-doc/sphinx/FlumeUserGuide.rst 56011e8
flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/AsyncHBaseSink.java 6df1f33
flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/HBaseSinkConfigurationConstants.java a16cda8
Diff: https://reviews.apache.org/r/6183/diff/
Testing
-------
All unit tests pass. Did functional testing.
Thanks,
Hari Shreedharan