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

Switch to Plain View
Flume >> mail # dev >> Review Request: FLUME-1516. Write dual checkpoints.


+
Hari Shreedharan 2013-01-09, 07:06
+
Hari Shreedharan 2013-01-09, 17:09
+
Hari Shreedharan 2013-01-21, 21:36
+
Hari Shreedharan 2013-01-22, 00:43
+
Hari Shreedharan 2013-03-04, 20:44
+
Hari Shreedharan 2013-03-14, 07:31
Copy link to this message
-
Re: Review Request: FLUME-1516. Write dual checkpoints.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8899/#review17876
-----------------------------------------------------------
Hari, looks good and thanks for the hard work!! Below is a first pass.
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/EventQueueBackingStoreFile.java
<https://reviews.apache.org/r/8899/#comment37844>

    This is duplicated elsewhere

flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/EventQueueBackingStoreFile.java
<https://reviews.apache.org/r/8899/#comment37843>

    Should we warn if it fails?

flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/EventQueueBackingStoreFile.java
<https://reviews.apache.org/r/8899/#comment37841>

    listFiles can return null

flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/EventQueueBackingStoreFile.java
<https://reviews.apache.org/r/8899/#comment37842>

    this is the same as backupFile, why are creating it? What if it fails?

flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/EventQueueBackingStoreFile.java
<https://reviews.apache.org/r/8899/#comment37850>

    I think that "Backup" would be a more consistent name than "BackUp"

flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/EventQueueBackingStoreFile.java
<https://reviews.apache.org/r/8899/#comment37851>

    listFiles can return null.

flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/EventQueueBackingStoreFile.java
<https://reviews.apache.org/r/8899/#comment37845>

    Did the backup fail or is it still in progress?

flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/EventQueueBackingStoreFile.java
<https://reviews.apache.org/r/8899/#comment37847>

    We should catch Throwable here incase a runtime exception or Error is thrown.

flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/EventQueueBackingStoreFile.java
<https://reviews.apache.org/r/8899/#comment37846>

    We should log the exception here

flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java
<https://reviews.apache.org/r/8899/#comment37854>

    This will be set to null by default, no need to have = null.

flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java
<https://reviews.apache.org/r/8899/#comment37855>

    What if they specify the same value twice? That would be ugly. Also since this property is not plural perhaps would we should introduce a new property, BACKPOINT_BACKUP_DIR?

flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java
<https://reviews.apache.org/r/8899/#comment37853>

    If useDualCheckpoints is true we should warn that we cannot? We what if people put none, or three checkpoint directories? I think we should use Splliter.on(",").trimResults().omitEmptyStrings() and then do a little more sanity checking on this.

flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java
<https://reviews.apache.org/r/8899/#comment37852>

    We don't do reconfiguration any longer, should we remove the reconfig stuff?

flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFile.java
<https://reviews.apache.org/r/8899/#comment37848>

    I know it's not your change, but can you change this to info?

flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Serialization.java
<https://reviews.apache.org/r/8899/#comment37849>

    This can return null, we should handle that.
- Brock Noland
On March 14, 2013, 7:31 a.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
+
Hari Shreedharan 2013-03-14, 18:10
+
Brock Noland 2013-03-14, 18:37
+
Hari Shreedharan 2013-03-14, 23:44
+
Brock Noland 2013-03-21, 03:01
+
Hari Shreedharan 2013-03-21, 21:30
+
Hari Shreedharan 2013-03-25, 21:03
+
Brock Noland 2013-03-25, 21:51
+
Hari Shreedharan 2013-03-25, 22:08
+
Brock Noland 2013-03-25, 22:12
+
Hari Shreedharan 2013-03-26, 22:52
+
Brock Noland 2013-04-04, 18:20
+
Hari Shreedharan 2013-04-04, 21:34
+
Hari Shreedharan 2013-04-04, 21:32
+
Hari Shreedharan 2013-04-05, 00:16
+
Brock Noland 2013-04-05, 14:35
+
Hari Shreedharan 2013-04-05, 18:01
+
Brock Noland 2013-04-05, 18:26