accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Josh Elser" <josh.el...@gmail.com>
Subject Re: Review Request 24741: Bug fix for ACCUMULO-1628
Date Mon, 18 Aug 2014 15:17:21 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message