Derby
  1. Derby
  2. DERBY-4347

Provide a property to increase network server start timeout for JUnit tests

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 10.5.3.0, 10.6.1.0
    • Fix Version/s: 10.5.3.1, 10.6.1.0
    • Component/s: Test
    • Labels:
      None

      Description

      Sometimes when running JUnit tests with jvm options that are known to slow things down significantly network server start timeouts can occur e.g.
      SecureServerTest( Opened = false, Authenticated= false, CustomDerbyProperties= null, WildCardHost= null )junit.framework.AssertionFailedError: Timed out waiting for network server to start:Spawned SpawnedNetworkServer exitCode=143
      at org.apache.derbyTesting.junit.NetworkServerTestSetup.setUp(NetworkServerTestSetup.java:203)
      at junit.extensions.TestSetup$1.protect(TestSetup.java:18)
      at junit.extensions.TestSetup.run(TestSetup.java:23)
      at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
      at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22)
      at junit.extensions.TestSetup$1.protect(TestSetup.java:19)
      at junit.extensions.TestSetup.run(TestSetup.java:23)
      at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22)
      at junit.extensions.TestSetup$1.protect(TestSetup.java:19)
      at junit.extensions.TestSetup.run(TestSetup.java:23)
      at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22)
      at junit.extensions.TestSetup$1.protect(TestSetup.java:19)
      at junit.extensions.TestSetup.run(TestSetup.java:23)

      The current wait time is 40 seconds and is set in org.apache.derbyTesting.junit.NetworkServerTestSetup

      /** Setting maximum wait time to 40 seconds. On some platforms

      • it may take this long to start the server. Increasing the wait
      • time should not adversely affect those
      • systems with fast port turnaround as the actual code loops for
      • SLEEP_TIME intervals, so should never see WAIT_TIME.
        */
        private static final long WAIT_TIME = 40000;

      It would be nice to have system property (maybe derby.tests.networkServerStartTimeout=<ms>) to allow this to be configurable in environments where we expect the start to take longer.

      I am not sure if there are other timeouts in the tests for replication etc or if they all use this same setting.

      1. DERBY-4347.diff
        2 kB
        Myrna van Lunteren
      2. DERBY-4347.diffv2
        3 kB
        Myrna van Lunteren

        Activity

        Hide
        Knut Anders Hatlen added a comment -

        We could also just increase the default wait time. It shouldn't have any negative effects except that it takes longer for the test to fail if the server doesn't start, but that's hardly worth optimizing for.

        Minor nit: If we add the system property, I think it would be better to specify the number of seconds to wait instead of milliseconds, as I don't see the need for ms resolution. It would also be more in line with the other (non-testing) timeout properties like derby.locks.*timeout.

        Show
        Knut Anders Hatlen added a comment - We could also just increase the default wait time. It shouldn't have any negative effects except that it takes longer for the test to fail if the server doesn't start, but that's hardly worth optimizing for. Minor nit: If we add the system property, I think it would be better to specify the number of seconds to wait instead of milliseconds, as I don't see the need for ms resolution. It would also be more in line with the other (non-testing) timeout properties like derby.locks.*timeout.
        Hide
        Myrna van Lunteren added a comment -

        Although I'm wary of adding configurable properties (because the old test harness had many, most of them badly documented and/or not working properly) this one has long been on my wish list.
        For instance, when running platform tests, for which I only have slower machines available. Or, like Kathey says, for doing experiments with jvm settings.
        You can make a specialized build with the slower setting, but it gets old, making the same change repeatedly.

        I don't think making that change permanently for these one-off situations is necessary.

        There are a few tests which expect the server to fail, and those would be sitting around even longer (although it's probably only 1 or 2 tests, so on a complete testrun it probably doesn't matter so much). Also, I'd worry that we hide server startup performance creep if we'd increase the default (although there's probably/hopefully a performance tests for that somewhere).

        I agree on Knut's point re the seconds vs. milliseconds.

        Show
        Myrna van Lunteren added a comment - Although I'm wary of adding configurable properties (because the old test harness had many, most of them badly documented and/or not working properly) this one has long been on my wish list. For instance, when running platform tests, for which I only have slower machines available. Or, like Kathey says, for doing experiments with jvm settings. You can make a specialized build with the slower setting, but it gets old, making the same change repeatedly. I don't think making that change permanently for these one-off situations is necessary. There are a few tests which expect the server to fail, and those would be sitting around even longer (although it's probably only 1 or 2 tests, so on a complete testrun it probably doesn't matter so much). Also, I'd worry that we hide server startup performance creep if we'd increase the default (although there's probably/hopefully a performance tests for that somewhere). I agree on Knut's point re the seconds vs. milliseconds.
        Hide
        Kathey Marsden added a comment -

        I seem to recall concern that increasing the default wait time might mask actual network server start performance regressions. Seconds instead of milliseconds sounds fine.

        Show
        Kathey Marsden added a comment - I seem to recall concern that increasing the default wait time might mask actual network server start performance regressions. Seconds instead of milliseconds sounds fine.
        Hide
        Myrna van Lunteren added a comment -

        Attaching a first attempt at implementing this.
        I'm running suites.All now. Would appreciate review.
        This patch would enable one to overwrite the default wait time passing on the number of seconds using the property derby.tests.networkServerStartTimeout.

        Show
        Myrna van Lunteren added a comment - Attaching a first attempt at implementing this. I'm running suites.All now. Would appreciate review. This patch would enable one to overwrite the default wait time passing on the number of seconds using the property derby.tests.networkServerStartTimeout.
        Hide
        Myrna van Lunteren added a comment -

        Forgot to report back - I got 1 "new" test failure on my machine (ignoring engine.ErrorStreamTest.testDefault which failed on windows although mamta checked in a fix yesterday, my build must be from before) when running suites.All (but not when running the test by itself);
        ttestSetPortPriority(org.apache.derbyTesting.functionTests.tests.derbynet.ServerPropertiesTest)junit.framework.AssertionFailedError: Can not ping server specified with -p
        But I reverted my change, ran the suite again, and got the same error, so I don't think it's related.

        Show
        Myrna van Lunteren added a comment - Forgot to report back - I got 1 "new" test failure on my machine (ignoring engine.ErrorStreamTest.testDefault which failed on windows although mamta checked in a fix yesterday, my build must be from before) when running suites.All (but not when running the test by itself); ttestSetPortPriority(org.apache.derbyTesting.functionTests.tests.derbynet.ServerPropertiesTest)junit.framework.AssertionFailedError: Can not ping server specified with -p But I reverted my change, ran the suite again, and got the same error, so I don't think it's related.
        Hide
        Kathey Marsden added a comment -

        Hi Myrna, I think it would be clearer for the test to throw an error if it cannot parse the timeout rather than just print a message. Sometimes with automated test runs the output is not immediately apparent as we found with the encryption test that was printing for a while that it was not running but did not fail.

        Show
        Kathey Marsden added a comment - Hi Myrna, I think it would be clearer for the test to throw an error if it cannot parse the timeout rather than just print a message. Sometimes with automated test runs the output is not immediately apparent as we found with the encryption test that was printing for a while that it was not running but did not fail.
        Hide
        Myrna van Lunteren added a comment -

        Attaching a new patch which attempts to address Kathey's comments.
        The getWaitTime() method has been restructured a bit, and now will cause a failure when an invalid value is passed in for derby.tests.networkServerStartTimeout.

        There were no problems when I ran suites.All with this patch.
        If there are no comments, I'll likely commit this in the next few days.

        Show
        Myrna van Lunteren added a comment - Attaching a new patch which attempts to address Kathey's comments. The getWaitTime() method has been restructured a bit, and now will cause a failure when an invalid value is passed in for derby.tests.networkServerStartTimeout. There were no problems when I ran suites.All with this patch. If there are no comments, I'll likely commit this in the next few days.
        Hide
        Myrna van Lunteren added a comment -

        I've committed the v2 patch to trunk with revision 814815.
        I'll work on backporting this to 10.5 and on updating appropriate areas in the wiki.

        Show
        Myrna van Lunteren added a comment - I've committed the v2 patch to trunk with revision 814815. I'll work on backporting this to 10.5 and on updating appropriate areas in the wiki.
        Hide
        Myrna van Lunteren added a comment -

        backported 814815 from trunk to 10.5 with revision 814906.
        I also added a section to http://wiki.apache.org/db-derby/DerbyJUnitTesting

        Show
        Myrna van Lunteren added a comment - backported 814815 from trunk to 10.5 with revision 814906. I also added a section to http://wiki.apache.org/db-derby/DerbyJUnitTesting

          People

          • Assignee:
            Myrna van Lunteren
            Reporter:
            Kathey Marsden
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development