Details

    • Bug
    • Status: Resolved
    • Major
    • Resolution: Won't Fix
    • 2.2.0
    • None
    • None
    • None

    Description

      With HADOOP-10252, we fixed the IAE issue. However, the server isn't starting properly since listeners are not loaded.

      Attachments

        1. hadoop-10254.patch
          2 kB
          Jimmy Xiang

        Issue Links

          Activity

            jxiang Jimmy Xiang added a comment -

            The patch added the listeners. With this patch. HBase TestInfoServer works fine now.

            jxiang Jimmy Xiang added a comment - The patch added the listeners. With this patch. HBase TestInfoServer works fine now.
            wheat9 Haohui Mai added a comment -

            The clean way to do so is to manipulate the builder object instead of copying code into the constructor.

            And can you elaborate what does the unit test cover?

            wheat9 Haohui Mai added a comment - The clean way to do so is to manipulate the builder object instead of copying code into the constructor. And can you elaborate what does the unit test cover?
            jxiang Jimmy Xiang added a comment -

            I thought about refractory the builder. However, we don't need all the functionality in the builder. This way is much simpler. My HBase cluster with the patch works. As to the unit test, I want to make sure the connector address is not null, since we see NPE in HBase unit tests. Thanks.

            jxiang Jimmy Xiang added a comment - I thought about refractory the builder. However, we don't need all the functionality in the builder. This way is much simpler. My HBase cluster with the patch works. As to the unit test, I want to make sure the connector address is not null, since we see NPE in HBase unit tests. Thanks.
            hadoopqa Hadoop QA added a comment -

            +1 overall. Here are the results of testing the latest attachment
            http://issues.apache.org/jira/secure/attachment/12624412/hadoop-10254.patch
            against trunk revision .

            +1 @author. The patch does not contain any @author tags.

            +1 tests included. The patch appears to include 1 new or modified test files.

            +1 javac. The applied patch does not increase the total number of javac compiler warnings.

            +1 javadoc. The javadoc tool did not generate any warning messages.

            +1 eclipse:eclipse. The patch built with eclipse:eclipse.

            +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

            +1 release audit. The applied patch does not increase the total number of release audit warnings.

            +1 core tests. The patch passed unit tests in hadoop-common-project/hadoop-common.

            +1 contrib tests. The patch passed contrib unit tests.

            Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/3461//testReport/
            Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3461//console

            This message is automatically generated.

            hadoopqa Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12624412/hadoop-10254.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-common-project/hadoop-common. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/3461//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3461//console This message is automatically generated.
            wheat9 Haohui Mai added a comment -

            Connector address is supposed to be null when the code is not listening on the privileged port. I'm yet to be convinced that changing the code just to pass unit tests in HBase.

            It seems to me that it might be worthwhile to take a step back to think through what is the right approach rather than quickly putting in some code and rushing for a release.

            wheat9 Haohui Mai added a comment - Connector address is supposed to be null when the code is not listening on the privileged port. I'm yet to be convinced that changing the code just to pass unit tests in HBase. It seems to me that it might be worthwhile to take a step back to think through what is the right approach rather than quickly putting in some code and rushing for a release.
            stack Michael Stack added a comment -

            Just took a look at HttpServer in branch-2 comparing to branch-2.2. Radical change.

            Jimmy copying what was in build into constructor, is there a means of making sure in the build call we don't re-add the unmanaged listener? Ditto for the createDefaultChannelConnector. wheat9 help us out please. We need this fixed. Thanks.

            stack Michael Stack added a comment - Just took a look at HttpServer in branch-2 comparing to branch-2.2. Radical change. Jimmy copying what was in build into constructor, is there a means of making sure in the build call we don't re-add the unmanaged listener? Ditto for the createDefaultChannelConnector. wheat9 help us out please. We need this fixed. Thanks.
            jxiang Jimmy Xiang added a comment -

            Those two methods are private, won't be called many times. wheat9, why is the connector address supposed to be null? The connectors are added in loadListeners(), right?

            jxiang Jimmy Xiang added a comment - Those two methods are private, won't be called many times. wheat9 , why is the connector address supposed to be null? The connectors are added in loadListeners(), right?
            wheat9 Haohui Mai added a comment -

            Looked at the InfoServer in hbase.

            Given the current status of the code, it seems to me that it is difficult to do so if InfoServer inherits HttpServer.

            My suggestion is to adopt parts of the early patches in HADOOP-10232, that are:

            1. Use composition instead of inheritance.
            2. Expose appDir() in the builder.
            3. Expose getWebAppContext() in {HttpServer}} to customize the log directory.

            It should work for the release aligned with 2.4, but just keep your heads up – there are no guarantees that this APIs will stay stable. HBase might be broken again whenever we move away from Jetty 6. I anticipate some security-related work (e.g. HDFS-5716) could potentially change the APIs as well. The work can start as early as 2.5, so you really need to evaluate the balance of riding along with HttpServer versus the difficulties on working with different Hadoop versions.

            On the other hand, can you please elaborate on your concerns of HBASE-10336? It is a big patch indeed, but most of them are copying from the hadoop codebase directly thus the risk is minimal.

            wheat9 Haohui Mai added a comment - Looked at the InfoServer in hbase. Given the current status of the code, it seems to me that it is difficult to do so if InfoServer inherits HttpServer . My suggestion is to adopt parts of the early patches in HADOOP-10232 , that are: Use composition instead of inheritance. Expose appDir() in the builder. Expose getWebAppContext() in {HttpServer}} to customize the log directory. It should work for the release aligned with 2.4, but just keep your heads up – there are no guarantees that this APIs will stay stable. HBase might be broken again whenever we move away from Jetty 6. I anticipate some security-related work (e.g. HDFS-5716 ) could potentially change the APIs as well. The work can start as early as 2.5, so you really need to evaluate the balance of riding along with HttpServer versus the difficulties on working with different Hadoop versions. On the other hand, can you please elaborate on your concerns of HBASE-10336 ? It is a big patch indeed, but most of them are copying from the hadoop codebase directly thus the risk is minimal.
            wheat9 Haohui Mai added a comment -

            Exposing Connector into the builder's API is essentially a hack to allow datanode to take privileged HTTP port. It is there for historical reasons. Other users should never use it.

            wheat9 Haohui Mai added a comment - Exposing Connector into the builder's API is essentially a hack to allow datanode to take privileged HTTP port. It is there for historical reasons. Other users should never use it.
            jxiang Jimmy Xiang added a comment -

            Per HADOOP-10253, it will be cleaner to bring the original HttpServer back.

            jxiang Jimmy Xiang added a comment - Per HADOOP-10253 , it will be cleaner to bring the original HttpServer back.
            stack Michael Stack added a comment - - edited

            wheat9 You have made the same request multiple times in multiple different issues. We do not want existing releases to break going from 2.2 to 2.4.

            jxiang If we revert HADOOP-9784, does stuff work again? HADOOP-9784 is a nice-to-have improvement that 'affects' 3.0 that gentleman lukekliu appled for 2.4 timeframe. I'm sure junping_du would be cool removing this from 2.4 if he knew it was messing up downstreamers.

            stack Michael Stack added a comment - - edited wheat9 You have made the same request multiple times in multiple different issues. We do not want existing releases to break going from 2.2 to 2.4. jxiang If we revert HADOOP-9784 , does stuff work again? HADOOP-9784 is a nice-to-have improvement that 'affects' 3.0 that gentleman lukekliu appled for 2.4 timeframe. I'm sure junping_du would be cool removing this from 2.4 if he knew it was messing up downstreamers.
            jxiang Jimmy Xiang added a comment -

            stack, it won't work just removing HADOOP-9784. We need to remove HDFS-5545 too. My patch here is kind of reverting some change to HDFS-5545: still keeping the listener init in the deprecated constructor. So, to me, the patch is a valid fix. Haohui's suggestion makes things more complex.

            jxiang Jimmy Xiang added a comment - stack , it won't work just removing HADOOP-9784 . We need to remove HDFS-5545 too. My patch here is kind of reverting some change to HDFS-5545 : still keeping the listener init in the deprecated constructor. So, to me, the patch is a valid fix. Haohui's suggestion makes things more complex.
            eric@apache.org Eric Charles added a comment -

            Reverting HADOOP-9784 and HDFS-5545 is too much work IMHO

            jxiang Your patch should work for the port connector. On HBASE-10336, you say "I just tested. My HBase cluster works fine with it." - do you mean you applied HADOOP-10254, ran hbase without any change, and got access to all web contexts. If this is the case, I guess the easy solution is to apply this on hadoop.

            eric@apache.org Eric Charles added a comment - Reverting HADOOP-9784 and HDFS-5545 is too much work IMHO jxiang Your patch should work for the port connector. On HBASE-10336 , you say "I just tested. My HBase cluster works fine with it." - do you mean you applied HADOOP-10254 , ran hbase without any change, and got access to all web contexts. If this is the case, I guess the easy solution is to apply this on hadoop.
            jxiang Jimmy Xiang added a comment -

            I checked the master web UI. I clicked several links there. I checked the region server UI too. Not tested every link, of course. I agree this is the easy solution.

            jxiang Jimmy Xiang added a comment - I checked the master web UI. I clicked several links there. I checked the region server UI too. Not tested every link, of course. I agree this is the easy solution.
            eric@apache.org Eric Charles added a comment -

            jxiang then it is ok (I was asking because I had issue decoupling hbase from hadoop jetty for a few servlets, but here if the server starts, all servlets should be up). master and region were also using different methods, but good to have checked both,

            eric@apache.org Eric Charles added a comment - jxiang then it is ok (I was asking because I had issue decoupling hbase from hadoop jetty for a few servlets, but here if the server starts, all servlets should be up). master and region were also using different methods, but good to have checked both,

            People

              jxiang Jimmy Xiang
              jxiang Jimmy Xiang
              Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: