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

Switch to Threaded View
Flume >> mail # dev >> Re: Review Request 12060: Revised design for Spillable Mem Channel


Copy link to this message
-
Re: Review Request 12060: Revised design for Spillable Mem Channel

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

Ship it!
Roshan,

This looks good to go, but I am fairly sure this needs considerable testing and improvements before we call it production ready. So let's commit this for now, but mark it as alpha and experimental in the user docs and the javadocs. Let's iterate on this over to make sure it is production ready.
Also can you please make sure the formats are correct. An if looks like:

if (condition) {
} else {
}

similarly for for/while etc. An IDE can probably fix most of it up for you.

Please make these fixes and attach the patch to the jira.
flume-ng-channels/flume-spillable-memory-channel/src/main/java/org/apache/flume/channel/SpillableMemoryChannel.java
<https://reviews.apache.org/r/12060/#comment54213>

    Nit: Space before and after += makes it more readable.

flume-ng-channels/flume-spillable-memory-channel/src/main/java/org/apache/flume/channel/SpillableMemoryChannel.java
<https://reviews.apache.org/r/12060/#comment54211>

    Extra space after if (should generally be before the paranthesis).

flume-ng-channels/flume-spillable-memory-channel/src/main/java/org/apache/flume/channel/SpillableMemoryChannel.java
<https://reviews.apache.org/r/12060/#comment54214>

    same as above

flume-ng-channels/flume-spillable-memory-channel/src/main/java/org/apache/flume/channel/SpillableMemoryChannel.java
<https://reviews.apache.org/r/12060/#comment54215>

    same as above

flume-ng-channels/flume-spillable-memory-channel/src/main/java/org/apache/flume/channel/SpillableMemoryChannel.java
<https://reviews.apache.org/r/12060/#comment54212>

    same as above

flume-ng-channels/flume-spillable-memory-channel/src/main/java/org/apache/flume/channel/SpillableMemoryChannel.java
<https://reviews.apache.org/r/12060/#comment54216>

    the spaces in the if are off. an if should look like:
    
    if (condition) {
    
    } else {
    
    }

flume-ng-channels/flume-spillable-memory-channel/src/main/java/org/apache/flume/channel/SpillableMemoryChannel.java
<https://reviews.apache.org/r/12060/#comment54217>

    additional space after return

flume-ng-channels/flume-spillable-memory-channel/src/main/java/org/apache/flume/channel/SpillableMemoryChannel.java
<https://reviews.apache.org/r/12060/#comment54218>

    Space after !

flume-ng-channels/flume-spillable-memory-channel/src/main/java/org/apache/flume/channel/SpillableMemoryChannel.java
<https://reviews.apache.org/r/12060/#comment54219>

    additional space within (), missing space before.

flume-ng-channels/flume-spillable-memory-channel/src/main/java/org/apache/flume/channel/SpillableMemoryChannel.java
<https://reviews.apache.org/r/12060/#comment54220>

    space issue

flume-ng-channels/flume-spillable-memory-channel/src/main/java/org/apache/flume/channel/SpillableMemoryChannel.java
<https://reviews.apache.org/r/12060/#comment54221>

    space issue
- Hari Shreedharan
On Oct. 24, 2013, 10:18 p.m., Roshan Naik wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12060/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2013, 10:18 p.m.)
>
>
> Review request for Flume.
>
>
> Bugs: FLUME-1227
>     https://issues.apache.org/jira/browse/FLUME-1227
>
>
> Repository: flume-git
>
>
> Description
> -------
>
> Revised design for Spillable Mem Channel.
> We no longer have Spillable channel config pointing to another channel (by name) as in the previous design.
>
> Spillable Channel instead derives from FileChannel (as per https://issues.apache.org/jira/browse/FLUME-1227?focusedCommentId=13628201&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13628201)
>