Uploaded image for project: 'Hadoop HDFS'
  1. Hadoop HDFS
  2. HDFS-3310

Make sure that we abort when no edit log directories are left

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.20.1
    • Fix Version/s: 1.0.3
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      We should make sure to abort when there are no edit log directories left to write to. It seems that there is at least one case that is slipping through the cracks right now in branch-1.

      1. HDFS-3310-b1.003.patch
        6 kB
        Colin P. McCabe
      2. HDFS-3310-b1.002.patch
        5 kB
        Colin P. McCabe
      3. HDFS-3310-b1.001.patch
        1 kB
        Colin P. McCabe

        Issue Links

          Activity

          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          I think should be replaced by exitIfNoStreams().

          +        if (editStreams.isEmpty()) {
          +          throw new IOException("no editlog streams");
          +        }
          

          Patch looks good other then that.

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - I think should be replaced by exitIfNoStreams(). + if (editStreams.isEmpty()) { + throw new IOException( "no editlog streams" ); + } Patch looks good other then that.
          Hide
          cmccabe Colin P. McCabe added a comment -
          • address Nicholas' comment (use exitIfNoStreams more consistently)
          • add new unit test in TestStorageDirectoryFailure
          Show
          cmccabe Colin P. McCabe added a comment - address Nicholas' comment (use exitIfNoStreams more consistently) add new unit test in TestStorageDirectoryFailure
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          +1 the new patch looks good.

          Could you run "test-patch" and all unit tests?

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - +1 the new patch looks good. Could you run "test-patch" and all unit tests?
          Hide
          eli2 Eli Collins added a comment -

          Mind updating the calls to updateRemovedDirs(sd, null) to use your new method w/o 2nd param? No need to re-run test-patch for this.

          Otherwise +1, looks great. Nice find btw!

          Show
          eli2 Eli Collins added a comment - Mind updating the calls to updateRemovedDirs(sd, null) to use your new method w/o 2nd param? No need to re-run test-patch for this. Otherwise +1, looks great. Nice find btw!
          Hide
          cmccabe Colin P. McCabe added a comment -

          I re-ran all unit tests with this change and saw no more than the usual failures (TestCLI, TestFileAppend4, etc.).

          I tried to run the test-patch script, but there seem to be a lot of environment setup issues at the moment.

          Show
          cmccabe Colin P. McCabe added a comment - I re-ran all unit tests with this change and saw no more than the usual failures (TestCLI, TestFileAppend4, etc.). I tried to run the test-patch script, but there seem to be a lot of environment setup issues at the moment.
          Hide
          cmccabe Colin P. McCabe added a comment -
          • use the one-argument form of FsImage#updateRemovedDirs where appropriate
          Show
          cmccabe Colin P. McCabe added a comment - use the one-argument form of FsImage#updateRemovedDirs where appropriate
          Hide
          cmccabe Colin P. McCabe added a comment -

          I ran testpatch (sorry for the delay) and got the expected 8 warnings. (see HADOOP-7847). So it looks good.

          C.

          Show
          cmccabe Colin P. McCabe added a comment - I ran testpatch (sorry for the delay) and got the expected 8 warnings. (see HADOOP-7847 ). So it looks good. C.
          Hide
          eli2 Eli Collins added a comment -

          +1 Nice work Colin!

          Show
          eli2 Eli Collins added a comment - +1 Nice work Colin!
          Hide
          eli2 Eli Collins added a comment -

          I've committed this to branch-1 and merged for 1.0.3.

          Show
          eli2 Eli Collins added a comment - I've committed this to branch-1 and merged for 1.0.3.
          Hide
          mattf Matt Foley added a comment -

          Closed upon release of Hadoop-1.0.3.

          Show
          mattf Matt Foley added a comment - Closed upon release of Hadoop-1.0.3.

            People

            • Assignee:
              cmccabe Colin P. McCabe
              Reporter:
              cmccabe Colin P. McCabe
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development