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