----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/6387/#review9860 -----------------------------------------------------------
In general I think the change to only remove logs after a successful checkpoint makes sense. Just have two items below. flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FlumeEventQueue.java <https://reviews.apache.org/r/6387/#comment20993> Shouldn't this return SortedSet as opposed to the concrete type? flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java <https://reviews.apache.org/r/6387/#comment20992> Can you explain why you are doing this? I don't follow the comment. SInce it's a set we can add as many duplicates as we want and they will get removed. In practice this set will be quite small. - Brock Noland On Aug. 5, 2012, 12:26 a.m., Hari Shreedharan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/6387/ > ----------------------------------------------------------- > > (Updated Aug. 5, 2012, 12:26 a.m.) > > > Review request for Flume, Arvind Prabhakar and Brock Noland. > > > Description > ------- > > Currently, the background worker will delete logs marked as inactive even if > there is no checkpoint. This can lead to checkpoints referencing logs which > were deleted after the checkpoint was done - causing the channel to not > startup unless the checkpoint itself is deleted. This patch collects all > fileIDs which are present in a checkpoint and deletes logs which were not > active at the time of the checkpoint. > > removeOldLogs is always called from the background worker and thus does not > need locking. Replay will not call this, since the channel is not open at the > time of replay. > > > This addresses bug FLUME-1417. > https://issues.apache.org/jira/browse/FLUME-1417 > > > Diffs > ----- > > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FlumeEventQueue.java > 64d3dec > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java > 64a70c8 > > Diff: https://reviews.apache.org/r/6387/diff/ > > > Testing > ------- > > All unit tests pass. Did functional testing. > > > Thanks, > > Hari Shreedharan > >
