Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Flume, mail # dev - Review Request 16415: flume-ng accumulo sink


Copy link to this message
-
Re: Review Request 16415: flume-ng accumulo sink
Hari Shreedharan 2014-01-09, 21:57


> On Dec. 31, 2013, 6:43 a.m., Bruno MAHE wrote:
> > flume-ng-sinks/flume-ng-accumulo-sink/src/main/java/org/apache/flume/sink/accumulo/AccumuloSink.java, lines 181-182
> > <https://reviews.apache.org/r/16415/diff/1/?file=401611#file401611line181>
> >
> >     Naive question:
> >      If your serializer fails to serialize an event for any reason (ex: event is not well formed), then currently you would rollback the transaction and try again forever.
>
> Matt Molek wrote:
>     Split the committing the transaction off into its own method, and changed the exception handling so the transaction is only rolled back for an exception in the commit itself. Failure in the serializer won't cause a rollback now. This follows the way the hbase sink is written.

Frankly, I think the approach in this class is fine. If the event is not well-formed, there is nothing that can be done (A bad event should cause issues that ops should notice). If the user is ok with dropping ill-formed events, they should catch any exceptions within the serializer and return an empty list of mutations. You need to throw an exception and retry if the serializer throws. So the code in this patch does look good to me - there is no real need to change it.

We recently fixed the HBaseSink code to do this - earlier it had issues with transactions not being closed if the serializer threw - which would happen if you don't catch exceptions thrown by the serializer
- Hari
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16415/#review30996
-----------------------------------------------------------
On Dec. 27, 2013, 6 p.m., Matt Molek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16415/
> -----------------------------------------------------------
>
> (Updated Dec. 27, 2013, 6 p.m.)
>
>
> Review request for Flume.
>
>
> Repository: flume-git
>
>
> Description
> -------
>
> This patch is identical to FLUME-2234-0.patch except I had to regenerate it with different git options to get review board to accept it.
>
>
> Diffs
> -----
>
>   flume-ng-sinks/flume-ng-accumulo-sink/pom.xml PRE-CREATION
>   flume-ng-sinks/flume-ng-accumulo-sink/src/main/java/org/apache/flume/sink/accumulo/AccumuloEventSerializer.java PRE-CREATION
>   flume-ng-sinks/flume-ng-accumulo-sink/src/main/java/org/apache/flume/sink/accumulo/AccumuloSink.java PRE-CREATION
>   flume-ng-sinks/flume-ng-accumulo-sink/src/main/java/org/apache/flume/sink/accumulo/AccumuloSinkConfigurationConstants.java PRE-CREATION
>   flume-ng-sinks/flume-ng-accumulo-sink/src/main/java/org/apache/flume/sink/accumulo/SimpleAccumuloEventSerializer.java PRE-CREATION
>   flume-ng-sinks/flume-ng-accumulo-sink/src/test/java/org/apache/flume/sink/accumulo/TestAccumuloSink.java PRE-CREATION
>   flume-ng-sinks/pom.xml d03576b
>
> Diff: https://reviews.apache.org/r/16415/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Matt Molek
>
>