Derby
  1. Derby
  2. DERBY-5316

Unload old JDBC drivers when done with them in the upgrade tests

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 10.9.1.0
    • Component/s: Test
    • Labels:
      None

      Description

      Discussed in this thread on derby-dev: http://mail-archives.apache.org/mod_mbox/db-derby-dev/201107.mbox/%3C4E146309.3000906@gmail.com%3E

      After we're done testing an old version in the upgrade tests, its classes are still loaded in the JVM because the old driver is referenced in DriverManager. We should find a way to unload the old drivers so that the memory usage of the upgrade tests is reduced. Now we typically need to run with -XX:MaxPermSize=200M or similar options to work around this issue.

      1. clear-contextservice.diff
        4 kB
        Knut Anders Hatlen
      2. deregister-check.diff
        5 kB
        Knut Anders Hatlen
      3. clear-threadlocal.diff
        3 kB
        Knut Anders Hatlen
      4. deregister-v2.diff
        8 kB
        Knut Anders Hatlen
      5. deregister.diff
        11 kB
        Knut Anders Hatlen

        Issue Links

          Activity

          Hide
          Kathey Marsden added a comment -

          Actually, I had hoped t that Lily's work on DERBY-2905 would resolve this issue, but it seems like it is still really hard to get Derby unloaded once loaded in a class loader. As it is more and more common to see users use custom class loaders, we should also document the appropriate way to not only shutdown but unload Derby when using class loaders.

          Show
          Kathey Marsden added a comment - Actually, I had hoped t that Lily's work on DERBY-2905 would resolve this issue, but it seems like it is still really hard to get Derby unloaded once loaded in a class loader. As it is more and more common to see users use custom class loaders, we should also document the appropriate way to not only shutdown but unload Derby when using class loaders.
          Hide
          Knut Anders Hatlen added a comment -

          I instrumented the DriverManager.registerDriver() and DriverManager.unregisterDriver() and ran the upgrade tests. It looks like the combination of DERBY-2905 and DERBY-5267 made the upgrade tests deregister driver versions lower than 10.2 and higher than 10.7. But we still have 11 versions that don't get deregistered.

          Show
          Knut Anders Hatlen added a comment - I instrumented the DriverManager.registerDriver() and DriverManager.unregisterDriver() and ran the upgrade tests. It looks like the combination of DERBY-2905 and DERBY-5267 made the upgrade tests deregister driver versions lower than 10.2 and higher than 10.7. But we still have 11 versions that don't get deregistered.
          Hide
          Kathey Marsden added a comment -

          That make sense. It is good to know that we are protected from class loader leaks moving forward with DERBY-2905. I keep forgetting with the upgrade tests we haven't solved the time travel problem.

          Perhaps the approach you mentioned in the email will work for the old versions. That just to have it here in the Jira was:
          "One possible solution may be to create a helper class with a method that
          unloads the embedded driver and somehow wire this class into the class
          loader in which the old version is running. The helper class could be
          used from the test tear-down. Then we should be able to trick
          DriverManager into unloading the driver. I think..."

          I wonder if it is possible to write a test to verify not only that the driver is deregistered (which I think was already done in DERBY-2905), but verify that class loaders get garbage collected and classes unloaded to prevent other issues from creeping in. Getting the upgrade tests to run within the default JVM parameters with the Sun JDK would be a good indicator but it would be nice to have a more targeted functional test. I am just not sure how we would test that directly.

          Show
          Kathey Marsden added a comment - That make sense. It is good to know that we are protected from class loader leaks moving forward with DERBY-2905 . I keep forgetting with the upgrade tests we haven't solved the time travel problem. Perhaps the approach you mentioned in the email will work for the old versions. That just to have it here in the Jira was: "One possible solution may be to create a helper class with a method that unloads the embedded driver and somehow wire this class into the class loader in which the old version is running. The helper class could be used from the test tear-down. Then we should be able to trick DriverManager into unloading the driver. I think..." I wonder if it is possible to write a test to verify not only that the driver is deregistered (which I think was already done in DERBY-2905 ), but verify that class loaders get garbage collected and classes unloaded to prevent other issues from creeping in. Getting the upgrade tests to run within the default JVM parameters with the Sun JDK would be a good indicator but it would be nice to have a more targeted functional test. I am just not sure how we would test that directly.
          Hide
          Kristian Waagan added a comment -

          The unstable and probably somewhat error-prone path would be to enable tracing of class loading/unloading and parse the output. This requires us to kick off a JVM process, and we have to deal with different options to enable the tracing, output formats etc. I would think such a test shouldn't be included in suites.All, since it may be sensitive to the test machine environment (i.e. VM vendor and version).

          Here's an example from running the upgrade tests with versions 10.7.1.1 and 10.8.1.2 (I did run GC explicitly before the JVM exited to be sure):
          $ grep "EmbeddedDriver" output.txt
          [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:release-jars/10.7.1.1/derby.jar]
          [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:trunk-netbeans/classes/]
          [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:release-jars/10.8.1.2/derby.jar]
          [Unloading class org.apache.derby.jdbc.EmbeddedDriver]

          Note that the "origin" of the class is not included in the unloading-message. I'm not sure if one would have to rely on counting of the events only, or if we can conclude based on our knowledge of the test (i.e. when/how classes are loaded/unloaded) and trace order. I expect that the filtering and parsing could be done in Java as a JUnit test.
          I have not experimented further.

          Show
          Kristian Waagan added a comment - The unstable and probably somewhat error-prone path would be to enable tracing of class loading/unloading and parse the output. This requires us to kick off a JVM process, and we have to deal with different options to enable the tracing, output formats etc. I would think such a test shouldn't be included in suites.All, since it may be sensitive to the test machine environment (i.e. VM vendor and version). Here's an example from running the upgrade tests with versions 10.7.1.1 and 10.8.1.2 (I did run GC explicitly before the JVM exited to be sure): $ grep "EmbeddedDriver" output.txt [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:release-jars/10.7.1.1/derby.jar] [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:trunk-netbeans/classes/] [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:release-jars/10.8.1.2/derby.jar] [Unloading class org.apache.derby.jdbc.EmbeddedDriver] Note that the "origin" of the class is not included in the unloading-message. I'm not sure if one would have to rely on counting of the events only, or if we can conclude based on our knowledge of the test (i.e. when/how classes are loaded/unloaded) and trace order. I expect that the filtering and parsing could be done in Java as a JUnit test. I have not experimented further.
          Hide
          Knut Anders Hatlen added a comment -

          I wrote this patch to run DriverManager.deregisterDriver() in the class loader that had loaded the driver. By instrumenting DriverManager, I've verified that the old drivers get deregistered when we're done with them. However, I still need to increase permgen from the default (64M) to get the upgrade tests to complete without OOME.

          Using the JVM flags Kristian mentioned, I see this trace:

          $ grep EmbeddedDriver out.txt
          [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.0.2.1/derby.jar]
          [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/trunk/jars/sane/derby.jar]
          [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.1.1.0/derby.jar]
          [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.1.2.1/derby.jar]
          [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.1.3.1/derby.jar]
          [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.2.1.6/derby.jar]
          [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.2.2.0/derby.jar]
          [Unloading class org.apache.derby.jdbc.EmbeddedDriver]
          [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.3.1.4/derby.jar]
          [Unloading class org.apache.derby.jdbc.EmbeddedDriver]
          [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.3.3.0/derby.jar]
          [Unloading class org.apache.derby.jdbc.EmbeddedDriver]
          [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.4.1.3/derby.jar]
          [Unloading class org.apache.derby.jdbc.EmbeddedDriver]
          [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.4.2.0/derby.jar]
          [Unloading class org.apache.derby.jdbc.EmbeddedDriver]
          [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.5.1.1/derby.jar]
          [Unloading class org.apache.derby.jdbc.EmbeddedDriver]
          [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.5.3.0/derby.jar]
          [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.6.1.0/derby.jar]
          [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.6.2.1/derby.jar]
          [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.7.1.1/derby.jar]
          [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.8.1.2/derby.jar]
          [Unloading class org.apache.derby.jdbc.EmbeddedDriver]

          Since it doesn't specify exactly which variant of the class that is unloaded, I'd have to guess, but it looks to me as if 10.0.x and 10.1.x don't get unloaded. 10.2.x, 10.3.x and 10.4.x do get unloaded. Then 10.5.x and 10.6.x don't get unloaded. Finally 10.7.1.1 is unloaded, whereas 10.8.1.2 isn't unloaded because the test completes so soon after the deregistration of the driver that the JVM doesn't have time to do a gc.

          Show
          Knut Anders Hatlen added a comment - I wrote this patch to run DriverManager.deregisterDriver() in the class loader that had loaded the driver. By instrumenting DriverManager, I've verified that the old drivers get deregistered when we're done with them. However, I still need to increase permgen from the default (64M) to get the upgrade tests to complete without OOME. Using the JVM flags Kristian mentioned, I see this trace: $ grep EmbeddedDriver out.txt [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.0.2.1/derby.jar] [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/trunk/jars/sane/derby.jar] [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.1.1.0/derby.jar] [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.1.2.1/derby.jar] [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.1.3.1/derby.jar] [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.2.1.6/derby.jar] [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.2.2.0/derby.jar] [Unloading class org.apache.derby.jdbc.EmbeddedDriver] [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.3.1.4/derby.jar] [Unloading class org.apache.derby.jdbc.EmbeddedDriver] [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.3.3.0/derby.jar] [Unloading class org.apache.derby.jdbc.EmbeddedDriver] [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.4.1.3/derby.jar] [Unloading class org.apache.derby.jdbc.EmbeddedDriver] [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.4.2.0/derby.jar] [Unloading class org.apache.derby.jdbc.EmbeddedDriver] [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.5.1.1/derby.jar] [Unloading class org.apache.derby.jdbc.EmbeddedDriver] [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.5.3.0/derby.jar] [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.6.1.0/derby.jar] [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.6.2.1/derby.jar] [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.7.1.1/derby.jar] [Loaded org.apache.derby.jdbc.EmbeddedDriver from file:/code/derby/oldreleases/10.8.1.2/derby.jar] [Unloading class org.apache.derby.jdbc.EmbeddedDriver] Since it doesn't specify exactly which variant of the class that is unloaded, I'd have to guess, but it looks to me as if 10.0.x and 10.1.x don't get unloaded. 10.2.x, 10.3.x and 10.4.x do get unloaded. Then 10.5.x and 10.6.x don't get unloaded. Finally 10.7.1.1 is unloaded, whereas 10.8.1.2 isn't unloaded because the test completes so soon after the deregistration of the driver that the JVM doesn't have time to do a gc.
          Hide
          Knut Anders Hatlen added a comment -

          Not sure about 10.0.x and 10.1.x, but DERBY-4895 could be why the classes are not unloaded in 10.5.x and 10.6.x. It matches the affected version numbers, and some of the classes that can't be unloaded seem to have a ThreadLocal as their gc root.

          Show
          Knut Anders Hatlen added a comment - Not sure about 10.0.x and 10.1.x, but DERBY-4895 could be why the classes are not unloaded in 10.5.x and 10.6.x. It matches the affected version numbers, and some of the classes that can't be unloaded seem to have a ThreadLocal as their gc root.
          Hide
          Knut Anders Hatlen added a comment -

          Even though the patch doesn't fix the problem completely, it looks like it is improving the situation. I managed to run the upgrade tests successfully with 80 MB permgen space with the patch. The upgrade tests failed when I tried with 100 MB permgen without the patch.

          Show
          Knut Anders Hatlen added a comment - Even though the patch doesn't fix the problem completely, it looks like it is improving the situation. I managed to run the upgrade tests successfully with 80 MB permgen space with the patch. The upgrade tests failed when I tried with 100 MB permgen without the patch.
          Hide
          Kathey Marsden added a comment -

          Was DERBY-4895 a regression?

          Show
          Kathey Marsden added a comment - Was DERBY-4895 a regression?
          Hide
          Knut Anders Hatlen added a comment -

          Yes, DERBY-4895 was a regression.

          Show
          Knut Anders Hatlen added a comment - Yes, DERBY-4895 was a regression.
          Hide
          Knut Anders Hatlen added a comment -

          Attaching a simplified patch (deregister-v2.diff). The v2 patch does not create a directory with the DriverUnloader class in the test directory. Instead, it creates a specialized class loader that reads it directly from the system classpath.

          Show
          Knut Anders Hatlen added a comment - Attaching a simplified patch (deregister-v2.diff). The v2 patch does not create a directory with the DriverUnloader class in the test directory. Instead, it creates a specialized class loader that reads it directly from the system classpath.
          Hide
          Knut Anders Hatlen added a comment -

          Committed revision 1145111.

          Show
          Knut Anders Hatlen added a comment - Committed revision 1145111.
          Hide
          Knut Anders Hatlen added a comment -

          Attaching a patch that works around DERBY-4895 by manually clearing the ThreadLocal that prevents garbage collection of the engine classes. I was able to run the upgrade tests successfully without setting MaxPermSize when this patch was applied.

          Show
          Knut Anders Hatlen added a comment - Attaching a patch that works around DERBY-4895 by manually clearing the ThreadLocal that prevents garbage collection of the engine classes. I was able to run the upgrade tests successfully without setting MaxPermSize when this patch was applied.
          Hide
          Kathey Marsden added a comment -

          I only took a brief look at the patch so I may have missed it there already. but think we should omit the deregister for versions that have the DERBY-2905 fix where the deregister should happen automatically.

          Show
          Kathey Marsden added a comment - I only took a brief look at the patch so I may have missed it there already. but think we should omit the deregister for versions that have the DERBY-2905 fix where the deregister should happen automatically.
          Hide
          Knut Anders Hatlen added a comment -

          We don't actually call deregisterDriver() on the versions that already deregister the driver as part of the shutdown, since DriverManager.getDrivers() will return an empty Enumeration for those versions. But you were perhaps thinking that we should skip the entire process of checking whether there are any drivers registered? Or perhaps have an assert to verify that the driver in fact has been deregistered?

          Show
          Knut Anders Hatlen added a comment - We don't actually call deregisterDriver() on the versions that already deregister the driver as part of the shutdown, since DriverManager.getDrivers() will return an empty Enumeration for those versions. But you were perhaps thinking that we should skip the entire process of checking whether there are any drivers registered? Or perhaps have an assert to verify that the driver in fact has been deregistered?
          Hide
          Kathey Marsden added a comment -

          I was thinking of skipping the process but an assert for those versions with the DERBY-2905 would be good too. I just in general want to make sure the workaround code put in for the old versions with the bugs doesn't obfuscate future regressions if they occur.

          Show
          Kathey Marsden added a comment - I was thinking of skipping the process but an assert for those versions with the DERBY-2905 would be good too. I just in general want to make sure the workaround code put in for the old versions with the bugs doesn't obfuscate future regressions if they occur.
          Hide
          Knut Anders Hatlen added a comment -

          Attaching deregister-check.diff, which checks that we only had to deregister the driver in versions known to be affected by DERBY-2905.

          Committed clear-threadlocal.diff with revision 1145552.
          Committed deregister-check.diff with revision 1145553.

          Show
          Knut Anders Hatlen added a comment - Attaching deregister-check.diff, which checks that we only had to deregister the driver in versions known to be affected by DERBY-2905 . Committed clear-threadlocal.diff with revision 1145552. Committed deregister-check.diff with revision 1145553.
          Hide
          Knut Anders Hatlen added a comment -

          I've verified (by adding a chatty finalizer to the URLClassLoader created in UpgradeClassLoader.createClassLoader()) that all old versions from 10.2 to 10.8 are garbage collected. I'm wondering if the problems we still see with 10.0 and 10.1 could be related to DERBY-23 (a memory leak fixed in 10.2).

          Show
          Knut Anders Hatlen added a comment - I've verified (by adding a chatty finalizer to the URLClassLoader created in UpgradeClassLoader.createClassLoader()) that all old versions from 10.2 to 10.8 are garbage collected. I'm wondering if the problems we still see with 10.0 and 10.1 could be related to DERBY-23 (a memory leak fixed in 10.2).
          Hide
          Knut Anders Hatlen added a comment -

          Attaching the patch clear-contextservice.diff, which works around DERBY-23 by clearing two fields in ContextService after the old engine has been shut down. I've verified that the 10.0.x and 10.1.x engines are garbage collected when this patch is applied.

          Show
          Knut Anders Hatlen added a comment - Attaching the patch clear-contextservice.diff, which works around DERBY-23 by clearing two fields in ContextService after the old engine has been shut down. I've verified that the 10.0.x and 10.1.x engines are garbage collected when this patch is applied.
          Hide
          Knut Anders Hatlen added a comment -

          Committed revision 1145973.

          Resolving the issue since all the old engines seem to get garbage collected in my environment now.

          Show
          Knut Anders Hatlen added a comment - Committed revision 1145973. Resolving the issue since all the old engines seem to get garbage collected in my environment now.

            People

            • Assignee:
              Knut Anders Hatlen
              Reporter:
              Knut Anders Hatlen
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development