> On March 21, 2014, 7:46 p.m., Sean Busbey wrote: > > Can you include a functional test that replicates the catastrophic failure > > that prompted ACCUMULO-2520 to verify that this change stops it?
Like a functional test? Changes in the unit test cover this case, but not in an end to end way. > On March 21, 2014, 7:46 p.m., Sean Busbey wrote: > > src/server/src/main/java/org/apache/accumulo/server/gc/SimpleGarbageCollector.java, > > lines 442-443 > > <https://reviews.apache.org/r/19546/diff/1/?file=531749#file531749line442> > > > > nit: would you mind including {} for the if block? sure On March 21, 2014, 7:46 p.m., kturner wrote: > > Once this is forward ported to 1.6.0-SNAP, will it have to switch to > > absolute paths? both absolute and relative? > > I did a good bit of refactoring of the GC as part of ACCUMULO-1773 to make it more testable and to handle abs/rel paths. I also added more sanity checks to the GC. I have not yet determined if those checks will cover this case. At the very least 1.6.0 will need to add new test. I think the diffs for 1.6.0 will be very different. I was thinking that once these changes are reviewed, that maybe I could post another set of diffs for 1.6.0. Not sure what the best way to handle this is. I wanted to wait on merging this change forward locally in case some major issue came up in the review. - kturner ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19546/#review38173 ----------------------------------------------------------- On March 21, 2014, 7:27 p.m., kturner wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19546/ > ----------------------------------------------------------- > > (Updated March 21, 2014, 7:27 p.m.) > > > Review request for accumulo. > > > Bugs: ACCUMULO-2520 > https://issues.apache.org/jira/browse/ACCUMULO-2520 > > > Repository: accumulo > > > Description > ------- > > This change should make the GC more robust against the case of it deleting > things it should not. A concern with this change is that may make the GC > fail in a situation where it should not, which could cause hdfs to fill. > > > Diffs > ----- > > > src/server/src/main/java/org/apache/accumulo/server/gc/SimpleGarbageCollector.java > 22c3c0e > > src/server/src/test/java/org/apache/accumulo/server/gc/SimpleGarbageCollectorTest.java > PRE-CREATION > > src/server/src/test/java/org/apache/accumulo/server/gc/TestConfirmDeletes.java > be444dd > > Diff: https://reviews.apache.org/r/19546/diff/ > > > Testing > ------- > > Added unit test > Ran 1.4.5-SNAPSHOT and verified GC worked and deleted an orphaned file > > > Thanks, > > kturner > >
