Details

    • Type: Sub-task Sub-task
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: QuorumJournalManager (HDFS-3077)
    • Component/s: ha
    • Labels:
      None

      Description

      HDFS-3695 added the ability for non-File journal managers to tie into calls like NameNode -format. This JIRA is to implement format() for QuorumJournalManager.

      1. hdfs-3793.txt
        32 kB
        Todd Lipcon

        Activity

        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Resolved Resolved
        1d 3h 16m 1 Todd Lipcon 15/Aug/12 01:53
        Todd Lipcon made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Hadoop Flags Reviewed [ 10343 ]
        Fix Version/s QuorumJournalManager (HDFS-3077) [ 12322478 ]
        Resolution Fixed [ 1 ]
        Hide
        Todd Lipcon added a comment -

        Fixed the two nits and committed to branch. Thanks.

        Seems like you should make the QuorumJournalManager#format and QuroumJournalManager#hasSomeData timeouts configurable, or at least use constants and add a comment or two justifying how you chose those values.

        I added constants and set them both to 60sec. Also added a comment explaining that, since they are only used in format and not normal operation, we can use a fairly long timeout and don't really need to configure them (if a user sees a timeout they can manually investigate why it's taking 60+sec and do something about it)

        I think I see the reasoning behind the need for the call to unlockAll in JNStorage#format, but you might want to add a comment explaining why it's necessary. Also, if this happens, when will the storage be locked again? Might want to add a comment explaining that as well.

        Added a comment:

            // Unlock the directory before formatting, because we will
            // re-analyze it after format(). The analyzeStorage() call
            // below is reponsible for re-locking it. This is a no-op
            // if the storage is not currently locked.
            unlockAll();
        
        Show
        Todd Lipcon added a comment - Fixed the two nits and committed to branch. Thanks. Seems like you should make the QuorumJournalManager#format and QuroumJournalManager#hasSomeData timeouts configurable, or at least use constants and add a comment or two justifying how you chose those values. I added constants and set them both to 60sec. Also added a comment explaining that, since they are only used in format and not normal operation, we can use a fairly long timeout and don't really need to configure them (if a user sees a timeout they can manually investigate why it's taking 60+sec and do something about it) I think I see the reasoning behind the need for the call to unlockAll in JNStorage#format, but you might want to add a comment explaining why it's necessary. Also, if this happens, when will the storage be locked again? Might want to add a comment explaining that as well. Added a comment: // Unlock the directory before formatting, because we will // re-analyze it after format(). The analyzeStorage() call // below is reponsible for re-locking it. This is a no-op // if the storage is not currently locked. unlockAll();
        Hide
        Aaron T. Myers added a comment -

        Patch looks pretty good to me. Two small comments:

        1. Seems like you should make the QuorumJournalManager#format and QuroumJournalManager#hasSomeData timeouts configurable, or at least use constants and add a comment or two justifying how you chose those values.
        2. I think I see the reasoning behind the need for the call to unlockAll in JNStorage#format, but you might want to add a comment explaining why it's necessary. Also, if this happens, when will the storage be locked again? Might want to add a comment explaining that as well.

        +1 once these are addressed.

        Show
        Aaron T. Myers added a comment - Patch looks pretty good to me. Two small comments: Seems like you should make the QuorumJournalManager#format and QuroumJournalManager#hasSomeData timeouts configurable, or at least use constants and add a comment or two justifying how you chose those values. I think I see the reasoning behind the need for the call to unlockAll in JNStorage#format, but you might want to add a comment explaining why it's necessary. Also, if this happens, when will the storage be locked again? Might want to add a comment explaining that as well. +1 once these are addressed.
        Hide
        Andrew Purtell added a comment -

        +1 Applied this patch after the generic support and confirmed with manual testing.

        Show
        Andrew Purtell added a comment - +1 Applied this patch after the generic support and confirmed with manual testing.
        Todd Lipcon made changes -
        Field Original Value New Value
        Attachment hdfs-3793.txt [ 12540765 ]
        Hide
        Todd Lipcon added a comment -

        Attached patch implements the formatting behavior.

        In addition to changing the tests to use this new API to format at startup, I also tested this manually on a cluster using both "namenode -format" and "namenode -initializeSharedEdits". Both the confirmation behavior and the formatting behavior reacted correctly.

        Show
        Todd Lipcon added a comment - Attached patch implements the formatting behavior. In addition to changing the tests to use this new API to format at startup, I also tested this manually on a cluster using both "namenode -format" and "namenode -initializeSharedEdits". Both the confirmation behavior and the formatting behavior reacted correctly.
        Todd Lipcon created issue -

          People

          • Assignee:
            Todd Lipcon
            Reporter:
            Todd Lipcon
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development