HBase
  1. HBase
  2. HBASE-3861

MiniZooKeeperCluster.startup() should refer to hbase.zookeeper.property.maxClientCnxns

    Details

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

      Description

      Currently the number of the client connections is hard-wired to 1000:

              standaloneServerFactory = new NIOServerCnxnFactory();
              standaloneServerFactory.configure(new InetSocketAddress(clientPort),1000);
            } catch (BindException e) {
       
      

      This should be set according to the test environment's hbase configuration. The property in
      question is : hbase.zookeeper.property.maxClientCnxns.

      Currently some tests such as org.apache.hadoop.hbase.client.TestHCM fail because the number of connections used by the HBase client exceeds 1000.

      Recently MAX_CACHED_HBASE_INSTANCES increased from 31 to 2000 on 0.90 branch:

      http://svn.apache.org/viewvc/hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java?p2=%2Fhbase%2Fbranches%2F0.90%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhadoop%2Fhbase%2Fclient%2FHConnectionManager.java&p1=%2Fhbase%2Fbranches%2F0.90%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhadoop%2Fhbase%2Fclient%2FHConnectionManager.java&r1=1096818&r2=1096817&view=diff&pathrev=1096818

      and correspondingly the hbase config on the Zookeeper server-side also increased in hbase-default.xml:

      http://svn.apache.org/viewvc/hbase/branches/0.90/src/main/resources/hbase-default.xml?p2=%2Fhbase%2Fbranches%2F0.90%2Fsrc%2Fmain%2Fresources%2Fhbase-default.xml&p1=%2Fhbase%2Fbranches%2F0.90%2Fsrc%2Fmain%2Fresources%2Fhbase-default.xml&r1=1091594&r2=1091593&view=diff&pathrev=1091594

      So if MiniZKCluster looks at this setting, the test won't have this failure.

      1. HBASE-3861.patch
        3 kB
        Eugene Koontz
      2. HBASE-3861.patch
        3 kB
        Eugene Koontz

        Issue Links

          Activity

          Lars Hofhansl made changes -
          Status Reopened [ 4 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Hide
          Lars Hofhansl added a comment -

          Looking at MiniZooKeeperCluster in trunk, this is already done:

                NIOServerCnxnFactory standaloneServerFactory;
                while (true) {
                  try {
                    standaloneServerFactory = new NIOServerCnxnFactory();
                    standaloneServerFactory.configure(
                      new InetSocketAddress(tentativePort),
                      configuration.getInt(HConstants.ZOOKEEPER_MAX_CLIENT_CNXNS,
                        1000));
                  } catch (BindException e) {
          

          Closing.

          Show
          Lars Hofhansl added a comment - Looking at MiniZooKeeperCluster in trunk, this is already done: NIOServerCnxnFactory standaloneServerFactory; while ( true ) { try { standaloneServerFactory = new NIOServerCnxnFactory(); standaloneServerFactory.configure( new InetSocketAddress(tentativePort), configuration.getInt(HConstants.ZOOKEEPER_MAX_CLIENT_CNXNS, 1000)); } catch (BindException e) { Closing.
          Eugene Koontz made changes -
          Description Currently the number of the client connections is hard-wired to 1000:

          {{{
                  standaloneServerFactory = new NIOServerCnxnFactory();
                  standaloneServerFactory.configure(new InetSocketAddress(clientPort),1000);
                } catch (BindException e) {
           
          }}}

          This should be set according to the test environment's hbase configuration. The property in
          question is : hbase.zookeeper.property.maxClientCnxns.

          Currently some tests such as org.apache.hadoop.hbase.client.TestHCM fail because the number of connections used by the HBase client exceeds 1000.

          Recently MAX_CACHED_HBASE_INSTANCES increased from 31 to 2000 on 0.90 branch:

          http://svn.apache.org/viewvc/hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java?p2=%2Fhbase%2Fbranches%2F0.90%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhadoop%2Fhbase%2Fclient%2FHConnectionManager.java&p1=%2Fhbase%2Fbranches%2F0.90%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhadoop%2Fhbase%2Fclient%2FHConnectionManager.java&r1=1096818&r2=1096817&view=diff&pathrev=1096818

          and correspondingly the hbase config on the Zookeeper server-side also increased in hbase-default.xml:

          http://svn.apache.org/viewvc/hbase/branches/0.90/src/main/resources/hbase-default.xml?p2=%2Fhbase%2Fbranches%2F0.90%2Fsrc%2Fmain%2Fresources%2Fhbase-default.xml&p1=%2Fhbase%2Fbranches%2F0.90%2Fsrc%2Fmain%2Fresources%2Fhbase-default.xml&r1=1091594&r2=1091593&view=diff&pathrev=1091594

          So if MiniZKCluster looks at this setting, the test won't have this failure.
          Currently the number of the client connections is hard-wired to 1000:

          {noformat}
                  standaloneServerFactory = new NIOServerCnxnFactory();
                  standaloneServerFactory.configure(new InetSocketAddress(clientPort),1000);
                } catch (BindException e) {
           
          {noformat}

          This should be set according to the test environment's hbase configuration. The property in
          question is : hbase.zookeeper.property.maxClientCnxns.

          Currently some tests such as org.apache.hadoop.hbase.client.TestHCM fail because the number of connections used by the HBase client exceeds 1000.

          Recently MAX_CACHED_HBASE_INSTANCES increased from 31 to 2000 on 0.90 branch:

          http://svn.apache.org/viewvc/hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java?p2=%2Fhbase%2Fbranches%2F0.90%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhadoop%2Fhbase%2Fclient%2FHConnectionManager.java&p1=%2Fhbase%2Fbranches%2F0.90%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhadoop%2Fhbase%2Fclient%2FHConnectionManager.java&r1=1096818&r2=1096817&view=diff&pathrev=1096818

          and correspondingly the hbase config on the Zookeeper server-side also increased in hbase-default.xml:

          http://svn.apache.org/viewvc/hbase/branches/0.90/src/main/resources/hbase-default.xml?p2=%2Fhbase%2Fbranches%2F0.90%2Fsrc%2Fmain%2Fresources%2Fhbase-default.xml&p1=%2Fhbase%2Fbranches%2F0.90%2Fsrc%2Fmain%2Fresources%2Fhbase-default.xml&r1=1091594&r2=1091593&view=diff&pathrev=1091594

          So if MiniZKCluster looks at this setting, the test won't have this failure.
          stack made changes -
          Fix Version/s 0.92.0 [ 12314223 ]
          Hide
          stack added a comment -

          Moving out of 0.92.0. Pull it back in if you think different.

          Show
          stack added a comment - Moving out of 0.92.0. Pull it back in if you think different.
          stack made changes -
          Fix Version/s 0.90.4 [ 12316406 ]
          Hide
          stack added a comment -

          Moving this to 0.92.0. It was committed for 0.90.3.

          Show
          stack added a comment - Moving this to 0.92.0. It was committed for 0.90.3.
          stack made changes -
          Fix Version/s 0.90.4 [ 12316406 ]
          Fix Version/s 0.90.3 [ 12316313 ]
          Hide
          Hudson added a comment -

          Integrated in HBase-TRUNK #1909 (See https://builds.apache.org/hudson/job/HBase-TRUNK/1909/)
          HBASE-3861 MiniZooKeeperCluster.startup() should refer to hbase.zookeeper.property.maxClientCnxns; backing it out till figure the TestHCM OOME issue

          Show
          Hudson added a comment - Integrated in HBase-TRUNK #1909 (See https://builds.apache.org/hudson/job/HBase-TRUNK/1909/ ) HBASE-3861 MiniZooKeeperCluster.startup() should refer to hbase.zookeeper.property.maxClientCnxns; backing it out till figure the TestHCM OOME issue
          Hide
          Eugene Koontz added a comment -

          HBASE-3777 may be related to this (has to do with relation between HConfiguration and HConnection).

          Show
          Eugene Koontz added a comment - HBASE-3777 may be related to this (has to do with relation between HConfiguration and HConnection).
          Eugene Koontz made changes -
          Link This issue is related to HBASE-3777 [ HBASE-3777 ]
          Andrew Purtell made changes -
          Resolution Fixed [ 1 ]
          Status Resolved [ 5 ] Reopened [ 4 ]
          Hide
          Andrew Purtell added a comment -

          Caused TestHCM to OOME on trunk up on Jenkins. Backed out for now. Reopening to investigate / fix.

          Show
          Andrew Purtell added a comment - Caused TestHCM to OOME on trunk up on Jenkins. Backed out for now. Reopening to investigate / fix.
          Hide
          stack added a comment -

          I just backed this out of TRUNK because its OOME'ing TestHCM up on hudson and locally on two different platforms (I can't see why it would do it though... must be some interaction w/ hbase-3777). It runs fine for me on branch.

          Show
          stack added a comment - I just backed this out of TRUNK because its OOME'ing TestHCM up on hudson and locally on two different platforms (I can't see why it would do it though... must be some interaction w/ hbase-3777). It runs fine for me on branch.
          Hide
          Eugene Koontz added a comment -

          HBASE-3861 patch causes TestHCM to fail on sometimes. TestHCM uses MAX_CACHED_HBASE_INSTANCES which is set to 2001 as part of fix for HBASE-3773.

          Show
          Eugene Koontz added a comment - HBASE-3861 patch causes TestHCM to fail on sometimes. TestHCM uses MAX_CACHED_HBASE_INSTANCES which is set to 2001 as part of fix for HBASE-3773 .
          Eugene Koontz made changes -
          Link This issue is related to HBASE-3773 [ HBASE-3773 ]
          Andrew Purtell made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Hadoop Flags [Reviewed]
          Release Note added new patch with numClientCnxns set to 5000 (enough for TestHCM to pass).
          Fix Version/s 0.92.0 [ 12314223 ]
          Resolution Fixed [ 1 ]
          Hide
          Andrew Purtell added a comment -

          Committed to trunk and 0.90 branch. Tests pass locally.

          Show
          Andrew Purtell added a comment - Committed to trunk and 0.90 branch. Tests pass locally.
          Hide
          Eugene Koontz added a comment -

          Thank you for looking at this, stack!! You are right about the constructor-calling. Thank you Andrew for fixing it.

          One thing which I can't explain is that TestHCM passes currently without this patch. It seems like it should fail because the maxNumCnxns is set too low at only 1000.

          So I think my patch is right but it doesn't fix a test that's not passing without it.

          Show
          Eugene Koontz added a comment - Thank you for looking at this, stack!! You are right about the constructor-calling. Thank you Andrew for fixing it. One thing which I can't explain is that TestHCM passes currently without this patch. It seems like it should fail because the maxNumCnxns is set too low at only 1000. So I think my patch is right but it doesn't fix a test that's not passing without it.
          Hide
          Hbase Build Acct added a comment -

          Sounds good

          Show
          Hbase Build Acct added a comment - Sounds good
          Hide
          Andrew Purtell added a comment -

          +1, I'll commit with Stack's comment addressed also.

          Show
          Andrew Purtell added a comment - +1, I'll commit with Stack's comment addressed also.
          Hide
          stack added a comment -

          Do you think the old constructor should call your new one Eugene? Else some data members will not be set up right? Otherwise the patch looks good to me.

          Show
          stack added a comment - Do you think the old constructor should call your new one Eugene? Else some data members will not be set up right? Otherwise the patch looks good to me.
          Eugene Koontz made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Release Note added new patch with numClientCnxns set to 5000 (enough for TestHCM to pass).
          Eugene Koontz made changes -
          Attachment HBASE-3861.patch [ 12478359 ]
          Eugene Koontz made changes -
          Status Patch Available [ 10002 ] Open [ 1 ]
          Hide
          Eugene Koontz added a comment -

          creating new patch with maxNumCnxns set sufficiently high for TestHCM to pass.

          Show
          Eugene Koontz added a comment - creating new patch with maxNumCnxns set sufficiently high for TestHCM to pass.
          Eugene Koontz made changes -
          Attachment HBASE-3861.patch [ 12478358 ]
          Eugene Koontz made changes -
          Attachment HBASE-3861.patch [ 12478357 ]
          Eugene Koontz made changes -
          Attachment HBASE-3861.patch [ 12478357 ]
          Eugene Koontz made changes -
          Field Original Value New Value
          Status Open [ 1 ] Patch Available [ 10002 ]
          Affects Version/s 0.90.3 [ 12316313 ]
          Assignee Eugene Koontz [ ekoontz ]
          Fix Version/s 0.90.3 [ 12316313 ]
          Hide
          Eugene Koontz added a comment -

          attaching HBASE-3861.patch

          Show
          Eugene Koontz added a comment - attaching HBASE-3861 .patch
          Eugene Koontz created issue -

            People

            • Assignee:
              Eugene Koontz
              Reporter:
              Eugene Koontz
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 1h
                1h
                Remaining:
                Remaining Estimate - 1h
                1h
                Logged:
                Time Spent - Not Specified
                Not Specified

                  Development