Flume
  1. Flume
  2. FLUME-2243

AvroSource to use TransceiverThreadFactory for Thread naming while initializing NioServerSocketChannelFactory

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: v1.4.0
    • Fix Version/s: v1.5.0
    • Component/s: Sinks+Sources
    • Labels:
      None

      Description

      Currently in Avro source, we use
      socketChannelFactory = new NioServerSocketChannelFactory
      (Executors .newCachedThreadPool(), Executors.newCachedThreadPool());

      This would create generic Thread names like pool-1-thread-1. It would be good to use a ThreadFactory, like we use in NettyAvroRpcClient for better identification of Threads in log file

      ExecutorService bossExecutor = Executors.newCachedThreadPool(new TransceiverThreadFactory(
      "Avro " + NettyTransceiver.class.getSimpleName() + " Boss"));
      ExecutorService workerExecutor = Executors.newCachedThreadPool(new TransceiverThreadFactory(
      "Avro " + NettyTransceiver.class.getSimpleName() + " I/O Worker"));

      Move the TransceiverThreadFactory class to util package (from private static class) and update the usage in code.

      1. FLUME-2243-1.patch
        2 kB
        Ashish Paliwal
      2. FLUME-2243-0.patch
        3 kB
        Ashish Paliwal

        Issue Links

          Activity

          Hide
          Gopinathan A added a comment -

          +1 for patch

          Show
          Gopinathan A added a comment - +1 for patch
          Hide
          Hari Shreedharan added a comment -

          This one got committed with FLUME-2238.

          Show
          Hari Shreedharan added a comment - This one got committed with FLUME-2238 .
          Hide
          Hari Shreedharan added a comment -

          Ashish Paliwal - Please confirm this is committed. If yes, please can you please close this one?

          Show
          Hari Shreedharan added a comment - Ashish Paliwal - Please confirm this is committed. If yes, please can you please close this one?
          Hide
          Ashish Paliwal added a comment -

          Hari Shreedharan This is not committed. FLUME-2238 was meant to control the number of Threads. This patch is only meant to use the Thread Naming Factory, which would aid in debugging in Thread dumps.

          Verified in both commit message and trunk source, the changes are not present. Let me know if it makes sense.

          Show
          Ashish Paliwal added a comment - Hari Shreedharan This is not committed. FLUME-2238 was meant to control the number of Threads. This patch is only meant to use the Thread Naming Factory, which would aid in debugging in Thread dumps. Verified in both commit message and trunk source, the changes are not present. Let me know if it makes sense.
          Hide
          Hari Shreedharan added a comment -

          Thanks Ashish. Looks good in general, but I have one suggestion. Looks like you copied over the TransceiverThreadFactory from the NettyAvroRpcClient. I think one thing you could do is to move that class from being a private static class to the org.apache.flume.util package (and perhaps rename it) and use it in AvroSource - this avoids duplicate code.

          Show
          Hari Shreedharan added a comment - Thanks Ashish. Looks good in general, but I have one suggestion. Looks like you copied over the TransceiverThreadFactory from the NettyAvroRpcClient. I think one thing you could do is to move that class from being a private static class to the org.apache.flume.util package (and perhaps rename it) and use it in AvroSource - this avoids duplicate code.
          Hide
          Ashish Paliwal added a comment -

          True Hari! I had the same issue, but since I was not sure where I could put it, I copied it. There were other instances where I faced same issue, mostly with methods for reuse (findFreePort() is a good example).

          I shall update the patch based on your suggestion. Will move the code out and update the usage in both classes.

          Show
          Ashish Paliwal added a comment - True Hari! I had the same issue, but since I was not sure where I could put it, I copied it. There were other instances where I faced same issue, mostly with methods for reuse (findFreePort() is a good example). I shall update the patch based on your suggestion. Will move the code out and update the usage in both classes.
          Hide
          Ashish Paliwal added a comment -

          Hari Shreedharan I am thinking of an alternative solution. Guava has ThreadFactoryBuilder, playing with it to see if I can use it instead of writing the class. Shall upload the updated patch in next couple of hours.

          Show
          Ashish Paliwal added a comment - Hari Shreedharan I am thinking of an alternative solution. Guava has ThreadFactoryBuilder, playing with it to see if I can use it instead of writing the class. Shall upload the updated patch in next couple of hours.
          Hide
          Hari Shreedharan added a comment -

          We use that in many other places in Flume.

          Show
          Hari Shreedharan added a comment - We use that in many other places in Flume.
          Hide
          Ashish Paliwal added a comment -

          Yup, I saw the usage in ExecSource while fixing FLUME-2024. I have not used it so far, so just wanted to play with it before adding it.

          Show
          Ashish Paliwal added a comment - Yup, I saw the usage in ExecSource while fixing FLUME-2024 . I have not used it so far, so just wanted to play with it before adding it.
          Hide
          Ashish Paliwal added a comment -

          Rebased the patch.
          Removed TransreceiverThreadFactory class and used Guava ThreadNamingFactory.
          Removed Unused imports

          Show
          Ashish Paliwal added a comment - Rebased the patch. Removed TransreceiverThreadFactory class and used Guava ThreadNamingFactory. Removed Unused imports
          Hide
          Hari Shreedharan added a comment -

          +1. Running tests and committing.

          Show
          Hari Shreedharan added a comment - +1. Running tests and committing.
          Hide
          ASF subversion and git services added a comment -

          Commit 47507bc40202c52e7156106642ffc0ab849fb96e in flume's branch refs/heads/trunk from Hari Shreedharan
          [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=47507bc ]

          FLUME-2243. Avro Source should name threads instead of using generic names.

          (Ashish Paliwal via Hari Shreedharan)

          Show
          ASF subversion and git services added a comment - Commit 47507bc40202c52e7156106642ffc0ab849fb96e in flume's branch refs/heads/trunk from Hari Shreedharan [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=47507bc ] FLUME-2243 . Avro Source should name threads instead of using generic names. (Ashish Paliwal via Hari Shreedharan)
          Hide
          ASF subversion and git services added a comment -

          Commit aacd016a9dadd70cc5d3ce49c21c95f7c3483268 in flume's branch refs/heads/flume-1.5 from Hari Shreedharan
          [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=aacd016 ]

          FLUME-2243. Avro Source should name threads instead of using generic names.

          (Ashish Paliwal via Hari Shreedharan)

          Show
          ASF subversion and git services added a comment - Commit aacd016a9dadd70cc5d3ce49c21c95f7c3483268 in flume's branch refs/heads/flume-1.5 from Hari Shreedharan [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=aacd016 ] FLUME-2243 . Avro Source should name threads instead of using generic names. (Ashish Paliwal via Hari Shreedharan)
          Hide
          Hari Shreedharan added a comment -

          Committed! Thanks Ashish!

          Show
          Hari Shreedharan added a comment - Committed! Thanks Ashish!
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in flume-trunk #561 (See https://builds.apache.org/job/flume-trunk/561/)
          FLUME-2243. Avro Source should name threads instead of using generic names. (hshreedharan: http://git-wip-us.apache.org/repos/asf/flume/repo/?p=flume.git&a=commit&h=47507bc40202c52e7156106642ffc0ab849fb96e)

          • flume-ng-core/src/main/java/org/apache/flume/source/AvroSource.java
          Show
          Hudson added a comment - SUCCESS: Integrated in flume-trunk #561 (See https://builds.apache.org/job/flume-trunk/561/ ) FLUME-2243 . Avro Source should name threads instead of using generic names. (hshreedharan: http://git-wip-us.apache.org/repos/asf/flume/repo/?p=flume.git&a=commit&h=47507bc40202c52e7156106642ffc0ab849fb96e ) flume-ng-core/src/main/java/org/apache/flume/source/AvroSource.java

            People

            • Assignee:
              Ashish Paliwal
              Reporter:
              Ashish Paliwal
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development