Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.2.1
    • Fix Version/s: 2.2.1, 2.2.2
    • Component/s: Jetty
    • Security Level: public (Regular issues)
    • Labels:
      None
    • Environment:

      Only tested on OS X 1.6 with Java 1.6

    • Patch Info:
      Patch Available

      Description

      Geronimo applies the maxThreads parameter as the number of acceptor threads in Jetty, with a default value of 50. With newer versions (7.2.0) of Jetty this gives the following warning during startup:

      1. Acceptors should be <=2*availableProcessors

      Server shutdowns are also very slow, it takes about 1 second to stop each acceptor thread.

      The attached patch sets the number of acceptor threads to 2 per connector (HTTP, HTTPS and AJP), and exposes this and the acceptor queue size (50) as a parameter in config-substitutions.properties.

      1. geronimo-2.2-1035896-jetty-7.2.1.patch
        4 kB
        Trygve Sanne Hardersen
      2. geronimo-2.2-1028765-jetty-trunk.patch
        2 kB
        Trygve Sanne Hardersen
      3. geronimo-2.2-1027647-jetty-acceptors.patch
        4 kB
        Trygve Sanne Hardersen

        Activity

        Hide
        Shawn Jiang added a comment -

        Patch committed with 22@r1027647. The closing speed increases remarkably with the patch. But it's still slower than before.

        Let's see if the upgrade will cause any new tck failures before deciding to revert the upgrade.

        Show
        Shawn Jiang added a comment - Patch committed with 22@r1027647. The closing speed increases remarkably with the patch. But it's still slower than before. Let's see if the upgrade will cause any new tck failures before deciding to revert the upgrade.
        Hide
        Trygve Sanne Hardersen added a comment -

        I have looked a bit more at this and have filed a bug for Jetty, see https://bugs.eclipse.org/bugs/show_bug.cgi?id=329180. This is based on Shawn's work, many thanks.

        I'll watch the Jetty bug and let you know if it gets fixed at the Jetty side so that Geronimo can be updated to use a newer Jetty version. I have a patch ready for running Geronimo against Jetty trunk.

        Show
        Trygve Sanne Hardersen added a comment - I have looked a bit more at this and have filed a bug for Jetty, see https://bugs.eclipse.org/bugs/show_bug.cgi?id=329180 . This is based on Shawn's work, many thanks. I'll watch the Jetty bug and let you know if it gets fixed at the Jetty side so that Geronimo can be updated to use a newer Jetty version. I have a patch ready for running Geronimo against Jetty trunk.
        Hide
        Trygve Sanne Hardersen added a comment -

        The slow shutdown was fixed in Jetty trunk in revision 2460. I have tested this with Geronimo 2.2 and with this shutdowns are fast even with many acceptor threads. However I still believe the original patch should be kept because 50 acceptor threads generates a warning.

        Attached is a patch that upgrades Geronimo 2.2 to use Jetty trunk (7.2.1-SNAPSHOT). The patch changes the Jetty version number, adds the Jetty snapshot repository to the pom, and changes the AbstractWebModuleTest to use the auth method string.

        Feel free to remove my comments from the code - they are only there to explain the changes. Thanks!

        Show
        Trygve Sanne Hardersen added a comment - The slow shutdown was fixed in Jetty trunk in revision 2460. I have tested this with Geronimo 2.2 and with this shutdowns are fast even with many acceptor threads. However I still believe the original patch should be kept because 50 acceptor threads generates a warning. Attached is a patch that upgrades Geronimo 2.2 to use Jetty trunk (7.2.1-SNAPSHOT). The patch changes the Jetty version number, adds the Jetty snapshot repository to the pom, and changes the AbstractWebModuleTest to use the auth method string. Feel free to remove my comments from the code - they are only there to explain the changes. Thanks!
        Hide
        Kevan Miller added a comment -

        Nice.

        Is there an outlook for Jetty 7.2.1 release? We're getting pretty close to a G 2.2.1 release, I think. Would be great to see this problem fixed in 2.2.1, but would hate to see a Jetty release holding up the 2.2.1 release...

        Trygve, many thanks for tracking this issue down!

        Show
        Kevan Miller added a comment - Nice. Is there an outlook for Jetty 7.2.1 release? We're getting pretty close to a G 2.2.1 release, I think. Would be great to see this problem fixed in 2.2.1, but would hate to see a Jetty release holding up the 2.2.1 release... Trygve, many thanks for tracking this issue down!
        Hide
        Trygve Sanne Hardersen added a comment -

        From the history it seems that Jetty 7.x releases are made approximately every 14 days. 7.2.0 was released on October 20th, and I can't find any open bugs targeting 7.2.x, but I have no idea when a new release will be made.

        Downgrading from 7.2.1-SNAPSHOT to 7.2.0 is trivial now, but I'm also maintaining a local Geronimo version against Jetty 7.2.1-SNAPSHOT, and can provide a new patch when 7.2.1 is released, be it before or after Geronimo 2.2.1 ships.

        Show
        Trygve Sanne Hardersen added a comment - From the history it seems that Jetty 7.x releases are made approximately every 14 days. 7.2.0 was released on October 20th, and I can't find any open bugs targeting 7.2.x, but I have no idea when a new release will be made. Downgrading from 7.2.1-SNAPSHOT to 7.2.0 is trivial now, but I'm also maintaining a local Geronimo version against Jetty 7.2.1-SNAPSHOT, and can provide a new patch when 7.2.1 is released, be it before or after Geronimo 2.2.1 ships.
        Hide
        Shawn Jiang added a comment -

        Thanks Trygve,

        With other fix 2.2@1031437, 2.2 TCK passed with jetty(7.2.0.v20101020) build. I think we are safe to release 2.2.1 without another jetty release.

        Sure it's better to include the new jetty release with the fix to the root cause if possible.

        Show
        Shawn Jiang added a comment - Thanks Trygve, With other fix 2.2@1031437, 2.2 TCK passed with jetty(7.2.0.v20101020) build. I think we are safe to release 2.2.1 without another jetty release. Sure it's better to include the new jetty release with the fix to the root cause if possible.
        Hide
        Trygve Sanne Hardersen added a comment -

        Thanks for testing Shawn. Jetty 7.2.1.v20101111 has been released to the central Maven repo. I'm attaching an updated patch against Geronimo 2.2 r1035896. The comments in the patch are there to explain my thinking - feel free to remove them.

        Thanks!

        Show
        Trygve Sanne Hardersen added a comment - Thanks for testing Shawn. Jetty 7.2.1.v20101111 has been released to the central Maven repo. I'm attaching an updated patch against Geronimo 2.2 r1035896. The comments in the patch are there to explain my thinking - feel free to remove them. Thanks!
        Hide
        Shawn Jiang added a comment -

        Should be fixed.

        Show
        Shawn Jiang added a comment - Should be fixed.

          People

          • Assignee:
            Shawn Jiang
            Reporter:
            Trygve Sanne Hardersen
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development