Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-1055

Improve thread naming for DataXceivers

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.22.0
    • Fix Version/s: 0.22.0
    • Component/s: datanode
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      The DataXceiver threads are named using the default Daemon naming, which is Runnable.toString(). Currently this isn't implemented, so threads have names like org.apache.hadoop.hdfs.server.datanode.DataXceiver@579c9a6b. It would be very handy for debugging (and even ops maybe) to have a better name like "DataXceiver for client 1.2.3.4 [reading block_234254242]"

      1. dataxceiver.patch
        3 kB
        Ramkumar Vadali
      2. dataxceiver-merged.patch
        4 kB
        Ramkumar Vadali
      3. hdfs-1055-1.patch
        4 kB
        Eli Collins
      4. hdfs-1055-branch20.txt
        6 kB
        Todd Lipcon

        Activity

        Hide
        Todd Lipcon added a comment -

        Thanks for the patch, Ramkumar. Looks like we did almost the same thing. Here's one I did against 20 (was waiting til I had a trunk version before posting)

        I'll take a look at yours asap.

        Show
        Todd Lipcon added a comment - Thanks for the patch, Ramkumar. Looks like we did almost the same thing. Here's one I did against 20 (was waiting til I had a trunk version before posting) I'll take a look at yours asap.
        Hide
        Ramkumar Vadali added a comment -

        Hi Todd, your patch looks better. Do you plan to merge it soon?

        Show
        Ramkumar Vadali added a comment - Hi Todd, your patch looks better. Do you plan to merge it soon?
        Hide
        Todd Lipcon added a comment -

        It's not top of my list - if you wanted to merge our two patches and post a patch against trunk, that would be great. Otherwise I will likely get to this some time in May.

        Show
        Todd Lipcon added a comment - It's not top of my list - if you wanted to merge our two patches and post a patch against trunk, that would be great. Otherwise I will likely get to this some time in May.
        Hide
        Ramkumar Vadali added a comment -

        Merged Todd Lipcon's changes.

        In trunk DataXceiver extends DataTransferProtocol.Receiver, so making it extend Thread would need some rewrite. Instead of making DataXceiver extend Thread, this patch uses Thread.currentThread().setName(). The friendly name can be set only after the thread starts, and that should be sufficient for this resolving this issue.

        Show
        Ramkumar Vadali added a comment - Merged Todd Lipcon's changes. In trunk DataXceiver extends DataTransferProtocol.Receiver, so making it extend Thread would need some rewrite. Instead of making DataXceiver extend Thread, this patch uses Thread.currentThread().setName(). The friendly name can be set only after the thread starts, and that should be sufficient for this resolving this issue.
        Hide
        Eli Collins added a comment -

        +1

        Patch attached merges with trunk and makes the naming consistent (names start with caps).

        Show
        Eli Collins added a comment - +1 Patch attached merges with trunk and makes the naming consistent (names start with caps).
        Hide
        Eli Collins added a comment -
             [exec] 
             [exec] -1 overall.  
             [exec] 
             [exec]     +1 @author.  The patch does not contain any @author tags.
             [exec] 
             [exec]     -1 tests included.  The patch doesn't appear to include any new or modified tests.
             [exec]                         Please justify why no new tests are needed for this patch.
             [exec]                         Also please list what manual steps were performed to verify this patch.
             [exec] 
             [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
             [exec] 
             [exec]     +1 javac.  The applied patch does not increase the total number of javac compiler warnings.
             [exec] 
             [exec]     +1 findbugs.  The patch does not introduce any new Findbugs warnings.
             [exec] 
             [exec]     -1 release audit.  The applied patch generated 103 release audit warnings (more than the trunk's current 1 warnings).
             [exec] 
             [exec]     +1 system test framework.  The patch passed system test framework compile.
             [exec] 
        

        Ran the tests, no additional tests are failing that aren't already failing on trunk. I verified via jstack that the thread names are as intended, eg:

        DataXceiver for client /127.0.0.1:41506 [Receiving block blk_2636278219925072830_1001 client=DFSClient_-283453089]" daemon prio=10 tid=0x00007f42b0205000 nid=0x399f runnable [0x00007f42bc147000]

        Show
        Eli Collins added a comment - [exec] [exec] -1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] -1 tests included. The patch doesn't appear to include any new or modified tests. [exec] Please justify why no new tests are needed for this patch. [exec] Also please list what manual steps were performed to verify this patch. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] -1 release audit. The applied patch generated 103 release audit warnings (more than the trunk's current 1 warnings). [exec] [exec] +1 system test framework. The patch passed system test framework compile. [exec] Ran the tests, no additional tests are failing that aren't already failing on trunk. I verified via jstack that the thread names are as intended, eg: DataXceiver for client /127.0.0.1:41506 [Receiving block blk_2636278219925072830_1001 client=DFSClient_-283453089] " daemon prio=10 tid=0x00007f42b0205000 nid=0x399f runnable [0x00007f42bc147000]
        Hide
        Eli Collins added a comment -

        I've committed this. Thanks Todd and Ramkumar!

        Show
        Eli Collins added a comment - I've committed this. Thanks Todd and Ramkumar!

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development