flume-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Denes Arvay <de...@cloudera.com>
Subject Re: Review Request 57617: FLUME-2620 File Channel to support empty values in headers
Date Tue, 18 Jul 2017 11:36:39 GMT


> On July 17, 2017, 4:20 p.m., Miklos Csanady wrote:
> > Thank you for this path.
> > 
> > I tested the patch with your solution which seems to be good.

Thank you for the patch Marcell and for the review Miklos, I'm going to commit this if nobody
has any concerns.


- Denes


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


On March 16, 2017, 9 a.m., Marcell Hegedus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57617/
> -----------------------------------------------------------
> 
> (Updated March 16, 2017, 9 a.m.)
> 
> 
> Review request for Flume.
> 
> 
> Bugs: FLUME-2620
>     https://issues.apache.org/jira/browse/FLUME-2620
> 
> 
> Repository: flume-git
> 
> 
> Description
> -------
> 
> Flume user guide does not specify whether a value in event header could be null or not.
Given an external system generating events which header vaules can be null and a user configures
Flume with Memory Channel then he will have no trouble. Later on when the user changes Memory
Channel to File Channel then Flume will fail with NPE. It is because FC is serializing events
with protocol buffer and header values are defined as required in the proto file.
> In this patch I have changed the value field to optional. However protocol buffer does
not have a notation for null and setting a field to null raises NPE again. Added a null check
before serialization to prevent this.
> There is on caveat: When an optional field is not set, at deserialization it will be
set to a default value: in this case it will be empty string.
> 
> 
> Diffs
> -----
> 
>   flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Put.java
0a70a24 
>   flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/proto/ProtosFactory.java
50492cc 
>   flume-ng-channels/flume-file-channel/src/main/proto/filechannel.proto 25520e8 
>   flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestFileChannel.java
8efe991 
>   flume-ng-core/src/test/java/org/apache/flume/channel/TestMemoryChannel.java 344bb58

> 
> 
> Diff: https://reviews.apache.org/r/57617/diff/1/
> 
> 
> Testing
> -------
> 
> Added unit test for both Memory Channel and File Channel to check if they accept null
values in header.
> 
> 
> Thanks,
> 
> Marcell Hegedus
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message