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