----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2150/#review2277 -----------------------------------------------------------
just a few nits, mostly looks good. A few questions I have that aren't directly related to this patch: - is SafeMode now a replicated thing, or does each NN separately enter safemode? I think the latter, right? - when transitioning between states, what happens if the "enterState" fails for the new state? The state variable will then indicate it's in that state, when in fact it's in no state at all. How do we recover from that? We need some kind of rollback? (eg if you're in standby and try to transition to active, but find that you can't take a lock in ZK) branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java <https://reviews.apache.org/r/2150/#comment5248> typo: secrect branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java <https://reviews.apache.org/r/2150/#comment5249> any reason that you switched the order of startHttpServer to the end of this function? I don't think it's a big deal, but there's some possibility the service plugins may want to do something with the http server, which wouldn't be started yet. branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java <https://reviews.apache.org/r/2150/#comment5251> this should at least be warn, right? and passing e as the second argument is better since it shows the trace. - Todd On 2011-10-03 18:36:41, Todd Lipcon wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/2150/ > ----------------------------------------------------------- > > (Updated 2011-10-03 18:36:41) > > > Review request for hadoop-hdfs and Todd Lipcon. > > > Summary > ------- > > Uploading Suresh's patch to reviewboard > (https://issues.apache.org/jira/secure/attachment/12496953/HDFS-2301.txt from > 29/Sep/11 00:56) > > > This addresses bug HDFS-2301. > https://issues.apache.org/jira/browse/HDFS-2301 > > > Diffs > ----- > > > branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/BackupNode.java > 1177130 > > branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java > 1177130 > > branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java > 1177130 > > branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/ActiveState.java > 1177128 > > branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/HAContext.java > PRE-CREATION > > branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/HAState.java > 1177128 > > branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/StandbyState.java > 1177128 > > Diff: https://reviews.apache.org/r/2150/diff > > > Testing > ------- > > > Thanks, > > Todd > >
