----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24741/#review50882 -----------------------------------------------------------
I stepped through the unit test without the patch and noticed the failure and can see how the changes fix it. I'm going to pull down the full patch and run it through the unit and functional tests (maybe the 1.6 ITs too since there are more of them). There are some formatting discrepancies which would be nice to fix, but not critical. server/src/test/java/org/apache/accumulo/server/tabletserver/InMemoryMapTest.java <https://reviews.apache.org/r/24741/#comment88748> Would be better to use something like System.getProperty("user.dir") + "/target instead of "/tmp" server/src/test/java/org/apache/accumulo/server/tabletserver/InMemoryMapTest.java <https://reviews.apache.org/r/24741/#comment88749> There's no '3' case for interleaving. Was that intentional like the '0' case? - Josh Elser On Aug. 15, 2014, 4:31 p.m., kturner wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24741/ > ----------------------------------------------------------- > > (Updated Aug. 15, 2014, 4:31 p.m.) > > > Review request for accumulo. > > > Bugs: ACCUMULO-1628 > https://issues.apache.org/jira/browse/ACCUMULO-1628 > > > Repository: accumulo > > > Description > ------- > > Bug fix for ACCUMULO-1628. If no activity on review by 8/20 I will push > patch. > > I was concerned about the case where two threads will concurrently switch > different deep copies. There is the scan thread and the thread deleting in > mem map and calling switchNow on each deepCopy. Access to each deep copy is > synchronized, so thats fine. This patch modified the deep copies to use a > shared rfile. I think deep copy of the shared rfile is synchronized > properly. The shared rfile was introduced to get around problems with the > temp file being deleted. > > > Diffs > ----- > > > core/src/main/java/org/apache/accumulo/core/iterators/system/SourceSwitchingIterator.java > a4970dc > > server/src/main/java/org/apache/accumulo/server/tabletserver/InMemoryMap.java > 914cd85 > > server/src/test/java/org/apache/accumulo/server/tabletserver/InMemoryMapTest.java > 97c8eec > > Diff: https://reviews.apache.org/r/24741/diff/ > > > Testing > ------- > > A unit test was added that reproduced the bug. After changes in patch, unit > test no longer reproduces bug. > > mvn package runs w/o a problem > > > Thanks, > > kturner > >
