Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: jackrabbit-core, test
    • Labels:
      None

      Description

      Create a unit test for the database auto-reconnect feature (JCR-940).

      1. DatabaseConnectionFailureTest.java
        7 kB
        Jukka Zitting
      2. jackrabbit-core.test-for-1400.patch
        19 kB
        Alexander Klimetschek

        Issue Links

          Activity

          Hide
          Alexander Klimetschek added a comment -

          I actually prefer the threaded solution now.

          > What about using Rutime.exec(String[] cmdarray) instead of Runtime.exec(String command).

          External process is no longer needed when derby server is started in new thread.

          As I am writing a test case for JCR-1428, I see the need for a better base class (eg. AbstractJackrabbitTest). It creates a temporary directory in and starts a repository in setUp() and cleans that up in tearDown(). The configuration should be fetched via a protected getConfig() method that returns a RepositoryConf().

          Show
          Alexander Klimetschek added a comment - I actually prefer the threaded solution now. > What about using Rutime.exec(String[] cmdarray) instead of Runtime.exec(String command). External process is no longer needed when derby server is started in new thread. As I am writing a test case for JCR-1428 , I see the need for a better base class (eg. AbstractJackrabbitTest). It creates a temporary directory in and starts a repository in setUp() and cleans that up in tearDown(). The configuration should be fetched via a protected getConfig() method that returns a RepositoryConf().
          Hide
          Thomas Mueller added a comment -

          > problem with spaces in paths

          What about using Rutime.exec(String[] cmdarray) instead of Runtime.exec(String command).

          Show
          Thomas Mueller added a comment - > problem with spaces in paths What about using Rutime.exec(String[] cmdarray) instead of Runtime.exec(String command).
          Hide
          Alexander Klimetschek added a comment -

          Agreed, if there is no difference in the exception thrown and the handling of that exception, the simpler case is to be preferred.

          My motivation for harsh tests comes from reading the excellent "Release It!" book by Michael Nygard which talks about making systems stable in production (he gives a lot of Java examples and typical pitfalls). One of his examples clarifies that sometimes it does matter to look at the details of the connection loss - eg. if a TCP server does not answer the SYN with an ACK (because of some new firewall setting or whatever), the JDBC connection would hang forever, which is worse than actually failing. But I think I am way off-topic now

          Show
          Alexander Klimetschek added a comment - Agreed, if there is no difference in the exception thrown and the handling of that exception, the simpler case is to be preferred. My motivation for harsh tests comes from reading the excellent "Release It!" book by Michael Nygard which talks about making systems stable in production (he gives a lot of Java examples and typical pitfalls). One of his examples clarifies that sometimes it does matter to look at the details of the connection loss - eg. if a TCP server does not answer the SYN with an ACK (because of some new firewall setting or whatever), the JDBC connection would hang forever, which is worse than actually failing. But I think I am way off-topic now
          Hide
          Jukka Zitting added a comment -

          > Not running under Windows

          It's actually not an issue about Windows as such, but a problem with spaces in paths. I have my m2 repository in C:\Users\Jukka Zitting\.m2\repository, so the cmd string breaks. Adding explicit quotes around the classpath fixed the problem for me, but I still think the solution is quite fragile.

          > external process vs. threaded

          I see your point about a "harder connection loss", but from the perspective of our code the result is just the same: a JDBC call throws an exception. I don't see how the manner in which the connection is lost could affect the behaviour of our code, and so I prefer that we use simplest possible way to introduce such a scenario.

          Show
          Jukka Zitting added a comment - > Not running under Windows It's actually not an issue about Windows as such, but a problem with spaces in paths. I have my m2 repository in C:\Users\Jukka Zitting\.m2\repository, so the cmd string breaks. Adding explicit quotes around the classpath fixed the problem for me, but I still think the solution is quite fragile. > external process vs. threaded I see your point about a "harder connection loss", but from the perspective of our code the result is just the same: a JDBC call throws an exception. I don't see how the manner in which the connection is lost could affect the behaviour of our code, and so I prefer that we use simplest possible way to introduce such a scenario.
          Hide
          Alexander Klimetschek added a comment -

          Hi Jukka, some justifications from my side (we developers always have to do that, don't we ):

          • False alarm: Yupp, I needed to understand that as well. Actually it's those trial loops that do the reconnect.
          • autoReconnect: I wrote the test before backporting and ran it against the 1.3.3 code and it failed. After backporting it worked. That is the reason why I spent some time writing this test: I have no experience on the code I had to backport, because I haven't wrote any of it, so I wanted to make sure there is a proper automatic test for it.
          • Not running under windows: Any clues? Is it my external process that does not start? Or the derby network server? If it is the first, it could be that he doesn't find the "java" executable because it's not in the path.
          • Complexity (external process vs. threaded):
            a) I wanted to actually kill the process to simulate a harder connection loss.
            b) And I thought one could re-use this external process setup for other performance and stability tests.

          Your threaded version does call "shutdown" on the network server, which allows for a normal connection termination. Maybe one could kill the Thread, don't know if that works the same as process killing. We could test both scenarios, normal connection shutdown and kill, we don't know the internals of the derby client driver and how he behaves in both situations. Otherwise it's true, your variant is slimmer.

          Show
          Alexander Klimetschek added a comment - Hi Jukka, some justifications from my side (we developers always have to do that, don't we ): False alarm: Yupp, I needed to understand that as well. Actually it's those trial loops that do the reconnect. autoReconnect: I wrote the test before backporting and ran it against the 1.3.3 code and it failed. After backporting it worked. That is the reason why I spent some time writing this test: I have no experience on the code I had to backport, because I haven't wrote any of it, so I wanted to make sure there is a proper automatic test for it. Not running under windows: Any clues? Is it my external process that does not start? Or the derby network server? If it is the first, it could be that he doesn't find the "java" executable because it's not in the path. Complexity (external process vs. threaded): a) I wanted to actually kill the process to simulate a harder connection loss. b) And I thought one could re-use this external process setup for other performance and stability tests. Your threaded version does call "shutdown" on the network server, which allows for a normal connection termination. Maybe one could kill the Thread, don't know if that works the same as process killing. We could test both scenarios, normal connection shutdown and kill, we don't know the internals of the derby client driver and how he behaves in both situations. Otherwise it's true, your variant is slimmer.
          Hide
          Jukka Zitting added a comment -

          Sorry, false alarm. It's actually the BundleDbPersistenceManager class that has the extra reconnect logic that I was wondering about. How convoluted!

          Show
          Jukka Zitting added a comment - Sorry, false alarm. It's actually the BundleDbPersistenceManager class that has the extra reconnect logic that I was wondering about. How convoluted!
          Hide
          Jukka Zitting added a comment -

          Another issue, I can't get the test (either your or my version) to fail even if I disable the autoReconnect feature in ConnectionRecoveryManager. It seems like the Derby client already has some automatic recovery mechanism built-in, so unless we find a way to disable that this test doesn't really prove anything.

          Show
          Jukka Zitting added a comment - Another issue, I can't get the test (either your or my version) to fail even if I disable the autoReconnect feature in ConnectionRecoveryManager. It seems like the Derby client already has some automatic recovery mechanism built-in, so unless we find a way to disable that this test doesn't really prove anything.
          Hide
          Jukka Zitting added a comment -

          I think you're being way too complex with this test case.

          See the attached version of DatabaseConnectionFailureTest.java for a version that uses the Derby NetworkServerControl class to control a Derby network server as a separate thread within the same process.

          Show
          Jukka Zitting added a comment - I think you're being way too complex with this test case. See the attached version of DatabaseConnectionFailureTest.java for a version that uses the Derby NetworkServerControl class to control a Derby network server as a separate thread within the same process.
          Hide
          Jukka Zitting added a comment -

          Re-targeted this issue for Jackrabbit trunk.

          Show
          Jukka Zitting added a comment - Re-targeted this issue for Jackrabbit trunk.
          Hide
          Alexander Klimetschek added a comment -

          Fixed the problem, now runs under mvn test. The trick was to get the JAR file path via the classes itself:

          /**

          • Returns the path to the JAR file that a certain class is located in. This only works
          • if the classloader loaded this class from a JAR file.
            */
            public static final String getJarFileForClass(Class clazz) { // eg. /org/apache/derby/drda/NetworkServerControl.class String classResource = "/" + clazz.getCanonicalName().replace(".", "/") + ".class"; // eg. jar:file:/Users/alex/.m2/repository/org/apache/derby/derbynet/10.2.1.6/derbynet-10.2.1.6.jar!/org/apache/derby/drda/NetworkServerControl.class String fullResourceURL = clazz.getResource(classResource).toString(); // eg. /Users/alex/.m2/repository/org/apache/derby/derbynet/10.2.1.6/derbynet-10.2.1.6.jar return fullResourceURL.replaceFirst("jar:file:([^!]+).*", "$1"); }

          See o.a.j.core.persistence.ext.ExternalDerbyProcess

          Show
          Alexander Klimetschek added a comment - Fixed the problem, now runs under mvn test. The trick was to get the JAR file path via the classes itself: /** Returns the path to the JAR file that a certain class is located in. This only works if the classloader loaded this class from a JAR file. */ public static final String getJarFileForClass(Class clazz) { // eg. /org/apache/derby/drda/NetworkServerControl.class String classResource = "/" + clazz.getCanonicalName().replace(".", "/") + ".class"; // eg. jar:file:/Users/alex/.m2/repository/org/apache/derby/derbynet/10.2.1.6/derbynet-10.2.1.6.jar!/org/apache/derby/drda/NetworkServerControl.class String fullResourceURL = clazz.getResource(classResource).toString(); // eg. /Users/alex/.m2/repository/org/apache/derby/derbynet/10.2.1.6/derbynet-10.2.1.6.jar return fullResourceURL.replaceFirst("jar:file:([^!]+).*", "$1"); } See o.a.j.core.persistence.ext.ExternalDerbyProcess
          Hide
          Alexander Klimetschek added a comment -

          Patch that tests with an external derby server. This server is started as a Java process with the derby, derbynet and derbyclient libraries in network server mode. Therefore I added derbynet and derbyclient as test dependencies to the jackrabbit-core POM as well as specified their versions in the root POM's dependencyManagement section.

          Test failed before backporting JCR-940 and works fine after it.

          Makes use of JCR-1412 for configuring jackrabbit without a repository.xml.

          This test (o.a.j.core.persistence.DatabaseConnectionFailureTest) currently does not work with "mvn test", only inside Eclipse. Therefore I disabled it in o.a.j.core.persistence.TestAll. The problem is that the external derby server needs a classpath with derbynet and derbyclient, which does not work with the way the maven-surefire-plugin sets the classpath. I am waiting for some help from the maven users mailing list:

          http://markmail.org/search/?q=surefire+list%3Aorg.apache.maven.users#query:surefire%20list%3Aorg.apache.maven.users%20order%3Adate-backward+page:1+mid:u5js4xu7a3bag6b3+state:results

          Show
          Alexander Klimetschek added a comment - Patch that tests with an external derby server. This server is started as a Java process with the derby, derbynet and derbyclient libraries in network server mode. Therefore I added derbynet and derbyclient as test dependencies to the jackrabbit-core POM as well as specified their versions in the root POM's dependencyManagement section. Test failed before backporting JCR-940 and works fine after it. Makes use of JCR-1412 for configuring jackrabbit without a repository.xml. This test (o.a.j.core.persistence.DatabaseConnectionFailureTest) currently does not work with "mvn test", only inside Eclipse. Therefore I disabled it in o.a.j.core.persistence.TestAll. The problem is that the external derby server needs a classpath with derbynet and derbyclient, which does not work with the way the maven-surefire-plugin sets the classpath. I am waiting for some help from the maven users mailing list: http://markmail.org/search/?q=surefire+list%3Aorg.apache.maven.users#query:surefire%20list%3Aorg.apache.maven.users%20order%3Adate-backward+page:1+mid:u5js4xu7a3bag6b3+state:results

            People

            • Assignee:
              Jukka Zitting
              Reporter:
              Alexander Klimetschek
            • Votes:
              1 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:

                Development