Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-3310

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

        Details

        • Type: Bug Bug
        • Status: Closed
        • Priority: Major 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. Text File
            HDFS-3310-b1.001.patch
            1 kB
            Colin P. McCabe
          2. Text File
            HDFS-3310-b1.002.patch
            5 kB
            Colin P. McCabe
          3. Text File
            HDFS-3310-b1.003.patch
            6 kB
            Colin P. McCabe

            Issue Links

            relates to

            Bug - A problem which impairs or prevents the functions of the product. HDFS-2702 A single failed name dir can cause the NN to exit

            • Critical - Crashes, loss of data, severe memory leak.
            • Closed

              Activity

              Ascending order - Click to sort in descending order
              Hide
              Permalink
              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
              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
              Permalink
              Colin P. McCabe added a comment -
              • address Nicholas' comment (use exitIfNoStreams more consistently)
              • add new unit test in TestStorageDirectoryFailure
              Show
              Colin P. McCabe added a comment - address Nicholas' comment (use exitIfNoStreams more consistently) add new unit test in TestStorageDirectoryFailure
              Hide
              Permalink
              Tsz Wo Nicholas Sze added a comment -

              +1 the new patch looks good.

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

              Show
              Tsz Wo Nicholas Sze added a comment - +1 the new patch looks good. Could you run "test-patch" and all unit tests?
              Hide
              Permalink
              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
              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
              Permalink
              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
              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
              Permalink
              Colin P. McCabe added a comment -
              • use the one-argument form of FsImage#updateRemovedDirs where appropriate
              Show
              Colin P. McCabe added a comment - use the one-argument form of FsImage#updateRemovedDirs where appropriate
              Hide
              Permalink
              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
              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
              Permalink
              Eli Collins added a comment -

              +1 Nice work Colin!

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

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

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

              Closed upon release of Hadoop-1.0.3.

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

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved:

                    Development