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

        Issue Links

          Activity

          Kathey Marsden created issue -
          Kathey Marsden made changes -
          Field Original Value New Value
          Summary Provide a property to increase network server start timeout for JUnitTests Provide a property to increase network server start timeout for JUnit tests
          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.
          Myrna van Lunteren made changes -
          Attachment DERBY-4347.diff [ 12418568 ]
          Myrna van Lunteren made changes -
          Issue & fix info [Patch Available]
          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.
          Myrna van Lunteren made changes -
          Attachment DERBY-4347.diffv2 [ 12419344 ]
          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.
          Myrna van Lunteren made changes -
          Assignee Myrna van Lunteren [ myrna ]
          Issue & fix info [Patch Available]
          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
          Myrna van Lunteren made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Fix Version/s 10.5.3.1 [ 12314182 ]
          Fix Version/s 10.6.0.0 [ 12313727 ]
          Resolution Fixed [ 1 ]
          Kathey Marsden made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Gavin made changes -
          Workflow jira [ 12472929 ] Default workflow, editable Closed status [ 12800043 ]
          Myrna van Lunteren made changes -
          Link This issue relates to DERBY-3873 [ DERBY-3873 ]
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Resolved Resolved
          34d 7h 26m 1 Myrna van Lunteren 15/Sep/09 00:12
          Resolved Resolved Closed Closed
          484d 23h 11m 1 Kathey Marsden 12/Jan/11 22:24

            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