Details

    • Sub-task
    • Status: Closed
    • Major
    • Resolution: Fixed
    • None
    • 0.99.0, hbase-10070
    • None
    • None
    • Reviewed

    Description

      While running the IT test from HBASE-10572, we've noticed that the number of threads jumps to 4K's when CM actions are going on.

      Our ndimiduk summarizes the problem quite good:

      MultiThreadedReader creates this pool for each HTable:

          ThreadPoolExecutor pool = new ThreadPoolExecutor(1, maxThreads, keepAliveTime, TimeUnit.SECONDS,
              new SynchronousQueue<Runnable>(), Threads.newDaemonThreadFactory("htable"));
      

      This comes from the HTable creation

        
      public HTable(Configuration conf, final TableName tableName)
      

      As well the javadoc says Recommended.
      This is wrong.

      In this issue we can change the LTT sub classes to use the shared connection object and initialize their tables using HConnection.getTable() rather than new HTable().

      This is relevant to trunk as well, but there since there is only one outstanding RPC per thread, it is not such a big problem.

      Attachments

        1. hbase-10810_v1.patch
          27 kB
          Enis Soztutar
        2. 0038-HBASE-10810-LoadTestTool-should-share-the-connection.patch
          28 kB
          Enis Soztutar

        Issue Links

          Activity

            ndimiduk Nick Dimiduk added a comment -

            nkeywal and I came to the same conclusion simultaneously. I think the test should be using a single pool for all tables it creates, instead of using this HTable constructor with it's managed pool. In general, I think the HTable constructor is dangerous for anything but a one-off and should be removed (see HBASE-9117).

            ndimiduk Nick Dimiduk added a comment - nkeywal and I came to the same conclusion simultaneously. I think the test should be using a single pool for all tables it creates, instead of using this HTable constructor with it's managed pool. In general, I think the HTable constructor is dangerous for anything but a one-off and should be removed (see HBASE-9117 ).
            enis Enis Soztutar added a comment -

            Here is a simple enough patch. It changes new HTable()'s with HConnection.getTable() calls.

            I've tested this with LTT and IntegrationTestTimeBoundedRequestsWithRegionReplicas with 100 writer / reader threads.

            The number of threads during execution with CM does not exceed 410 (100 threads for readers + 256 for pool + some other).

            enis Enis Soztutar added a comment - Here is a simple enough patch. It changes new HTable()'s with HConnection.getTable() calls. I've tested this with LTT and IntegrationTestTimeBoundedRequestsWithRegionReplicas with 100 writer / reader threads. The number of threads during execution with CM does not exceed 410 (100 threads for readers + 256 for pool + some other).
            ndimiduk Nick Dimiduk added a comment -

            +1 this should resolve the problem of creating so many pools with so many threads in total.

            What's unclear to me is how the 100 reader threads will contend for the (up to) 256 threads in the shared pool used by all HTables for the connection. I need a more thorough understanding of AsyncProcess...

            ndimiduk Nick Dimiduk added a comment - +1 this should resolve the problem of creating so many pools with so many threads in total. What's unclear to me is how the 100 reader threads will contend for the (up to) 256 threads in the shared pool used by all HTables for the connection. I need a more thorough understanding of AsyncProcess...
            enis Enis Soztutar added a comment -

            Thanks Nick for the review.

            What's unclear to me is how the 100 reader threads will contend for the (up to) 256 threads in the shared pool used by all HTables for the connection

            In case this becomes a problem, we can increase the default pool side that the LTT connection is using.

            enis Enis Soztutar added a comment - Thanks Nick for the review. What's unclear to me is how the 100 reader threads will contend for the (up to) 256 threads in the shared pool used by all HTables for the connection In case this becomes a problem, we can increase the default pool side that the LTT connection is using.
            ddas Devaraj Das added a comment -

            +1

            ddas Devaraj Das added a comment - +1
            enis Enis Soztutar added a comment -

            Committed this to branch. Thanks Nick and Devaraj for review.

            enis Enis Soztutar added a comment - Committed this to branch. Thanks Nick and Devaraj for review.
            enis Enis Soztutar added a comment -

            Attaching rebased patch for master that is committed

            enis Enis Soztutar added a comment - Attaching rebased patch for master that is committed
            enis Enis Soztutar added a comment -

            Committed to master as part of hbase-10070 branch merge

            enis Enis Soztutar added a comment - Committed to master as part of hbase-10070 branch merge
            hudson Hudson added a comment -

            FAILURE: Integrated in HBase-TRUNK #5245 (See https://builds.apache.org/job/HBase-TRUNK/5245/)
            HBASE-10810 LoadTestTool should share the connection and connection pool (enis: rev 25baace0dec1fc4f3b5fb51292c8ec8a6da85ba0)

            • hbase-it/src/test/java/org/apache/hadoop/hbase/test/IntegrationTestTimeBoundedRequestsWithRegionReplicas.java
            • hbase-server/src/test/java/org/apache/hadoop/hbase/util/MultiThreadedReaderWithACL.java
            • hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestMiniClusterLoadSequential.java
            • hbase-server/src/test/java/org/apache/hadoop/hbase/util/MultiThreadedWriterWithACL.java
            • hbase-server/src/test/java/org/apache/hadoop/hbase/util/MultiThreadedWriter.java
            • hbase-server/src/test/java/org/apache/hadoop/hbase/util/MultiThreadedWriterBase.java
            • hbase-server/src/test/java/org/apache/hadoop/hbase/util/MultiThreadedReader.java
            • hbase-server/src/test/java/org/apache/hadoop/hbase/util/MultiThreadedAction.java
            • hbase-server/src/test/java/org/apache/hadoop/hbase/util/MultiThreadedUpdaterWithACL.java
            • hbase-server/src/test/java/org/apache/hadoop/hbase/util/MultiThreadedUpdater.java
            hudson Hudson added a comment - FAILURE: Integrated in HBase-TRUNK #5245 (See https://builds.apache.org/job/HBase-TRUNK/5245/ ) HBASE-10810 LoadTestTool should share the connection and connection pool (enis: rev 25baace0dec1fc4f3b5fb51292c8ec8a6da85ba0) hbase-it/src/test/java/org/apache/hadoop/hbase/test/IntegrationTestTimeBoundedRequestsWithRegionReplicas.java hbase-server/src/test/java/org/apache/hadoop/hbase/util/MultiThreadedReaderWithACL.java hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestMiniClusterLoadSequential.java hbase-server/src/test/java/org/apache/hadoop/hbase/util/MultiThreadedWriterWithACL.java hbase-server/src/test/java/org/apache/hadoop/hbase/util/MultiThreadedWriter.java hbase-server/src/test/java/org/apache/hadoop/hbase/util/MultiThreadedWriterBase.java hbase-server/src/test/java/org/apache/hadoop/hbase/util/MultiThreadedReader.java hbase-server/src/test/java/org/apache/hadoop/hbase/util/MultiThreadedAction.java hbase-server/src/test/java/org/apache/hadoop/hbase/util/MultiThreadedUpdaterWithACL.java hbase-server/src/test/java/org/apache/hadoop/hbase/util/MultiThreadedUpdater.java
            enis Enis Soztutar added a comment -

            Closing this issue after 0.99.0 release.

            enis Enis Soztutar added a comment - Closing this issue after 0.99.0 release.

            People

              enis Enis Soztutar
              enis Enis Soztutar
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: