-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5684/
-----------------------------------------------------------

Review request for accumulo.
Description
-------

A patch that fixes the issue of failed bulk imports being copied through the master.
This addresses bug ACCUMULO-409.
    https://issues.apache.org/jira/browse/ACCUMULO-409
Diffs
-----

  /branches/1.4/src/core/src/main/java/org/apache/accumulo/core/Constants.java 1355540
  /branches/1.4/src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooUtil.java 1355540
  /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/client/BulkImporter.java 1355540
  /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/master/tableOps/BulkImport.java 1355540
  /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/tabletserver/BIFCopyProcessor.java PRE-CREATION
  /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/tabletserver/TabletServer.java 1355540
  /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/zookeeper/DistributedWorkQueue.java PRE-CREATION
  /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/zookeeper/IZooReaderWriter.java 1355540
  /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/zookeeper/ZooReaderWriter.java 1355540
  /branches/1.4/test/system/test4/bulk_import_test.sh 1355540

Diff: https://reviews.apache.org/r/5684/diff/
Testing
-------
Thanks,

kturner

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5684/#review8792
-----------------------------------------------------------

/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/Constants.java
<https://reviews.apache.org/r/5684/#comment18613>

    Is it obvious to the general developer who might be trying to read this code that BIF stands for bulk import failure? Maybe a better naming scheme for variables, constants, and directories is in order.

/branches/1.4/src/server/src/main/java/org/apache/accumulo/server/master/tableOps/BulkImport.java
<https://reviews.apache.org/r/5684/#comment18615>

    Ack! Formatting!

/branches/1.4/src/server/src/main/java/org/apache/accumulo/server/master/tableOps/BulkImport.java
<https://reviews.apache.org/r/5684/#comment18616>

    Also need to check whether a concurrent operation could be in progress that writes the load flag after we check for its existence.

/branches/1.4/src/server/src/main/java/org/apache/accumulo/server/tabletserver/TabletServer.java
<https://reviews.apache.org/r/5684/#comment18617>

    Fixed-sized thread pool seems like a problem for scaling to different-sized machines. Also, what does 2 threads imply given the thread pool queue size checks in DistributedWorkQueue? Seems like if we generalize the thread pool size that would cause problems.
    
    Another thought here: we do a bunch of stuff that is distributed through tablet servers. Does it make sense to have threads dedicated to this particular distrubted task, or should this be part of a generalized framework?

/branches/1.4/src/server/src/main/java/org/apache/accumulo/server/zookeeper/DistributedWorkQueue.java
<https://reviews.apache.org/r/5684/#comment18614>

    What is special about a queue size of greater than one? Also, this code is repeated in several spots -- should it be turned into a method?
- Adam Fuchs
On June 30, 2012, 3:10 a.m., kturner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5684/
> -----------------------------------------------------------
>
> (Updated June 30, 2012, 3:10 a.m.)
>
>
> Review request for accumulo.
>
>
> Description
> -------
>
> A patch that fixes the issue of failed bulk imports being copied through the master.
>
>
> This addresses bug ACCUMULO-409.
>     https://issues.apache.org/jira/browse/ACCUMULO-409
>
>
> Diffs
> -----
>
>   /branches/1.4/src/core/src/main/java/org/apache/accumulo/core/Constants.java 1355540
>   /branches/1.4/src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooUtil.java 1355540
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/client/BulkImporter.java 1355540
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/master/tableOps/BulkImport.java 1355540
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/tabletserver/BIFCopyProcessor.java PRE-CREATION
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/tabletserver/TabletServer.java 1355540
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/zookeeper/DistributedWorkQueue.java PRE-CREATION
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/zookeeper/IZooReaderWriter.java 1355540
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/zookeeper/ZooReaderWriter.java 1355540
>   /branches/1.4/test/system/test4/bulk_import_test.sh 1355540
>
> Diff: https://reviews.apache.org/r/5684/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> kturner
>
>

> On July 2, 2012, 3:13 p.m., Adam Fuchs wrote:
> > /branches/1.4/src/core/src/main/java/org/apache/accumulo/core/Constants.java, line 80
> > <https://reviews.apache.org/r/5684/diff/1/?file=117631#file117631line80>
> >
> >     Is it obvious to the general developer who might be trying to read this code that BIF stands for bulk import failure? Maybe a better naming scheme for variables, constants, and directories is in order.

Its not obvious
> On July 2, 2012, 3:13 p.m., Adam Fuchs wrote:
> > /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/master/tableOps/BulkImport.java, line 394
> > <https://reviews.apache.org/r/5684/diff/1/?file=117634#file117634line394>
> >
> >     Also need to check whether a concurrent operation could be in progress that writes the load flag after we check for its existence.

the code in CleanUpBulkImport.isReady() needs to happen before CopyFailed
> On July 2, 2012, 3:13 p.m., Adam Fuchs wrote:
> > /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/tabletserver/TabletServer.java, line 2687
> > <https://reviews.apache.org/r/5684/diff/1/?file=117636#file117636line2687>
> >
> >     Fixed-sized thread pool seems like a problem for scaling to different-sized machines. Also, what does 2 threads imply given the thread pool queue size checks in DistributedWorkQueue? Seems like if we generalize the thread pool size that would cause problems.
> >    
> >     Another thought here: we do a bunch of stuff that is distributed through tablet servers. Does it make sense to have threads dedicated to this particular distrubted task, or should this be part of a generalized framework?

Probably should make it configurable, yet another knob.

The code is general, in 1.5 I plan to use the DistributedWorkQueue for write ahead log sorts.  The code was taken from 1.5 log sorts and generalized along with a fixing a zookeeper locking bug.  When this is merged to 1.5, the existing code can be modified to use it.
> On July 2, 2012, 3:13 p.m., Adam Fuchs wrote:
> > /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/zookeeper/DistributedWorkQueue.java, line 52
> > <https://reviews.apache.org/r/5684/diff/1/?file=117637#file117637line52>
> >
> >     What is special about a queue size of greater than one? Also, this code is repeated in several spots -- should it be turned into a method?

I am not sure if there is way to tell if all threads are busy in the pool other than the fact that at least one thing is queued.  Basically if all threads are busy, then the tablet server should not reserve it because another tablet server my have processing capacity.
- kturner
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5684/#review8792
-----------------------------------------------------------
On June 30, 2012, 3:10 a.m., kturner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5684/
> -----------------------------------------------------------
>
> (Updated June 30, 2012, 3:10 a.m.)
>
>
> Review request for accumulo.
>
>
> Description
> -------
>
> A patch that fixes the issue of failed bulk imports being copied through the master.
>
>
> This addresses bug ACCUMULO-409.
>     https://issues.apache.org/jira/browse/ACCUMULO-409
>
>
> Diffs
> -----
>
>   /branches/1.4/src/core/src/main/java/org/apache/accumulo/core/Constants.java 1355540
>   /branches/1.4/src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooUtil.java 1355540
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/client/BulkImporter.java 1355540
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/master/tableOps/BulkImport.java 1355540
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/tabletserver/BIFCopyProcessor.java PRE-CREATION
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/tabletserver/TabletServer.java 1355540

> On July 2, 2012, 3:13 p.m., Adam Fuchs wrote:
> > /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/zookeeper/DistributedWorkQueue.java, line 52
> > <https://reviews.apache.org/r/5684/diff/1/?file=117637#file117637line52>
> >
> >     What is special about a queue size of greater than one? Also, this code is repeated in several spots -- should it be turned into a method?
>
> kturner wrote:
>     I am not sure if there is way to tell if all threads are busy in the pool other than the fact that at least one thing is queued.  Basically if all threads are busy, then the tablet server should not reserve it because another tablet server my have processing capacity.

The condition would be > 0 and not > 1, right?

Seems like there's a potential race condition, where the first thread would just empty the queue and none of the other threads would make progress.
- Adam
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5684/#review8792
-----------------------------------------------------------
On June 30, 2012, 3:10 a.m., kturner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5684/
> -----------------------------------------------------------
>
> (Updated June 30, 2012, 3:10 a.m.)
>
>
> Review request for accumulo.
>
>
> Description
> -------
>
> A patch that fixes the issue of failed bulk imports being copied through the master.
>
>
> This addresses bug ACCUMULO-409.
>     https://issues.apache.org/jira/browse/ACCUMULO-409
>
>
> Diffs
> -----
>
>   /branches/1.4/src/core/src/main/java/org/apache/accumulo/core/Constants.java 1355540
>   /branches/1.4/src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooUtil.java 1355540
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/client/BulkImporter.java 1355540
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/master/tableOps/BulkImport.java 1355540
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/tabletserver/BIFCopyProcessor.java PRE-CREATION
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/tabletserver/TabletServer.java 1355540
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/zookeeper/DistributedWorkQueue.java PRE-CREATION
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/zookeeper/IZooReaderWriter.java 1355540
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/zookeeper/ZooReaderWriter.java 1355540
>   /branches/1.4/test/system/test4/bulk_import_test.sh 1355540
>
> Diff: https://reviews.apache.org/r/5684/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> kturner
>
>

> On July 2, 2012, 3:13 p.m., Adam Fuchs wrote:
> > /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/zookeeper/DistributedWorkQueue.java, line 52
> > <https://reviews.apache.org/r/5684/diff/1/?file=117637#file117637line52>
> >
> >     What is special about a queue size of greater than one? Also, this code is repeated in several spots -- should it be turned into a method?
>
> kturner wrote:
>     I am not sure if there is way to tell if all threads are busy in the pool other than the fact that at least one thing is queued.  Basically if all threads are busy, then the tablet server should not reserve it because another tablet server my have processing capacity.
>
> Adam Fuchs wrote:
>     The condition would be > 0 and not > 1, right?
>    
>     Seems like there's a potential race condition, where the first thread would just empty the queue and none of the other threads would make progress.

Could increment an atomic long before adding a task to the thread pool and then decrement the atomic long when a task finishes.  Could check this atomic long against the thread pool size before adding to the pool.  This count will include queued and running.  Need to decrement in a finally block of the task.
- kturner
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5684/#review8792
-----------------------------------------------------------
On June 30, 2012, 3:10 a.m., kturner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5684/
> -----------------------------------------------------------
>
> (Updated June 30, 2012, 3:10 a.m.)
>
>
> Review request for accumulo.
>
>
> Description
> -------
>
> A patch that fixes the issue of failed bulk imports being copied through the master.
>
>
> This addresses bug ACCUMULO-409.
>     https://issues.apache.org/jira/browse/ACCUMULO-409
>
>
> Diffs
> -----
>
>   /branches/1.4/src/core/src/main/java/org/apache/accumulo/core/Constants.java 1355540
>   /branches/1.4/src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooUtil.java 1355540
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/client/BulkImporter.java 1355540
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/master/tableOps/BulkImport.java 1355540
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/tabletserver/BIFCopyProcessor.java PRE-CREATION
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/tabletserver/TabletServer.java 1355540
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/zookeeper/DistributedWorkQueue.java PRE-CREATION
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/zookeeper/IZooReaderWriter.java 1355540
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/zookeeper/ZooReaderWriter.java 1355540
>   /branches/1.4/test/system/test4/bulk_import_test.sh 1355540
>
> Diff: https://reviews.apache.org/r/5684/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> kturner
>
>

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5684/
-----------------------------------------------------------

(Updated July 2, 2012, 9:56 p.m.)
Review request for accumulo.
Changes
-------

Updated patch based on comments
Description
-------

A patch that fixes the issue of failed bulk imports being copied through the master.
This addresses bug ACCUMULO-409.
    https://issues.apache.org/jira/browse/ACCUMULO-409
Diffs (updated)
-----

  /branches/1.4/src/core/src/main/java/org/apache/accumulo/core/Constants.java 1356490
  /branches/1.4/src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooUtil.java 1356490
  /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/client/BulkImporter.java 1356490
  /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/master/tableOps/BulkImport.java 1356490
  /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/tabletserver/BulkFailedCopyProcessor.java PRE-CREATION
  /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/tabletserver/TabletServer.java 1356490
  /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/zookeeper/DistributedWorkQueue.java PRE-CREATION
  /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/zookeeper/IZooReaderWriter.java 1356490
  /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/zookeeper/ZooReaderWriter.java 1356490
  /branches/1.4/test/system/test4/bulk_import_test.sh 1356490

Diff: https://reviews.apache.org/r/5684/diff/
Testing
-------
Thanks,

kturner

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5684/
-----------------------------------------------------------

(Updated July 3, 2012, 12:38 p.m.)
Review request for accumulo.
Changes
-------

made number of threads configurable
Description
-------

A patch that fixes the issue of failed bulk imports being copied through the master.
This addresses bug ACCUMULO-409.
    https://issues.apache.org/jira/browse/ACCUMULO-409
Diffs (updated)
-----

  /branches/1.4/src/core/src/main/java/org/apache/accumulo/core/Constants.java 1356670
  /branches/1.4/src/core/src/main/java/org/apache/accumulo/core/conf/Property.java 1356670
  /branches/1.4/src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooUtil.java 1356670
  /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/client/BulkImporter.java 1356670
  /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/master/tableOps/BulkImport.java 1356670
  /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/tabletserver/BulkFailedCopyProcessor.java PRE-CREATION
  /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/tabletserver/TabletServer.java 1356670
  /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/zookeeper/DistributedWorkQueue.java PRE-CREATION
  /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/zookeeper/IZooReaderWriter.java 1356670
  /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/zookeeper/ZooReaderWriter.java 1356670
  /branches/1.4/test/system/test4/bulk_import_test.sh 1356670

Diff: https://reviews.apache.org/r/5684/diff/
Testing
-------
Thanks,

kturner

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5684/#review8832
-----------------------------------------------------------

/branches/1.4/src/server/src/main/java/org/apache/accumulo/server/master/tableOps/BulkImport.java
<https://reviews.apache.org/r/5684/#comment18744>

    Prefer for the master to expose the FileSystem it is using.

/branches/1.4/src/server/src/main/java/org/apache/accumulo/server/master/tableOps/BulkImport.java
<https://reviews.apache.org/r/5684/#comment18745>

    needs try {} finally { out.close(); }

/branches/1.4/src/server/src/main/java/org/apache/accumulo/server/zookeeper/DistributedWorkQueue.java
<https://reviews.apache.org/r/5684/#comment18746>

    Could use a short summary.  Suggest:
    
    Worker processes watch a zookeeper node for tasks to be performed.  After getting an exclusive lock on the node, the worker will perform the task.  
- Eric Newton
On July 3, 2012, 12:38 p.m., kturner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5684/
> -----------------------------------------------------------
>
> (Updated July 3, 2012, 12:38 p.m.)
>
>
> Review request for accumulo.
>
>
> Description
> -------
>
> A patch that fixes the issue of failed bulk imports being copied through the master.
>
>
> This addresses bug ACCUMULO-409.
>     https://issues.apache.org/jira/browse/ACCUMULO-409
>
>
> Diffs
> -----
>
>   /branches/1.4/src/core/src/main/java/org/apache/accumulo/core/Constants.java 1356670
>   /branches/1.4/src/core/src/main/java/org/apache/accumulo/core/conf/Property.java 1356670
>   /branches/1.4/src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooUtil.java 1356670
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/client/BulkImporter.java 1356670
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/master/tableOps/BulkImport.java 1356670
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/tabletserver/BulkFailedCopyProcessor.java PRE-CREATION
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/tabletserver/TabletServer.java 1356670
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/zookeeper/DistributedWorkQueue.java PRE-CREATION
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/zookeeper/IZooReaderWriter.java 1356670
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/zookeeper/ZooReaderWriter.java 1356670
>   /branches/1.4/test/system/test4/bulk_import_test.sh 1356670
>
> Diff: https://reviews.apache.org/r/5684/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> kturner
>
>

> On July 3, 2012, 1:45 p.m., Eric Newton wrote:
> > /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/master/tableOps/BulkImport.java, line 390
> > <https://reviews.apache.org/r/5684/diff/3/?file=119229#file119229line390>
> >
> >     Prefer for the master to expose the FileSystem it is using.

I will do this when merging to trunk
- kturner
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5684/#review8832
-----------------------------------------------------------
On July 3, 2012, 12:38 p.m., kturner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5684/
> -----------------------------------------------------------
>
> (Updated July 3, 2012, 12:38 p.m.)
>
>
> Review request for accumulo.
>
>
> Description
> -------
>
> A patch that fixes the issue of failed bulk imports being copied through the master.
>
>
> This addresses bug ACCUMULO-409.
>     https://issues.apache.org/jira/browse/ACCUMULO-409
>
>
> Diffs
> -----
>
>   /branches/1.4/src/core/src/main/java/org/apache/accumulo/core/Constants.java 1356670
>   /branches/1.4/src/core/src/main/java/org/apache/accumulo/core/conf/Property.java 1356670
>   /branches/1.4/src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooUtil.java 1356670
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/client/BulkImporter.java 1356670
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/master/tableOps/BulkImport.java 1356670
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/tabletserver/BulkFailedCopyProcessor.java PRE-CREATION
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/tabletserver/TabletServer.java 1356670
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/zookeeper/DistributedWorkQueue.java PRE-CREATION
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/zookeeper/IZooReaderWriter.java 1356670
>   /branches/1.4/src/server/src/main/java/org/apache/accumulo/server/zookeeper/ZooReaderWriter.java 1356670
>   /branches/1.4/test/system/test4/bulk_import_test.sh 1356670
>
> Diff: https://reviews.apache.org/r/5684/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> kturner
>
>

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5911/
-----------------------------------------------------------

Review request for accumulo.
Description
-------

A patch that dynamically creates transient indexes on cached rfile blocks.
This addresses bug ACCUMULO-473.
    https://issues.apache.org/jira/browse/ACCUMULO-473
Diffs
-----

  /trunk/core/src/main/java/org/apache/accumulo/core/file/blockfile/cache/CachedBlock.java 1359683
  /trunk/core/src/main/java/org/apache/accumulo/core/file/blockfile/cache/LruBlockCache.java 1359683
  /trunk/core/src/main/java/org/apache/accumulo/core/file/blockfile/impl/CachableBlockFile.java 1359683
  /trunk/core/src/main/java/org/apache/accumulo/core/file/rfile/BlockIndex.java PRE-CREATION
  /trunk/core/src/main/java/org/apache/accumulo/core/file/rfile/RFile.java 1359683
  /trunk/core/src/main/java/org/apache/accumulo/core/file/rfile/RelativeKey.java 1359683
  /trunk/core/src/test/java/org/apache/accumulo/core/file/rfile/RFileTest.java 1359683

Diff: https://reviews.apache.org/r/5911/diff/
Testing
-------
Thanks,

kturner

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5911/
-----------------------------------------------------------

(Updated July 12, 2012, 11:38 p.m.)
Review request for accumulo.
Changes
-------

Someone suggested the previous patch went past interfaces. In this patch, RFile and the Cache are now cleanly separated.
Description
-------

A patch that dynamically creates transient indexes on cached rfile blocks.
This addresses bug ACCUMULO-473.
    https://issues.apache.org/jira/browse/ACCUMULO-473
Diffs (updated)
-----

  /trunk/core/src/main/java/org/apache/accumulo/core/file/blockfile/ABlockReader.java 1359683
  /trunk/core/src/main/java/org/apache/accumulo/core/file/blockfile/cache/BlockCache.java 1359683
  /trunk/core/src/main/java/org/apache/accumulo/core/file/blockfile/cache/CacheEntry.java 1359683
  /trunk/core/src/main/java/org/apache/accumulo/core/file/blockfile/cache/CachedBlock.java 1359683
  /trunk/core/src/main/java/org/apache/accumulo/core/file/blockfile/cache/LruBlockCache.java 1359683
  /trunk/core/src/main/java/org/apache/accumulo/core/file/blockfile/cache/SimpleBlockCache.java 1359683
  /trunk/core/src/main/java/org/apache/accumulo/core/file/blockfile/impl/CachableBlockFile.java 1359683
  /trunk/core/src/main/java/org/apache/accumulo/core/file/rfile/BlockIndex.java PRE-CREATION
  /trunk/core/src/main/java/org/apache/accumulo/core/file/rfile/RFile.java 1359683
  /trunk/core/src/main/java/org/apache/accumulo/core/file/rfile/RelativeKey.java 1359683
  /trunk/core/src/test/java/org/apache/accumulo/core/file/blockfile/cache/TestLruBlockCache.java 1359683
  /trunk/core/src/test/java/org/apache/accumulo/core/file/rfile/RFileTest.java 1359683

Diff: https://reviews.apache.org/r/5911/diff/
Testing
-------
Thanks,

kturner

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5911/#review9104
-----------------------------------------------------------

/trunk/core/src/main/java/org/apache/accumulo/core/file/blockfile/impl/CachableBlockFile.java
<https://reviews.apache.org/r/5911/#comment19298>

    Does the code match the comment?  I'm missing where the key is being passed.

/trunk/core/src/main/java/org/apache/accumulo/core/file/blockfile/impl/CachableBlockFile.java
<https://reviews.apache.org/r/5911/#comment19299>

    Spelling: lenght
- Eric Newton
On July 12, 2012, 11:38 p.m., kturner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5911/
> -----------------------------------------------------------
>
> (Updated July 12, 2012, 11:38 p.m.)
>
>
> Review request for accumulo.
>
>
> Description
> -------
>
> A patch that dynamically creates transient indexes on cached rfile blocks.
>
>
> This addresses bug ACCUMULO-473.
>     https://issues.apache.org/jira/browse/ACCUMULO-473
>
>
> Diffs
> -----
>
>   /trunk/core/src/main/java/org/apache/accumulo/core/file/blockfile/ABlockReader.java 1359683
>   /trunk/core/src/main/java/org/apache/accumulo/core/file/blockfile/cache/BlockCache.java 1359683
>   /trunk/core/src/main/java/org/apache/accumulo/core/file/blockfile/cache/CacheEntry.java 1359683
>   /trunk/core/src/main/java/org/apache/accumulo/core/file/blockfile/cache/CachedBlock.java 1359683
>   /trunk/core/src/main/java/org/apache/accumulo/core/file/blockfile/cache/LruBlockCache.java 1359683
>   /trunk/core/src/main/java/org/apache/accumulo/core/file/blockfile/cache/SimpleBlockCache.java 1359683
>   /trunk/core/src/main/java/org/apache/accumulo/core/file/blockfile/impl/CachableBlockFile.java 1359683
>   /trunk/core/src/main/java/org/apache/accumulo/core/file/rfile/BlockIndex.java PRE-CREATION
>   /trunk/core/src/main/java/org/apache/accumulo/core/file/rfile/RFile.java 1359683
>   /trunk/core/src/main/java/org/apache/accumulo/core/file/rfile/RelativeKey.java 1359683
>   /trunk/core/src/test/java/org/apache/accumulo/core/file/blockfile/cache/TestLruBlockCache.java 1359683
>   /trunk/core/src/test/java/org/apache/accumulo/core/file/rfile/RFileTest.java 1359683
>
> Diff: https://reviews.apache.org/r/5911/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> kturner
>
>