----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/196/#review83 -----------------------------------------------------------
http://svn.apache.org/repos/asf/hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/BlockManager.java <https://reviews.apache.org/r/196/#comment146> maybe assert namesystem.hasWriteLock() here? I think it's all correct, but provides good documentation. http://svn.apache.org/repos/asf/hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/BlockManager.java <https://reviews.apache.org/r/196/#comment144> should probably note that these are "out" parameters http://svn.apache.org/repos/asf/hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/BlockManager.java <https://reviews.apache.org/r/196/#comment147> excessReplicateTmp.isEmpty()? http://svn.apache.org/repos/asf/hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/BlockManager.java <https://reviews.apache.org/r/196/#comment148> !originalDatanodes.isEmpty() http://svn.apache.org/repos/asf/hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/BlockManager.java <https://reviews.apache.org/r/196/#comment149> can you keep this as a separate addToExcessReplicate function? http://svn.apache.org/repos/asf/hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/BlockManager.java <https://reviews.apache.org/r/196/#comment151> add back the isDebugEnabled guard? http://svn.apache.org/repos/asf/hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/BlockManager.java <https://reviews.apache.org/r/196/#comment150> this is just moved code, but this comment should say "instructions to the datanode" right? http://svn.apache.org/repos/asf/hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/BlockManager.java <https://reviews.apache.org/r/196/#comment152> this is already logged inside addToInvalidates. Perhaps pass third argument false to that function so it doesn't log, if you want the specialized message? http://svn.apache.org/repos/asf/hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java <https://reviews.apache.org/r/196/#comment145> update javadoc to explain excessReplicateMapTmp is where the results end up Also maybe worth noting here and in the replicator interface that 'inode' here might get modified by another thread during execution? - Todd On 2010-12-28 11:12:46, Dhruba Borthakur wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/196/ > ----------------------------------------------------------- > > (Updated 2010-12-28 11:12:46) > > > Review request for hadoop-hdfs. > > > Summary > ------- > > Make exiting safemode a lot faster. > > > This addresses bug HDFS-1391. > https://issues.apache.org/jira/browse/HDFS-1391 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/BlockManager.java > 1053405 > > http://svn.apache.org/repos/asf/hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java > 1053405 > > http://svn.apache.org/repos/asf/hadoop/hdfs/trunk/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestOverReplicatedBlocks.java > 1053405 > > Diff: https://reviews.apache.org/r/196/diff > > > Testing > ------- > > Running on our biggest production cluster of 3000 nodes since Dec 13th > > > Thanks, > > Dhruba > >
