Details

    • Type: Improvement Improvement
    • Status: Patch Available
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 2.2.0
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      Jetty6 is no longer maintained. Update the dependency to jetty9.

        Issue Links

          Activity

          Hide
          Robert Rati added a comment -

          This patch brings the source from 6->9. There is a different jira (https://issues.apache.org/jira/browse/HADOOP-9650) for updating to jetty8. If that patch is accepted, this will need to be updated to address moving from 8->9.

          This patch uses glassfish in combination with jetty9 for jsp compilation.

          Show
          Robert Rati added a comment - This patch brings the source from 6->9. There is a different jira ( https://issues.apache.org/jira/browse/HADOOP-9650 ) for updating to jetty8. If that patch is accepted, this will need to be updated to address moving from 8->9. This patch uses glassfish in combination with jetty9 for jsp compilation.
          Hide
          Colin Patrick McCabe added a comment -

          be sure to hit "submit patch" so that you will get a jenkins run on this.

          Show
          Colin Patrick McCabe added a comment - be sure to hit "submit patch" so that you will get a jenkins run on this.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12610564/HADOOP-10075.patch
          against trunk revision .

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3303//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12610564/HADOOP-10075.patch against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3303//console This message is automatically generated.
          Hide
          Colin Patrick McCabe added a comment -

          Thanks for looking at this. I think you will need to re-generate the patch, since it failed to apply on jenkins.

          --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestSSLHttpServer.java
          +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestSSLHttpServer.java
          @@ -76,6 +76,7 @@ public void setup() throws Exception {
           
               conf.setInt(HttpServer.HTTP_MAX_THREADS, 10);
               conf.addResource(CONFIG_SITE_XML);
          +    conf.addResource(conf.get("hadoop.ssl.server.conf","ssl-server.xml"));
               server = createServer("test", conf);
               server.addServlet("echo", "/echo", TestHttpServer.EchoServlet.class);
               server.start();
          

          Why do we need this addition?

          -        InetAddress.getByName(server.getConnectors()[0].getHost());
          -      int port = server.getConnectors()[0].getPort();
          +        InetAddress.getByName(((ServerConnector)server.getConnectors()[0]).getHost());
          +      int port = ((ServerConnector)server.getConnectors()[0]).getPort();
          

          I see a lot of new typecasts like this. Is it possible to avoid these? If not, could we have an accessor function that makes this easier to read? Thanks.

          Show
          Colin Patrick McCabe added a comment - Thanks for looking at this. I think you will need to re-generate the patch, since it failed to apply on jenkins. --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestSSLHttpServer.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestSSLHttpServer.java @@ -76,6 +76,7 @@ public void setup() throws Exception { conf.setInt(HttpServer.HTTP_MAX_THREADS, 10); conf.addResource(CONFIG_SITE_XML); + conf.addResource(conf.get( "hadoop.ssl.server.conf" , "ssl-server.xml" )); server = createServer( "test" , conf); server.addServlet( "echo" , "/echo" , TestHttpServer.EchoServlet.class); server.start(); Why do we need this addition? - InetAddress.getByName(server.getConnectors()[0].getHost()); - int port = server.getConnectors()[0].getPort(); + InetAddress.getByName(((ServerConnector)server.getConnectors()[0]).getHost()); + int port = ((ServerConnector)server.getConnectors()[0]).getPort(); I see a lot of new typecasts like this. Is it possible to avoid these? If not, could we have an accessor function that makes this easier to read? Thanks.
          Hide
          Robert Rati added a comment -

          The patch applies cleanly against branch-2.2.0. Which branch should I target? branch-2.2? The nature of these changes will probably result in the patch failing to apply pretty consistently as development continues until/less it makes it into a source mainline.

          conf.addResource(conf.get("hadoop.ssl.server.conf","ssl-server.xml"));
          was needed for tests to succeed on Fedora. Jetty 8 revamped the ssl configuration and this change was part of that for the test to pass.

          Jetty 9 completely revamped Connector and related classes and casts such as:
          (ServerConnector)server.getConnectors()[0])
          was the only way I could see to get at the needed data. If there's a cleaner way I'm all for it.

          Show
          Robert Rati added a comment - The patch applies cleanly against branch-2.2.0. Which branch should I target? branch-2.2? The nature of these changes will probably result in the patch failing to apply pretty consistently as development continues until/less it makes it into a source mainline. conf.addResource(conf.get("hadoop.ssl.server.conf","ssl-server.xml")); was needed for tests to succeed on Fedora. Jetty 8 revamped the ssl configuration and this change was part of that for the test to pass. Jetty 9 completely revamped Connector and related classes and casts such as: (ServerConnector)server.getConnectors() [0] ) was the only way I could see to get at the needed data. If there's a cleaner way I'm all for it.
          Hide
          Colin Patrick McCabe added a comment -

          Please target trunk, as we do for (almost) all upstream patches. Then we will backport it into stable branches as is appropriate.

          Jetty 8 revamped the ssl configuration and [conf.addResource] was part of that for the test to pass.

          OK.

          Can you be a little more clear about why the typecasts are needed? What other kinds of connectors might be returned by server#getConnectors? Have you considered making a utility routine to do this typecast?

          Note that in general, nothing can be committed without a clean jenkins run. There's more information here: http://wiki.apache.org/hadoop/HowToContribute

          Show
          Colin Patrick McCabe added a comment - Please target trunk, as we do for (almost) all upstream patches. Then we will backport it into stable branches as is appropriate. Jetty 8 revamped the ssl configuration and [conf.addResource] was part of that for the test to pass. OK. Can you be a little more clear about why the typecasts are needed? What other kinds of connectors might be returned by server#getConnectors ? Have you considered making a utility routine to do this typecast? Note that in general, nothing can be committed without a clean jenkins run. There's more information here: http://wiki.apache.org/hadoop/HowToContribute
          Hide
          Robert Rati added a comment -

          Jetty 9 redid the Connector interface and set of classes. The Connector interface no longer contains any network related methods and those are now in the NetworkConnector interface, which extends the Connector interface.

          server#getConnectors still returns an array of Connectors, but obviously those Connectors no longer have any networking data so I needed to cast to something that does. It might be better to cast to NetworkConnector instead of ServerConnector though.

          I have no problem creating a utility routine if that would make things more readable. I'd prefer to batch any requested changes into a single update if possible. Any other concerns?

          Show
          Robert Rati added a comment - Jetty 9 redid the Connector interface and set of classes. The Connector interface no longer contains any network related methods and those are now in the NetworkConnector interface, which extends the Connector interface. server#getConnectors still returns an array of Connectors, but obviously those Connectors no longer have any networking data so I needed to cast to something that does. It might be better to cast to NetworkConnector instead of ServerConnector though. I have no problem creating a utility routine if that would make things more readable. I'd prefer to batch any requested changes into a single update if possible. Any other concerns?
          Hide
          Colin Patrick McCabe added a comment -

          it looks fine aside from the things I mentioned.

          Show
          Colin Patrick McCabe added a comment - it looks fine aside from the things I mentioned.
          Hide
          Colin Patrick McCabe added a comment -

          Another thing: will this break HBase, which is currently using some of our jetty stuff?

          Show
          Colin Patrick McCabe added a comment - Another thing: will this break HBase, which is currently using some of our jetty stuff?
          Hide
          Liang Xie added a comment -

          Hi stack, could you invite some guys to double-check Colin Patrick McCabe's last comments? I'd like to help when i read but i am not familiar this area

          Show
          Liang Xie added a comment - Hi stack , could you invite some guys to double-check Colin Patrick McCabe 's last comments? I'd like to help when i read but i am not familiar this area
          Hide
          Robert Rati added a comment -

          Yes, HBase will need to change to jetty 9 as well. I'm working on HBase and will be providing a patch to them when done. The change needed for HBase to interact with Hadoop using jetty9 is pretty minor from the work I've done so far. Iiirc, it's a returned variable type change from a call into hadoop (Connector class issue again).

          Show
          Robert Rati added a comment - Yes, HBase will need to change to jetty 9 as well. I'm working on HBase and will be providing a patch to them when done. The change needed for HBase to interact with Hadoop using jetty9 is pretty minor from the work I've done so far. Iiirc, it's a returned variable type change from a call into hadoop (Connector class issue again).
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12610564/HADOOP-10075.patch
          against trunk revision .

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3828//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12610564/HADOOP-10075.patch against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3828//console This message is automatically generated.
          Hide
          Demai Ni added a comment -

          Robert Rati, do you have a hbase jira/patch available? Thanks... Demai

          Show
          Demai Ni added a comment - Robert Rati , do you have a hbase jira/patch available? Thanks... Demai
          Hide
          Jinghui Wang added a comment -

          I am running some unit tests with the attached patch applied and started seeing errors in many httpserver related UTs:
          java.lang.SecurityException: Signers of 'javax.servlet.HttpConstraintElement' do not match signers of other classes in package

          After find going through maven dependency tree, I found that both javax.servlet:servlet-api and org.eclipse.jetty.orbit:javax.servlet (through jetty-server) are included on the classpath. Wondering if I am missing something or the patch has not been tested with Hadoop unit tests yet.

          maven dependency output:

          [INFO] +- javax.servlet:servlet-api:jar:3.0-alpha-1:compile
          [INFO] | +- org.eclipse.jetty.orbit:javax.servlet:jar:3.0.0.v201112011016:compile
          [INFO] +- org.eclipse.jetty:jetty-servlet:jar:9.0.4.v20130625:compile
          [INFO] | - (org.eclipse.jetty:jetty-servlet:jar:9.0.4.v20130625:compile - omitted for duplicate)
          [INFO] +- org.glassfish.web:javax.servlet.jsp:jar:2.2.6:compile
          [INFO] | - javax.servlet.jsp:javax.servlet.jsp-api:jar:2.2.1:compile
          [INFO] +- org.apache.tomcat:tomcat-servlet-api:jar:7.0.37:compile
          [INFO] | +- (javax.servlet:servlet-api:jar:3.0-alpha-1:provided - omitted for duplicate)
          [INFO] | | +- org.eclipse.jetty.orbit:javax.servlet:jar:3.0.0.v201112011016:provided
          [INFO] | +- org.eclipse.jetty:jetty-servlet:jar:9.0.4.v20130625:provided
          [INFO] | | - (org.eclipse.jetty:jetty-servlet:jar:9.0.4.v20130625:provided - omitted for duplicate)
          [INFO] | +- org.glassfish.web:javax.servlet.jsp:jar:2.2.6:provided
          [INFO] | | - javax.servlet.jsp:javax.servlet.jsp-api:jar:2.2.1:provided
          [INFO] | +- org.apache.tomcat:tomcat-servlet-api:jar:7.0.37:provided
          [INFO] +- javax.servlet:servlet-api:jar:3.0-alpha-1:provided

          Show
          Jinghui Wang added a comment - I am running some unit tests with the attached patch applied and started seeing errors in many httpserver related UTs: java.lang.SecurityException: Signers of 'javax.servlet.HttpConstraintElement' do not match signers of other classes in package After find going through maven dependency tree, I found that both javax.servlet:servlet-api and org.eclipse.jetty.orbit:javax.servlet (through jetty-server) are included on the classpath. Wondering if I am missing something or the patch has not been tested with Hadoop unit tests yet. maven dependency output: [INFO] +- javax.servlet:servlet-api:jar:3.0-alpha-1:compile [INFO] | +- org.eclipse.jetty.orbit:javax.servlet:jar:3.0.0.v201112011016:compile [INFO] +- org.eclipse.jetty:jetty-servlet:jar:9.0.4.v20130625:compile [INFO] | - (org.eclipse.jetty:jetty-servlet:jar:9.0.4.v20130625:compile - omitted for duplicate) [INFO] +- org.glassfish.web:javax.servlet.jsp:jar:2.2.6:compile [INFO] | - javax.servlet.jsp:javax.servlet.jsp-api:jar:2.2.1:compile [INFO] +- org.apache.tomcat:tomcat-servlet-api:jar:7.0.37:compile [INFO] | +- (javax.servlet:servlet-api:jar:3.0-alpha-1:provided - omitted for duplicate) [INFO] | | +- org.eclipse.jetty.orbit:javax.servlet:jar:3.0.0.v201112011016:provided [INFO] | +- org.eclipse.jetty:jetty-servlet:jar:9.0.4.v20130625:provided [INFO] | | - (org.eclipse.jetty:jetty-servlet:jar:9.0.4.v20130625:provided - omitted for duplicate) [INFO] | +- org.glassfish.web:javax.servlet.jsp:jar:2.2.6:provided [INFO] | | - javax.servlet.jsp:javax.servlet.jsp-api:jar:2.2.1:provided [INFO] | +- org.apache.tomcat:tomcat-servlet-api:jar:7.0.37:provided [INFO] +- javax.servlet:servlet-api:jar:3.0-alpha-1:provided
          Hide
          Steve Loughran added a comment -

          IMO we'd be better off moving out of Jetty and into jersey as the server; this would eliminate jersey version problems altogether, and more importantly, jersey "quirks"

          Show
          Steve Loughran added a comment - IMO we'd be better off moving out of Jetty and into jersey as the server; this would eliminate jersey version problems altogether, and more importantly, jersey "quirks"

            People

            • Assignee:
              Robert Rati
              Reporter:
              Robert Rati
            • Votes:
              0 Vote for this issue
              Watchers:
              15 Start watching this issue

              Dates

              • Created:
                Updated:

                Development