> 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? > > kturner wrote: > Like a functional test? Changes in the unit test cover this case, but > not in an end to end way.
Yeah, given the severity of the failure mode, I'd really like an end-to-end. I also think a functional test is more likely to catch a future regression. 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? > > > > kturner wrote: > 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. That sounds reasonable. - Sean ----------------------------------------------------------- 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 > >
