Uploaded image for project: 'Hadoop HDFS'
  1. Hadoop HDFS
  2. HDFS-1623 High Availability Framework for HDFS NN
  3. HDFS-2301

Start/stop appropriate namenode internal services during transition to active and standby

    Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: HA branch (HDFS-1623)
    • Fix Version/s: HA branch (HDFS-1623)
    • Component/s: ha, namenode
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      These changes are related to HDFS-1974 which introduced active and standby states. This jira will address starting and stopping appropriate NN services when entering and existing active and standby states.

      1. HDFS-2301.txt
        27 kB
        Suresh Srinivas
      2. HDFS-2301.txt
        26 kB
        Suresh Srinivas
      3. HDFS-2301.txt
        40 kB
        Suresh Srinivas
      4. HDFS-2301.txt
        25 kB
        Suresh Srinivas
      5. HDFS-2301.txt
        26 kB
        Suresh Srinivas
      6. HDFS-2301.txt
        25 kB
        Suresh Srinivas

        Issue Links

          Activity

          Hide
          sureshms Suresh Srinivas added a comment - - edited

          Current behavior:

          Namenode starts the following services:

          1. FSNamesystem secret manager
          2. FSNamesystem activation performs:
            • NNResourceChecker, activate block manager, starts lease manager thread, resource monitor threads and register mbeans and initializes metrics.
          3. Starts http server, rpc server, trash emptier, service plugins

          Backup node starts all the services above except:

          1. secret manager
          2. Sets lease period to MAX_VALUE, effectively not use lease manager.

          Proposed behavior:

          Following services are started as common to both active and standby namenode states:

          1. Starts common FSNamesystem common services to both active and standby
            • NNResourceChecker, activate block manager, resource monitor threads and register mbeans and initializes metrics.
          2. Starts http server, rpc server, service plugins

          Following services are started when moving to active state and stopped when exiting active state:

          1. Start/stops FSNamesystem active services:
            • Which starts/stop FSNamesystem secrtet manager and lease manager.

          Currently standby state services are marked as TODO. This is where reading editlogs from active will be handled.

          BackupNode with this patch will no longer start lease manager thread.

          Code changes

          1. HAState no longer knows about NameNode. Instead NameNode exposes HAContext
          2. Reorganized and cleaned up the code for starting and stopping services.
          Show
          sureshms Suresh Srinivas added a comment - - edited Current behavior: Namenode starts the following services: FSNamesystem secret manager FSNamesystem activation performs: NNResourceChecker, activate block manager, starts lease manager thread, resource monitor threads and register mbeans and initializes metrics. Starts http server, rpc server, trash emptier, service plugins Backup node starts all the services above except: secret manager Sets lease period to MAX_VALUE, effectively not use lease manager. Proposed behavior: Following services are started as common to both active and standby namenode states: Starts common FSNamesystem common services to both active and standby NNResourceChecker, activate block manager, resource monitor threads and register mbeans and initializes metrics. Starts http server, rpc server, service plugins Following services are started when moving to active state and stopped when exiting active state: Start/stops FSNamesystem active services: Which starts/stop FSNamesystem secrtet manager and lease manager. Currently standby state services are marked as TODO. This is where reading editlogs from active will be handled. BackupNode with this patch will no longer start lease manager thread. Code changes HAState no longer knows about NameNode. Instead NameNode exposes HAContext Reorganized and cleaned up the code for starting and stopping services.
          Hide
          sureshms Suresh Srinivas added a comment -

          Preliminary patch attached...

          Show
          sureshms Suresh Srinivas added a comment - Preliminary patch attached...
          Hide
          jiraposter@reviews.apache.org jiraposter@reviews.apache.org added a comment -

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

          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

          Show
          jiraposter@reviews.apache.org jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2150/ ----------------------------------------------------------- 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
          Hide
          jiraposter@reviews.apache.org jiraposter@reviews.apache.org added a comment -

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

          Show
          jiraposter@reviews.apache.org jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- 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
          Hide
          sureshms Suresh Srinivas added a comment -

          New patch addresses some of the comment. See review board for my responses to other comments. BTW, how does the comments I posted on review board make it to jira?

          Show
          sureshms Suresh Srinivas added a comment - New patch addresses some of the comment. See review board for my responses to other comments. BTW, how does the comments I posted on review board make it to jira?
          Hide
          sureshms Suresh Srinivas added a comment -

          New patch with missing files included.

          Show
          sureshms Suresh Srinivas added a comment - New patch with missing files included.
          Hide
          tlipcon Todd Lipcon added a comment -

          Did you click on "Publish" on review board? I don't see your responses there. Once you publish, it will copy to JIRA and email.

          Show
          tlipcon Todd Lipcon added a comment - Did you click on "Publish" on review board? I don't see your responses there. Once you publish, it will copy to JIRA and email.
          Hide
          jiraposter@reviews.apache.org jiraposter@reviews.apache.org added a comment -

          On 2011-10-03 18:58:13, Todd Lipcon wrote:

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

          is SafeMode now a replicated thing, or does each NN separately enter safemode? I think the latter, right?

          Safemode is the state of namespace(FSNamesystem), unlike active, standby which are the states of the namenode. Each NN separately enters safemode.

          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)

          This is tricky. Say enterState fails to start services because of some namenode process related issues. Then most likely rolling back to previous state, and starting services relevant to previous states will also fail. The particular example you are bringing up related to ZK, I think failover controller is the one that deals with ZK and not namenode.

          I can think of two solutions: namenode shutsdown when this happens (as done during startup) or move to a failed state.

          On 2011-10-03 18:58:13, Todd Lipcon wrote:

          > branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java, line 464

          > <https://reviews.apache.org/r/2150/diff/1/?file=47529#file47529line464>

          >

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

          No particular reason. Not sure who uses ServicePlugins. But the description says it is RPC related. But will move it back up.

          • Suresh

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

          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

          Show
          jiraposter@reviews.apache.org jiraposter@reviews.apache.org added a comment - On 2011-10-03 18:58:13, Todd Lipcon wrote: > 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) is SafeMode now a replicated thing, or does each NN separately enter safemode? I think the latter, right? Safemode is the state of namespace(FSNamesystem), unlike active, standby which are the states of the namenode. Each NN separately enters safemode. 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) This is tricky. Say enterState fails to start services because of some namenode process related issues. Then most likely rolling back to previous state, and starting services relevant to previous states will also fail. The particular example you are bringing up related to ZK, I think failover controller is the one that deals with ZK and not namenode. I can think of two solutions: namenode shutsdown when this happens (as done during startup) or move to a failed state. On 2011-10-03 18:58:13, Todd Lipcon wrote: > branches/ HDFS-1623 /hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java, line 464 > < https://reviews.apache.org/r/2150/diff/1/?file=47529#file47529line464 > > > 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. No particular reason. Not sure who uses ServicePlugins. But the description says it is RPC related. But will move it back up. Suresh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2150/#review2277 ----------------------------------------------------------- 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
          Hide
          jiraposter@reviews.apache.org jiraposter@reviews.apache.org added a comment -

          On 2011-10-03 18:58:13, Todd Lipcon wrote:

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

          Suresh Srinivas wrote:

          > is SafeMode now a replicated thing, or does each NN separately enter safemode? I think the latter, right?

          Safemode is the state of namespace(FSNamesystem), unlike active, standby which are the states of the namenode. Each NN separately enters safemode.

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

          This is tricky. Say enterState fails to start services because of some namenode process related issues. Then most likely rolling back to previous state, and starting services relevant to previous states will also fail. The particular example you are bringing up related to ZK, I think failover controller is the one that deals with ZK and not namenode.

          I can think of two solutions: namenode shutsdown when this happens (as done during startup) or move to a failed state.

          Let's just add a TODO for now that we need to consider these situations in a test plan. I imagine the most likely real scenario is that you try to do a failover, but for some reason the standby has an IO problem trying to read the latest logs from the primary (eg maybe the primary barfed some bad data into the edit logs as it crashed, or maybe the primary crashed because the shared storage caught on fire).

          On 2011-10-03 18:58:13, Todd Lipcon wrote:

          > branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java, line 464

          > <https://reviews.apache.org/r/2150/diff/1/?file=47529#file47529line464>

          >

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

          Suresh Srinivas wrote:

          No particular reason. Not sure who uses ServicePlugins. But the description says it is RPC related. But will move it back up.

          Hue currently uses service plugins to expose a Thrift interface. But with Sanjay's recent work on protocol adapters, this may be largely unnecessary in the future. Nonetheless, we should leave it around

          • Todd

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

          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

          Show
          jiraposter@reviews.apache.org jiraposter@reviews.apache.org added a comment - On 2011-10-03 18:58:13, Todd Lipcon wrote: > 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) Suresh Srinivas wrote: > is SafeMode now a replicated thing, or does each NN separately enter safemode? I think the latter, right? Safemode is the state of namespace(FSNamesystem), unlike active, standby which are the states of the namenode. Each NN separately enters safemode. > 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) This is tricky. Say enterState fails to start services because of some namenode process related issues. Then most likely rolling back to previous state, and starting services relevant to previous states will also fail. The particular example you are bringing up related to ZK, I think failover controller is the one that deals with ZK and not namenode. I can think of two solutions: namenode shutsdown when this happens (as done during startup) or move to a failed state. Let's just add a TODO for now that we need to consider these situations in a test plan. I imagine the most likely real scenario is that you try to do a failover, but for some reason the standby has an IO problem trying to read the latest logs from the primary (eg maybe the primary barfed some bad data into the edit logs as it crashed, or maybe the primary crashed because the shared storage caught on fire). On 2011-10-03 18:58:13, Todd Lipcon wrote: > branches/ HDFS-1623 /hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java, line 464 > < https://reviews.apache.org/r/2150/diff/1/?file=47529#file47529line464 > > > 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. Suresh Srinivas wrote: No particular reason. Not sure who uses ServicePlugins. But the description says it is RPC related. But will move it back up. Hue currently uses service plugins to expose a Thrift interface. But with Sanjay's recent work on protocol adapters, this may be largely unnecessary in the future. Nonetheless, we should leave it around Todd ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2150/#review2277 ----------------------------------------------------------- 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
          Hide
          jiraposter@reviews.apache.org jiraposter@reviews.apache.org added a comment -

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

          (Updated 2011-10-06 23:25:18.391776)

          Review request for hadoop-hdfs and Todd Lipcon.

          Changes
          -------

          updating reviewboard patch to Suresh's new revision

          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 (updated)


          branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/HAUtil.java PRE-CREATION
          branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/BackupNode.java 1179521
          branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java 1179521
          branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java 1179521
          branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/ActiveState.java 1179521
          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 1179521
          branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/StandbyState.java 1179521
          branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestHAUtil.java PRE-CREATION

          Diff: https://reviews.apache.org/r/2150/diff

          Testing
          -------

          Thanks,

          Todd

          Show
          jiraposter@reviews.apache.org jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2150/ ----------------------------------------------------------- (Updated 2011-10-06 23:25:18.391776) Review request for hadoop-hdfs and Todd Lipcon. Changes ------- updating reviewboard patch to Suresh's new revision 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 (updated) branches/ HDFS-1623 /hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/HAUtil.java PRE-CREATION branches/ HDFS-1623 /hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/BackupNode.java 1179521 branches/ HDFS-1623 /hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java 1179521 branches/ HDFS-1623 /hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java 1179521 branches/ HDFS-1623 /hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/ActiveState.java 1179521 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 1179521 branches/ HDFS-1623 /hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/StandbyState.java 1179521 branches/ HDFS-1623 /hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestHAUtil.java PRE-CREATION Diff: https://reviews.apache.org/r/2150/diff Testing ------- Thanks, Todd
          Hide
          jiraposter@reviews.apache.org jiraposter@reviews.apache.org added a comment -

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

          branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/HAUtil.java
          <https://reviews.apache.org/r/2150/#comment5514>

          perhaps should be abstract since it won't ever be instantiated?

          these functions are meant only for the server side, right? Otherwise they should all take an authority, and look at configs prefixed/suffixed with that authority?

          let me jump over to HDFS-2231 and try to review that first.. having a hard time following this.

          branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/HAUtil.java
          <https://reviews.apache.org/r/2150/#comment5515>

          !collection.isEmpty()

          branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/HAUtil.java
          <https://reviews.apache.org/r/2150/#comment5516>

          long line

          branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/HAUtil.java
          <https://reviews.apache.org/r/2150/#comment5518>

          strange formatting

          branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/HAUtil.java
          <https://reviews.apache.org/r/2150/#comment5517>

          so this patch now depends on HDFS-2231 (Conf changes for HA NN), right?

          • Todd

          On 2011-10-06 23:25:18, Todd Lipcon wrote:

          -----------------------------------------------------------

          This is an automatically generated e-mail. To reply, visit:

          https://reviews.apache.org/r/2150/

          -----------------------------------------------------------

          (Updated 2011-10-06 23:25:18)

          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/HAUtil.java PRE-CREATION

          branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/BackupNode.java 1179521

          branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java 1179521

          branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java 1179521

          branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/ActiveState.java 1179521

          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 1179521

          branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/StandbyState.java 1179521

          branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestHAUtil.java PRE-CREATION

          Diff: https://reviews.apache.org/r/2150/diff

          Testing

          -------

          Thanks,

          Todd

          Show
          jiraposter@reviews.apache.org jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2150/#review2422 ----------------------------------------------------------- branches/ HDFS-1623 /hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/HAUtil.java < https://reviews.apache.org/r/2150/#comment5514 > perhaps should be abstract since it won't ever be instantiated? these functions are meant only for the server side, right? Otherwise they should all take an authority, and look at configs prefixed/suffixed with that authority? let me jump over to HDFS-2231 and try to review that first.. having a hard time following this. branches/ HDFS-1623 /hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/HAUtil.java < https://reviews.apache.org/r/2150/#comment5515 > !collection.isEmpty() branches/ HDFS-1623 /hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/HAUtil.java < https://reviews.apache.org/r/2150/#comment5516 > long line branches/ HDFS-1623 /hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/HAUtil.java < https://reviews.apache.org/r/2150/#comment5518 > strange formatting branches/ HDFS-1623 /hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/HAUtil.java < https://reviews.apache.org/r/2150/#comment5517 > so this patch now depends on HDFS-2231 (Conf changes for HA NN), right? Todd On 2011-10-06 23:25:18, Todd Lipcon wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2150/ ----------------------------------------------------------- (Updated 2011-10-06 23:25:18) 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/HAUtil.java PRE-CREATION branches/ HDFS-1623 /hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/BackupNode.java 1179521 branches/ HDFS-1623 /hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java 1179521 branches/ HDFS-1623 /hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java 1179521 branches/ HDFS-1623 /hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/ActiveState.java 1179521 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 1179521 branches/ HDFS-1623 /hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/StandbyState.java 1179521 branches/ HDFS-1623 /hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestHAUtil.java PRE-CREATION Diff: https://reviews.apache.org/r/2150/diff Testing ------- Thanks, Todd
          Hide
          jiraposter@reviews.apache.org jiraposter@reviews.apache.org added a comment -

          On 2011-10-06 23:37:24, Todd Lipcon wrote:

          > branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/HAUtil.java, line 28

          > <https://reviews.apache.org/r/2150/diff/2/?file=48551#file48551line28>

          >

          > perhaps should be abstract since it won't ever be instantiated?

          >

          > these functions are meant only for the server side, right? Otherwise they should all take an authority, and look at configs prefixed/suffixed with that authority?

          >

          > let me jump over to HDFS-2231 and try to review that first.. having a hard time following this.

          Class has private constructor. I prefer that to abstract class. For some reason, I included HAUtil in this patch causing lot of confusion. It belongs to 2231. So I am attaching a patch without HAUtil changes. Also I will address rest of your HAUtil comments in 2231.

          • Suresh

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

          On 2011-10-06 23:25:18, Todd Lipcon wrote:

          -----------------------------------------------------------

          This is an automatically generated e-mail. To reply, visit:

          https://reviews.apache.org/r/2150/

          -----------------------------------------------------------

          (Updated 2011-10-06 23:25:18)

          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/HAUtil.java PRE-CREATION

          branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/BackupNode.java 1179521

          branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java 1179521

          branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java 1179521

          branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/ActiveState.java 1179521

          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 1179521

          branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/StandbyState.java 1179521

          branches/HDFS-1623/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestHAUtil.java PRE-CREATION

          Diff: https://reviews.apache.org/r/2150/diff

          Testing

          -------

          Thanks,

          Todd

          Show
          jiraposter@reviews.apache.org jiraposter@reviews.apache.org added a comment - On 2011-10-06 23:37:24, Todd Lipcon wrote: > branches/ HDFS-1623 /hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/HAUtil.java, line 28 > < https://reviews.apache.org/r/2150/diff/2/?file=48551#file48551line28 > > > perhaps should be abstract since it won't ever be instantiated? > > these functions are meant only for the server side, right? Otherwise they should all take an authority, and look at configs prefixed/suffixed with that authority? > > let me jump over to HDFS-2231 and try to review that first.. having a hard time following this. Class has private constructor. I prefer that to abstract class. For some reason, I included HAUtil in this patch causing lot of confusion. It belongs to 2231. So I am attaching a patch without HAUtil changes. Also I will address rest of your HAUtil comments in 2231. Suresh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2150/#review2422 ----------------------------------------------------------- On 2011-10-06 23:25:18, Todd Lipcon wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2150/ ----------------------------------------------------------- (Updated 2011-10-06 23:25:18) 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/HAUtil.java PRE-CREATION branches/ HDFS-1623 /hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/BackupNode.java 1179521 branches/ HDFS-1623 /hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java 1179521 branches/ HDFS-1623 /hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java 1179521 branches/ HDFS-1623 /hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/ActiveState.java 1179521 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 1179521 branches/ HDFS-1623 /hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/StandbyState.java 1179521 branches/ HDFS-1623 /hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestHAUtil.java PRE-CREATION Diff: https://reviews.apache.org/r/2150/diff Testing ------- Thanks, Todd
          Hide
          tlipcon Todd Lipcon added a comment -

          Just two nits below. Feel free to just fix these on commit - no need for another review. +1.


          Some funky indentation here:
          + throw new IOException("Service failed to start", e);
          + }


          Better to pass "ie" as the second parameter:
          + LOG.info("Caught interrupted exception " + ie);

          Show
          tlipcon Todd Lipcon added a comment - Just two nits below. Feel free to just fix these on commit - no need for another review. +1. Some funky indentation here: + throw new IOException("Service failed to start", e); + } Better to pass "ie" as the second parameter: + LOG.info("Caught interrupted exception " + ie);
          Hide
          sureshms Suresh Srinivas added a comment -

          Patch addresses comments from Todd.

          Show
          sureshms Suresh Srinivas added a comment - Patch addresses comments from Todd.
          Hide
          sureshms Suresh Srinivas added a comment -

          I committed the patch.

          Show
          sureshms Suresh Srinivas added a comment - I committed the patch.

            People

            • Assignee:
              sureshms Suresh Srinivas
              Reporter:
              sureshms Suresh Srinivas
            • Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development