Details

    • Type: Sub-task Sub-task
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      If dfs.domain.socket.path is set, but we don't have anything enabled which would need it (like dfs.client.read.shortcircuit), don't create the socket.

      1. HDFS-4473.001.patch
        5 kB
        Colin Patrick McCabe

        Issue Links

          Activity

          Colin Patrick McCabe created issue -
          Colin Patrick McCabe made changes -
          Field Original Value New Value
          Component/s datanode [ 12312927 ]
          Component/s hdfs-client [ 12312928 ]
          Component/s performance [ 12316501 ]
          Hide
          Colin Patrick McCabe added a comment -

          This also fixes an issue in TestShortCircuitLocalRead#testSkipWithVerifyChecksum and TestShortCircuitLocalRead#testHandleTruncatedBlockFile where we weren't setting the socket path (and hence not really testing short-circuit local reads.)

          Show
          Colin Patrick McCabe added a comment - This also fixes an issue in TestShortCircuitLocalRead#testSkipWithVerifyChecksum and TestShortCircuitLocalRead#testHandleTruncatedBlockFile where we weren't setting the socket path (and hence not really testing short-circuit local reads.)
          Colin Patrick McCabe made changes -
          Attachment HDFS-4473.001.patch [ 12568177 ]
          Hide
          Aaron T. Myers added a comment -

          The patch looks pretty good to me. One question: is there some reason that the domain socket paths can't be under the normal test build dir, and so have to be under /tmp? If not, I recommend moving those files to be under the test build dir.

          +1 once this is addressed, either by answering the question or changing the code.

          Show
          Aaron T. Myers added a comment - The patch looks pretty good to me. One question: is there some reason that the domain socket paths can't be under the normal test build dir, and so have to be under /tmp? If not, I recommend moving those files to be under the test build dir. +1 once this is addressed, either by answering the question or changing the code.
          Hide
          Colin Patrick McCabe added a comment -

          One question: is there some reason that the domain socket paths can't be under the normal test build dir, and so have to be under /tmp? If not, I recommend moving those files to be under the test build dir.

          That's a good question. Actually, there is a reason: UNIX domain socket paths can't be longer than like 108 bytes in total, which would cause failures for a lot of people who have long test build directory paths-- including Jenkins, if I remember correctly.

          Show
          Colin Patrick McCabe added a comment - One question: is there some reason that the domain socket paths can't be under the normal test build dir, and so have to be under /tmp? If not, I recommend moving those files to be under the test build dir. That's a good question. Actually, there is a reason: UNIX domain socket paths can't be longer than like 108 bytes in total, which would cause failures for a lot of people who have long test build directory paths-- including Jenkins, if I remember correctly.
          Hide
          Aaron T. Myers added a comment -

          Wow, good to know. What happens if the path is too long? Hopefully there's some decent error. If not, we might consider checking for this during DN startup.

          Regardless, that can be done as a separate JIRA. I'm going to commit this momentarily.

          Show
          Aaron T. Myers added a comment - Wow, good to know. What happens if the path is too long? Hopefully there's some decent error. If not, we might consider checking for this during DN startup. Regardless, that can be done as a separate JIRA. I'm going to commit this momentarily.
          Hide
          Aaron T. Myers added a comment - - edited

          I've just committed this to the HDFS-347 branch.

          Thanks a lot for the contribution, Colin.

          Show
          Aaron T. Myers added a comment - - edited I've just committed this to the HDFS-347 branch. Thanks a lot for the contribution, Colin.
          Aaron T. Myers made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Hadoop Flags Reviewed [ 10343 ]
          Resolution Fixed [ 1 ]
          Hide
          Colin Patrick McCabe added a comment -

          Wow, good to know. What happens if the path is too long? Hopefully there's some decent error. If not, we might consider checking for this during DN startup.

          Yeah. It will catch this error during DN startup and give an IOException like "error computing UNIX domain socket path: path too long. The longest UNIX domain socket path possible on this host is 108 bytes." (see DomainSocket.c)

          Show
          Colin Patrick McCabe added a comment - Wow, good to know. What happens if the path is too long? Hopefully there's some decent error. If not, we might consider checking for this during DN startup. Yeah. It will catch this error during DN startup and give an IOException like "error computing UNIX domain socket path: path too long. The longest UNIX domain socket path possible on this host is 108 bytes." (see DomainSocket.c)
          Hide
          Aaron T. Myers added a comment -

          Excellent. Thanks, Colin.

          Show
          Aaron T. Myers added a comment - Excellent. Thanks, Colin.
          Colin Patrick McCabe made changes -
          Link This issue relates to HDFS-4496 [ HDFS-4496 ]
          Colin Patrick McCabe made changes -
          Summary don't create domain socket unless we need it DataNode: don't create domain socket unless we need it

            People

            • Assignee:
              Colin Patrick McCabe
              Reporter:
              Colin Patrick McCabe
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development