Details

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

      Description

      Currently, the JN does not validate the configured journal directory until it tries to write into it. This is counter-intuitive for users, since they would expect to find out about a misconfiguration at startup time, rather than on first access. Additionally, two testers accidentally configured the journal dir to be a URI, which the code accidentally understood as a relative path (CWD/file:/foo/bar.

      We should validate the config at startup to be an accessible absolute path.

      1. hdfs-3795.txt
        6 kB
        Todd Lipcon
      2. hdfs-3795.txt
        6 kB
        Todd Lipcon

        Activity

        Hide
        Aaron T. Myers added a comment -

        +1, the updated patch looks good to me.

        Show
        Aaron T. Myers added a comment - +1, the updated patch looks good to me.
        Hide
        Todd Lipcon added a comment -

        Updated patch addresses ATM's feedback.

        Show
        Todd Lipcon added a comment - Updated patch addresses ATM's feedback.
        Hide
        Aaron T. Myers added a comment -
        1. Instead of "!dir.getPath().startsWith("/")" how about "!dis.isAbsolute()" ?
        2. If the path is not a directory, this will fail with a misleading error message:
          +    if (!dir.isDirectory() && !dir.mkdirs()) {
          +      throw new IOException("Could not create journal dir '" +
          +          dir + "'");
          +    }
          

        Patch looks good otherwise.

        Show
        Aaron T. Myers added a comment - Instead of "!dir.getPath().startsWith("/")" how about "!dis.isAbsolute()" ? If the path is not a directory, this will fail with a misleading error message: + if (!dir.isDirectory() && !dir.mkdirs()) { + throw new IOException( "Could not create journal dir '" + + dir + "'" ); + } Patch looks good otherwise.
        Hide
        Todd Lipcon added a comment -

        Simple patch attached.

        Show
        Todd Lipcon added a comment - Simple patch attached.

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development