Derby
  1. Derby
  2. DERBY-4314

With derby client setTransactionIsolation executes and commits even if isolation has not changed

    Details

    • Issue & fix info:
      Newcomer, Patch Available
    • Bug behavior facts:
      Embedded/Client difference

      Description

      With in EmbedConnection.setIsolation() we have a check to see if the isolation level is the same and if so just return without doing a commit:
      public void setTransactionIsolation(int level) throws SQLException {

      if (level == getTransactionIsolation())
      return;

      with org.apache.derby.client.am.Connection we have no such check. It would be good if the client driver acted like embedded.

      1. DERBY-4314.diff
        1 kB
        Lily Wei
      2. DERBY-4314-2.diff
        4 kB
        Lily Wei
      3. DERBY-4314-3.diff
        4 kB
        Lily Wei
      4. DERBY-4314-5.diff
        4 kB
        Lily Wei
      5. derby-4314-6a-initial_piggybacking.diff
        8 kB
        Kristian Waagan
      6. derby-4314-6a-initial_piggybacking.stat
        0.2 kB
        Kristian Waagan
      7. DERBY-4314-6b-combinepiggybacking.diff
        11 kB
        Lily Wei
      8. DERBY-4314-6c-combineaftermerge.diff
        11 kB
        Lily Wei
      9. derby-4314-6d-handle_xa.diff
        15 kB
        Kristian Waagan
      10. DERBY-4314-7b-combine.diff
        16 kB
        Lily Wei
      11. DERBY-4314-7-withoutpiggybacking.diff
        5 kB
        Lily Wei
      12. releaseNote.html
        4 kB
        Lily Wei
      13. releaseNote.html
        4 kB
        Lily Wei
      14. ReproIsoLost.java
        4 kB
        Kathey Marsden
      15. TestConnReuse.java
        2 kB
        Lily Wei
      16. utilXid.java
        2 kB
        Kathey Marsden

        Issue Links

          Activity

          Hide
          Lily Wei added a comment -

          This is the diff from trunk for the patch.

          Show
          Lily Wei added a comment - This is the diff from trunk for the patch.
          Hide
          Lily Wei added a comment -

          The patch is to change org.apache.derby.client.am.Connection according to the bug description. When running suites.All, SetTransactionIsolationTest test failed. I think the failure was due to the old behavior of DerbyNetClient. I change the test to reflect the new behavior. I did not take out the logic for the old behavior. If I need to, I will be happy to do that. The changes is made to trunk. I am running derbyall tests now.

          Show
          Lily Wei added a comment - The patch is to change org.apache.derby.client.am.Connection according to the bug description. When running suites.All, SetTransactionIsolationTest test failed. I think the failure was due to the old behavior of DerbyNetClient. I change the test to reflect the new behavior. I did not take out the logic for the old behavior. If I need to, I will be happy to do that. The changes is made to trunk. I am running derbyall tests now.
          Hide
          Kathey Marsden added a comment -

          Thanks Lily for looking at this issue. Here are some comments.

          • I think it would be good to put the check after the trace call so the call will still get logged if tracing is on.
          • In the test, I guess we can just remove the whole switch statment passCommitCheck etc. and just have.
            assertEquals(1,count); since the behavior is the same for the drivers. The comment in the beginning of the test is no longer relevant and maybe the fixture name should be changed to testSetTransactionIsolationDoesNotCommit() or some such.
          • I guess there was already an issue for this DERBY-2064 which should be duped to this one, since you are posting your work here.
          • I think calling getTransactionIsolation() will introduce another server round trip to get the isolation if it is not cached. Under what circumstances will the isolation be cached or not cached? Is this a performance issue or does it happen rarely?
          • Also calling getTransactionIsolation() directly will cause the getTransactionIsolation call to get traced which it probably shouldn't. We might need to make a getTransactionIsolationX() which does the work but doesn't have the tracing call. and then have the X method called by getTransasctionIsolation() and setTransactionIsolation()
          Show
          Kathey Marsden added a comment - Thanks Lily for looking at this issue. Here are some comments. I think it would be good to put the check after the trace call so the call will still get logged if tracing is on. In the test, I guess we can just remove the whole switch statment passCommitCheck etc. and just have. assertEquals(1,count); since the behavior is the same for the drivers. The comment in the beginning of the test is no longer relevant and maybe the fixture name should be changed to testSetTransactionIsolationDoesNotCommit() or some such. I guess there was already an issue for this DERBY-2064 which should be duped to this one, since you are posting your work here. I think calling getTransactionIsolation() will introduce another server round trip to get the isolation if it is not cached. Under what circumstances will the isolation be cached or not cached? Is this a performance issue or does it happen rarely? Also calling getTransactionIsolation() directly will cause the getTransactionIsolation call to get traced which it probably shouldn't. We might need to make a getTransactionIsolationX() which does the work but doesn't have the tracing call. and then have the X method called by getTransasctionIsolation() and setTransactionIsolation()
          Hide
          Lily Wei added a comment -

          Thanks Kathey for looking at the patch. I change the check to be after trace. I will mark DERBY-2064 as dup of this bug. I think getTransactionIsolation() will introduce server round trip. I am not sure the performance impact will be huge. I add a check for supportsSessionDataCaching() to prevent introduce server round trip if it is not cache. getTransactionIsolationX()is introduce and setTransactionIsolation() and getTransactionIsolation() will be calling it. testSetTransactionIsolationCommitRollback will be checking for assertEquals(1, count) thanks to Kathey's great points. I run suits.all again and all the tests passed. I am running derbyall now.

          Show
          Lily Wei added a comment - Thanks Kathey for looking at the patch. I change the check to be after trace. I will mark DERBY-2064 as dup of this bug. I think getTransactionIsolation() will introduce server round trip. I am not sure the performance impact will be huge. I add a check for supportsSessionDataCaching() to prevent introduce server round trip if it is not cache. getTransactionIsolationX()is introduce and setTransactionIsolation() and getTransactionIsolation() will be calling it. testSetTransactionIsolationCommitRollback will be checking for assertEquals(1, count) thanks to Kathey's great points. I run suits.all again and all the tests passed. I am running derbyall now.
          Hide
          Kristian Waagan added a comment -

          FYI, there are some related comments (and code) in Connection.completeReset.

          Show
          Kristian Waagan added a comment - FYI, there are some related comments (and code) in Connection.completeReset.
          Hide
          Kathey Marsden added a comment - - edited

          For
          if (supportsSessionDataCaching() && level == getTransactionIsolationX())
          return;

          I think we need to remove supportsSessionDataCaching() because we don't want setTrasactionIsolation to behave differently and always commit if the server does not support session data caching.

          It seems that _isolation is set to TRANSACTION_UNKNOWN for new connections. This means we will always make an extra round trip for the first setTransactionIsolation call on a connection. Can we initialize _isolation to READ_COMMITTED when session data caching is supported instead since that is the default for new connections?

          Show
          Kathey Marsden added a comment - - edited For if (supportsSessionDataCaching() && level == getTransactionIsolationX()) return; I think we need to remove supportsSessionDataCaching() because we don't want setTrasactionIsolation to behave differently and always commit if the server does not support session data caching. It seems that _isolation is set to TRANSACTION_UNKNOWN for new connections. This means we will always make an extra round trip for the first setTransactionIsolation call on a connection. Can we initialize _isolation to READ_COMMITTED when session data caching is supported instead since that is the default for new connections?
          Hide
          Lily Wei added a comment -

          After chatting with Kathey and Kristian, I made this new patch. I run suites.all and derbyall is running now. All tests passed on suites.all with this patch change.

          When debugging with the existing tests, I do see we do round trip for the first setTransactionIsolation call on a connection. However, I am not totally sure it justifies setting _isolation to TRANSACTION_UNKNOWN. Kristian was telling me client driver will ask the server about isolation level if it is unknown. This might be avoidable if piggybacking is available.

          Show
          Lily Wei added a comment - After chatting with Kathey and Kristian, I made this new patch. I run suites.all and derbyall is running now. All tests passed on suites.all with this patch change. When debugging with the existing tests, I do see we do round trip for the first setTransactionIsolation call on a connection. However, I am not totally sure it justifies setting _isolation to TRANSACTION_UNKNOWN. Kristian was telling me client driver will ask the server about isolation level if it is unknown. This might be avoidable if piggybacking is available.
          Hide
          Kristian Waagan added a comment -

          I found some useful information about the piggybacking here: http://wiki.apache.org/db-derby/Derby3192Writeup
          Thank you, Dyre!

          Kathey wrote:


          It seems that _isolation is set to TRANSACTION_UNKNOWN for new connections. This means we will always make an extra round trip for the first setTransactionIsolation call on a connection. Can we initialize _isolation to READ_COMMITTED when session data caching is supported instead since that is the default for new connections?


          I'm slightly skeptical about this suggested change:
          1) It seems we have had bugs regarding assumptions about and caching of the isolation level earlier.
          2) What happens if we in the future add the option to configure the default isolation level on the server?
          It seems I have already introduced an assumption in the client code that may break (hard-coded assumption that the default level is RC, limited to pooled/XA connections - see Connection.completeReset).
          3) This happens once for each connection if setTransactionIsolation is called before any other round trip to the server, assuming session state caching is available. Shouldn't applications creating loads of connections use a connection pool?

          It seems to me that the client should be informed about the default isolation level as part of the connection initialization sequence. The default value could be stored, and used later on connection resets etc. I don't know if the DRDA specification allows this somehow. One solution could be the product specific code points.

          The latest patch looks good to me. Nitpicks:

          • mixed tabs/spaces indentation at @@ -909,6 +909,9 @@
          • mixed tabs/spaces indentation at @@ -1005,8 +1008,21 @@
          • traceExit is used instead of traceEntry. Is this intended?
            (I see this is also done in some other places in the client code)
          Show
          Kristian Waagan added a comment - I found some useful information about the piggybacking here: http://wiki.apache.org/db-derby/Derby3192Writeup Thank you, Dyre! Kathey wrote: It seems that _isolation is set to TRANSACTION_UNKNOWN for new connections. This means we will always make an extra round trip for the first setTransactionIsolation call on a connection. Can we initialize _isolation to READ_COMMITTED when session data caching is supported instead since that is the default for new connections? I'm slightly skeptical about this suggested change: 1) It seems we have had bugs regarding assumptions about and caching of the isolation level earlier. 2) What happens if we in the future add the option to configure the default isolation level on the server? It seems I have already introduced an assumption in the client code that may break (hard-coded assumption that the default level is RC, limited to pooled/XA connections - see Connection.completeReset). 3) This happens once for each connection if setTransactionIsolation is called before any other round trip to the server, assuming session state caching is available. Shouldn't applications creating loads of connections use a connection pool? It seems to me that the client should be informed about the default isolation level as part of the connection initialization sequence. The default value could be stored, and used later on connection resets etc. I don't know if the DRDA specification allows this somehow. One solution could be the product specific code points. The latest patch looks good to me. Nitpicks: mixed tabs/spaces indentation at @@ -909,6 +909,9 @@ mixed tabs/spaces indentation at @@ -1005,8 +1008,21 @@ traceExit is used instead of traceEntry. Is this intended? (I see this is also done in some other places in the client code)
          Hide
          Lily Wei added a comment -

          This patch fixs

          • mixed tabs/spaces indentation at @@ -909,6 +909,9 @@
          • mixed tabs/spaces indentation at @@ -1005,8 +1008,21 @@
          • use traceEntry instead of traceExit

          If it will be better for performance, I will be better to change the _isolation to READ_COMMITTED. However, we have to handle the assumption in the code and piggybacking situation. Is this the best place to change the current assumption?

          Show
          Lily Wei added a comment - This patch fixs mixed tabs/spaces indentation at @@ -909,6 +909,9 @@ mixed tabs/spaces indentation at @@ -1005,8 +1008,21 @@ use traceEntry instead of traceExit If it will be better for performance, I will be better to change the _isolation to READ_COMMITTED. However, we have to handle the assumption in the code and piggybacking situation. Is this the best place to change the current assumption?
          Hide
          Lily Wei added a comment -

          Run Suites.all and derbyall against trunk 799169. No failure.

          Show
          Lily Wei added a comment - Run Suites.all and derbyall against trunk 799169. No failure.
          Hide
          Kathey Marsden added a comment -

          I posted a program to DERBY-4343 which would be interesting to run with this patch with tracing on to understand the server round trips in various scenarios. Of course the assertion failure would have to be fixed first. I am curious though if this patch might actually fix that bug.

          Show
          Kathey Marsden added a comment - I posted a program to DERBY-4343 which would be interesting to run with this patch with tracing on to understand the server round trips in various scenarios. Of course the assertion failure would have to be fixed first. I am curious though if this patch might actually fix that bug.
          Hide
          Kristian Waagan added a comment -

          Attaching patch 6a, which demonstrates the piggy-backing of session data on connection initialization.

          Comments on the approach/patch is welcome.
          Note the following:

          • I inserted the PBSD (session data code point(s)) at the end of the ACCRDB reply. Is this allowed by the DRDA standard?
            It will only be sent if the client is Derby and the version is 10.6 or greater.
          • I have not yet implemented the change suggested, where the isolation level is initialized to READ_COMMITTED unconditionally in the client.
          • The extra completeInitialPiggyBackSchema method was added because the debug assert fails with NPE (the metadata object hasn't been created yet, this happens in completeConnection).
          • My test run hasn't completed yet.
          Show
          Kristian Waagan added a comment - Attaching patch 6a, which demonstrates the piggy-backing of session data on connection initialization. Comments on the approach/patch is welcome. Note the following: I inserted the PBSD (session data code point(s)) at the end of the ACCRDB reply. Is this allowed by the DRDA standard? It will only be sent if the client is Derby and the version is 10.6 or greater. I have not yet implemented the change suggested, where the isolation level is initialized to READ_COMMITTED unconditionally in the client. The extra completeInitialPiggyBackSchema method was added because the debug assert fails with NPE (the metadata object hasn't been created yet, this happens in completeConnection). My test run hasn't completed yet.
          Hide
          Lily Wei added a comment -

          Thanks Kristian. I apply the patch. I don't know about your question to DRDA standard.
          I run it against the repro from DERBY-4343 TestConnReuser.java. I got the ASSERT failure with defaultisolation change. This is my output from running TestConnReuse.java
          $ java TestConnReuse
          FirstConnection
          count = 22
          Seccond Connection. Reuse Connection
          Third Connection. Set same isolation
          Exception in thread "main" org.apache.derby.shared.common.sanity.AssertFailure:
          ASSERT FAILED
          at org.apache.derby.shared.common.sanity.SanityManager.ASSERT(SanityMana
          ger.java:98)
          at org.apache.derby.client.am.Connection.setTransactionIsolationX(Connec
          tion.java:987)
          at org.apache.derby.client.am.Connection.setTransactionIsolation(Connect
          ion.java:915)
          at org.apache.derby.client.am.LogicalConnection.setTransactionIsolation(
          LogicalConnection.java:253)
          at TestConnReuse.main(TestConnReuse.java:33)
          ---------------
          Stack traces for all live threads:
          Thread name=Finalizer id=3 priority=8 state=WAITING isdaemon=true
          java.lang.Object.wait(Native Method)
          java.lang.ref.ReferenceQueue.remove(Unknown Source)
          java.lang.ref.ReferenceQueue.remove(Unknown Source)
          java.lang.ref.Finalizer$FinalizerThread.run(Unknown Source)

          Thread name=main id=1 priority=5 state=RUNNABLE isdaemon=false
          java.lang.Thread.dumpThreads(Native Method)
          java.lang.Thread.getAllStackTraces(Unknown Source)
          org.apache.derby.shared.common.sanity.ThreadDump.getStackDumpString(Thre
          adDump.java:34)
          sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
          sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
          sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
          java.lang.reflect.Method.invoke(Unknown Source)
          org.apache.derby.shared.common.sanity.AssertFailure$1.run(AssertFailure.
          java:165)

          Do you think I am missing something?

          Show
          Lily Wei added a comment - Thanks Kristian. I apply the patch. I don't know about your question to DRDA standard. I run it against the repro from DERBY-4343 TestConnReuser.java. I got the ASSERT failure with defaultisolation change. This is my output from running TestConnReuse.java $ java TestConnReuse FirstConnection count = 22 Seccond Connection. Reuse Connection Third Connection. Set same isolation Exception in thread "main" org.apache.derby.shared.common.sanity.AssertFailure: ASSERT FAILED at org.apache.derby.shared.common.sanity.SanityManager.ASSERT(SanityMana ger.java:98) at org.apache.derby.client.am.Connection.setTransactionIsolationX(Connec tion.java:987) at org.apache.derby.client.am.Connection.setTransactionIsolation(Connect ion.java:915) at org.apache.derby.client.am.LogicalConnection.setTransactionIsolation( LogicalConnection.java:253) at TestConnReuse.main(TestConnReuse.java:33) --------------- Stack traces for all live threads: Thread name=Finalizer id=3 priority=8 state=WAITING isdaemon=true java.lang.Object.wait(Native Method) java.lang.ref.ReferenceQueue.remove(Unknown Source) java.lang.ref.ReferenceQueue.remove(Unknown Source) java.lang.ref.Finalizer$FinalizerThread.run(Unknown Source) Thread name=main id=1 priority=5 state=RUNNABLE isdaemon=false java.lang.Thread.dumpThreads(Native Method) java.lang.Thread.getAllStackTraces(Unknown Source) org.apache.derby.shared.common.sanity.ThreadDump.getStackDumpString(Thre adDump.java:34) sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) java.lang.reflect.Method.invoke(Unknown Source) org.apache.derby.shared.common.sanity.AssertFailure$1.run(AssertFailure. java:165) Do you think I am missing something?
          Hide
          Lily Wei added a comment -

          After careful review the code, I think there might be some issue relate to DRDA protocal with the patch. When running suites.all test suite, testSetIsolationWithStatement failed with the following assertion:
          There was 1 error:
          1) testSetIsolationWithStatement(org.apache.derbyTesting.functionTests.tests.jdb
          capi.J2EEDataSourceTest)org.apache.derby.client.am.XaException: XAER_DUPID : Err
          or executing a XAResource.start(), server returned XAER_DUPID.
          at org.apache.derby.client.net.NetXAResource.throwXAException(NetXAResou
          rce.java:756)
          at org.apache.derby.client.net.NetXAResource.start(NetXAResource.java:64
          7)
          at org.apache.derbyTesting.functionTests.tests.jdbcapi.J2EEDataSourceTes
          t.testSetIsolationWithStatement(J2EEDataSourceTest.java:1623)
          at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
          at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
          at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
          at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:
          109)

          Looking at NetXAResource.start with Xid xid2 = new cdsXid(1, (byte) 93, (byte) 103);
          xar.start(xid2, XAResource.TMNOFLAGS); on J2EEDataSourceTest, I am thinking it might be relate to change of behavior of DRDA protocal. I need to further investigate on this.

          With repro on Derby-4343 (Please refer to second attachment file TestConnReuse.java), if the third connection does not set the transaction isolation level to Connection.TRANSACTION_READ_COMMITTED but Connection.TRANSACTION_READ_UNCOMMITTED, we can get third connection without assertion.

          This is the assertion trace if the third connection set the transcationIsolation to Connection.TRANSACTION_READ_COMMITTED.
          Exception in thread "main" org.apache.derby.shared.common.sanity.AssertFailure:
          ASSERT FAILED
          at org.apache.derby.shared.common.sanity.SanityManager.ASSERT(SanityMana
          ger.java:98)
          at org.apache.derby.client.am.Connection.setTransactionIsolationX(Connec
          tion.java:987)
          at org.apache.derby.client.am.Connection.setTransactionIsolation(Connect
          ion.java:915)
          at org.apache.derby.client.am.LogicalConnection.setTransactionIsolation(
          LogicalConnection.java:253)
          at TestConnReuse.main(TestConnReuse.java:33)

          Any guideline on the cause of reason is highly appreciated.

          Show
          Lily Wei added a comment - After careful review the code, I think there might be some issue relate to DRDA protocal with the patch. When running suites.all test suite, testSetIsolationWithStatement failed with the following assertion: There was 1 error: 1) testSetIsolationWithStatement(org.apache.derbyTesting.functionTests.tests.jdb capi.J2EEDataSourceTest)org.apache.derby.client.am.XaException: XAER_DUPID : Err or executing a XAResource.start(), server returned XAER_DUPID. at org.apache.derby.client.net.NetXAResource.throwXAException(NetXAResou rce.java:756) at org.apache.derby.client.net.NetXAResource.start(NetXAResource.java:64 7) at org.apache.derbyTesting.functionTests.tests.jdbcapi.J2EEDataSourceTes t.testSetIsolationWithStatement(J2EEDataSourceTest.java:1623) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java: 109) Looking at NetXAResource.start with Xid xid2 = new cdsXid(1, (byte) 93, (byte) 103); xar.start(xid2, XAResource.TMNOFLAGS); on J2EEDataSourceTest, I am thinking it might be relate to change of behavior of DRDA protocal. I need to further investigate on this. With repro on Derby-4343 (Please refer to second attachment file TestConnReuse.java), if the third connection does not set the transaction isolation level to Connection.TRANSACTION_READ_COMMITTED but Connection.TRANSACTION_READ_UNCOMMITTED, we can get third connection without assertion. This is the assertion trace if the third connection set the transcationIsolation to Connection.TRANSACTION_READ_COMMITTED. Exception in thread "main" org.apache.derby.shared.common.sanity.AssertFailure: ASSERT FAILED at org.apache.derby.shared.common.sanity.SanityManager.ASSERT(SanityMana ger.java:98) at org.apache.derby.client.am.Connection.setTransactionIsolationX(Connec tion.java:987) at org.apache.derby.client.am.Connection.setTransactionIsolation(Connect ion.java:915) at org.apache.derby.client.am.LogicalConnection.setTransactionIsolation( LogicalConnection.java:253) at TestConnReuse.main(TestConnReuse.java:33) Any guideline on the cause of reason is highly appreciated.
          Hide
          Lily Wei added a comment -

          Hi Kristian or Knut:
          Would you mind provide any information to 6b patch which has Kristian's piggy backing change of session data change cause TestConnReuse to give exception on Connection.setTransactionIsolationX at the first connection when the isolation level is TRANSACTION_READ_COMMITTED? Thanks, Lily

          Show
          Lily Wei added a comment - Hi Kristian or Knut: Would you mind provide any information to 6b patch which has Kristian's piggy backing change of session data change cause TestConnReuse to give exception on Connection.setTransactionIsolationX at the first connection when the isolation level is TRANSACTION_READ_COMMITTED? Thanks, Lily
          Hide
          Knut Anders Hatlen added a comment -

          Hi Lily,

          I'm afraid I'm not able to reproduce the assert failure when I apply the 6b patch on head of trunk and run TestConnReuse. I tried to change the isolation levels as described above, but I may have got it wrong. Perhaps you could attach an updated version of TestConnReuse.java that shows the problem with no modification? Thanks.

          Show
          Knut Anders Hatlen added a comment - Hi Lily, I'm afraid I'm not able to reproduce the assert failure when I apply the 6b patch on head of trunk and run TestConnReuse. I tried to change the isolation levels as described above, but I may have got it wrong. Perhaps you could attach an updated version of TestConnReuse.java that shows the problem with no modification? Thanks.
          Hide
          Lily Wei added a comment -

          Thank you, Knut. You are right. The repro is passing with 6b. I did not merge right after Connection.java has newer version. However, I am still seeing J2EEDataSourceTest exception from DERBY-1325 repro. I am seeing exception as following:
          1) testSetIsolationWithStatement(org.apache.derbyTesting.functionTests.tests.jdb
          capi.J2EEDataSourceTest)org.apache.derby.client.am.XaException: XAER_DUPID : Err
          or executing a XAResource.start(), server returned XAER_DUPID.
          at org.apache.derby.client.net.NetXAResource.throwXAException(NetXAResou
          rce.java:756)
          at org.apache.derby.client.net.NetXAResource.start(NetXAResource.java:64
          7)
          at org.apache.derbyTesting.functionTests.tests.jdbcapi.J2EEDataSourceTes
          t.testSetIsolationWithStatement(J2EEDataSourceTest.java:1623)
          at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
          at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
          at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
          at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:
          109)

          I am thinking this is related to piggy backing of session data. However, I am all open to new suggestion.

          Show
          Lily Wei added a comment - Thank you, Knut. You are right. The repro is passing with 6b. I did not merge right after Connection.java has newer version. However, I am still seeing J2EEDataSourceTest exception from DERBY-1325 repro. I am seeing exception as following: 1) testSetIsolationWithStatement(org.apache.derbyTesting.functionTests.tests.jdb capi.J2EEDataSourceTest)org.apache.derby.client.am.XaException: XAER_DUPID : Err or executing a XAResource.start(), server returned XAER_DUPID. at org.apache.derby.client.net.NetXAResource.throwXAException(NetXAResou rce.java:756) at org.apache.derby.client.net.NetXAResource.start(NetXAResource.java:64 7) at org.apache.derbyTesting.functionTests.tests.jdbcapi.J2EEDataSourceTes t.testSetIsolationWithStatement(J2EEDataSourceTest.java:1623) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java: 109) I am thinking this is related to piggy backing of session data. However, I am all open to new suggestion.
          Hide
          Lily Wei added a comment -

          Post the diff after merge Connection.java

          Show
          Lily Wei added a comment - Post the diff after merge Connection.java
          Hide
          Kathey Marsden added a comment -

          I think the more relevant failure is the first one that occurs:

          1) testGlobalLocalInterleaf(org.apache.derbyTesting.functionTests.tests.jdbcapi.
          J2EEDataSourceTest)junit.framework.AssertionFailedError: expected:<1> but was:<2
          >
          at org.apache.derbyTesting.functionTests.tests.jdbcapi.J2EEDataSourceTes
          t.assertConnectionState(J2EEDataSourceTest.java:3341)
          at org.apache.derbyTesting.functionTests.tests.jdbcapi.J2EEDataSourceTes
          t.testGlobalLocalInterleaf(J2EEDataSourceTest.java:1501)
          at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
          at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.
          java:48)
          at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAcces
          sorImpl.java:37)
          at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:
          109)
          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 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 org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57

          Around line 1406 we end a global transaction:
          assertConnectionState(
          ResultSet.CLOSE_CURSORS_AT_COMMIT,
          Connection.TRANSACTION_READ_UNCOMMITTED,
          false, ReadOnly, cs1);

          xar.end(xid, XAResource.TMSUCCESS);

          Then we get a new logical connection in between (with isolation READ_COMMITTED) and then

          about 1499 we join the global transaction
          xar.start(xid, XAResource.TMJOIN);
          cs1 = xac.getConnection();
          // re-join with new handle X1
          assertConnectionState(
          ResultSet.CLOSE_CURSORS_AT_COMMIT,
          Connection.TRANSACTION_READ_UNCOMMITTED,
          false, ReadOnly, cs1);

          But we get the error
          J2EEDataSourceTest)junit.framework.AssertionFailedError: expected:<1> but was:<2
          >
          So the isolation level was not restored to READ_UNCOMMITED when we joined. I haven't looked at the patch, but I think this is the issue that would be best to look at first. I think the DUPID error may be just a cascading failure that occurs because of this one.

          Show
          Kathey Marsden added a comment - I think the more relevant failure is the first one that occurs: 1) testGlobalLocalInterleaf(org.apache.derbyTesting.functionTests.tests.jdbcapi. J2EEDataSourceTest)junit.framework.AssertionFailedError: expected:<1> but was:<2 > at org.apache.derbyTesting.functionTests.tests.jdbcapi.J2EEDataSourceTes t.assertConnectionState(J2EEDataSourceTest.java:3341) at org.apache.derbyTesting.functionTests.tests.jdbcapi.J2EEDataSourceTes t.testGlobalLocalInterleaf(J2EEDataSourceTest.java:1501) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl. java:48) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAcces sorImpl.java:37) at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java: 109) 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 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 org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57 Around line 1406 we end a global transaction: assertConnectionState( ResultSet.CLOSE_CURSORS_AT_COMMIT, Connection.TRANSACTION_READ_UNCOMMITTED, false, ReadOnly, cs1); xar.end(xid, XAResource.TMSUCCESS); Then we get a new logical connection in between (with isolation READ_COMMITTED) and then about 1499 we join the global transaction xar.start(xid, XAResource.TMJOIN); cs1 = xac.getConnection(); // re-join with new handle X1 assertConnectionState( ResultSet.CLOSE_CURSORS_AT_COMMIT, Connection.TRANSACTION_READ_UNCOMMITTED, false, ReadOnly, cs1); But we get the error J2EEDataSourceTest)junit.framework.AssertionFailedError: expected:<1> but was:<2 > So the isolation level was not restored to READ_UNCOMMITED when we joined. I haven't looked at the patch, but I think this is the issue that would be best to look at first. I think the DUPID error may be just a cascading failure that occurs because of this one.
          Hide
          Kathey Marsden added a comment -

          Here is a reproduction for the problem with the c patch where the isolation level is not restored after rejoining the global transaction. ReproIsoLost and utilXID, Run like
          java ReproIsoLost

          with the patch we get:
          FAIL: Isolation is:TRANSACTION_READ_COMMITTED instead of READ_UNCOMMITTED

          Show
          Kathey Marsden added a comment - Here is a reproduction for the problem with the c patch where the isolation level is not restored after rejoining the global transaction. ReproIsoLost and utilXID, Run like java ReproIsoLost with the patch we get: FAIL: Isolation is:TRANSACTION_READ_COMMITTED instead of READ_UNCOMMITTED
          Hide
          Kathey Marsden added a comment -

          I noticed, with the patch, if I add

          Statement s = cs1.createStatement();
          s.executeUpdate("create table foo (i int)");

          just before
          xar.end(xid, XAResource.TMSUCCESS);

          I get on the subsequent cs1 = xaconn.getConnection();
          $ java ReproIsoLost
          2010-04-27 00:18:34.201 GMT : Apache Derby Network Server - 10.7.0.0 alpha - (93
          8130:938140M) started and ready to accept connections on port 1597
          Exception in thread "main" java.sql.SQLNonTransientConnectionException: An error
          occurred during connect reset and the connection has been terminated. See chai
          ned exceptions for details.
          at org.apache.derby.client.am.SQLExceptionFactory40.getSQLException(SQLE
          xceptionFactory40.java:70)
          at org.apache.derby.client.am.SqlException.getSQLException(SqlException.
          java:358)
          at org.apache.derby.client.ClientPooledConnection.getConnection(ClientPo
          oledConnection.java:259)
          at org.apache.derby.client.ClientXAConnection.getConnection(ClientXAConn
          ection.java:70)
          at ReproIsoLost.main(ReproIsoLost.java:51)
          Caused by: org.apache.derby.client.am.DisconnectException: An error occurred dur
          ing connect reset and the connection has been terminated. See chained exception
          s for details.
          at org.apache.derby.client.am.Connection.reset(Connection.java:2175)
          at org.apache.derby.client.ClientPooledConnection.getConnection(ClientPo
          oledConnection.java:245)
          ... 2 more

          In the derby.log I have:
          , (SESSIONID = 3), (DATABASE = wombat), (DRDAID = NF000001.GC36-867504605894726310

          {2}

          ), Failed Statement is: SET CURRENT ISOLATION = CS

          ERROR X0Y77: Cannot issue set transaction isolation statement on a global transaction that is in progress because it would have implicitly committed the global transaction.

          at org.apache.derby.iapi.error.StandardException.newException(StandardException.java:276)

          at org.apache.derby.impl.sql.conn.GenericLanguageConnectionContext.setIsolationLevel(GenericLanguageConnectionContext.java:2650)

          at org.apache.derby.impl.sql.execute.SetTransactionIsolationConstantAction.executeConstantAction(SetTransactionIsolationConstantAction.java:86)

          at org.apache.derby.impl.sql.execute.MiscResultSet.open(MiscResultSet.java:61)

          at org.apache.derby.impl.sql.GenericPreparedStatement.executeStmt(GenericPreparedStatement.java:436)

          at org.apache.derby.impl.sql.GenericPreparedStatement.execute(GenericPreparedStatement.java:317)

          at org.apache.derby.impl.jdbc.EmbedStatement.executeStatement(EmbedStatement.java:1232)

          at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:625)

          at org.apache.derby.impl.jdbc.EmbedStatement.executeUpdate(EmbedStatement.java:175)

          at org.apache.derby.iapi.jdbc.BrokeredStatement.executeUpdate(BrokeredStatement.java:113)

          at org.apache.derby.impl.drda.DRDAConnThread.parseEXCSQLIMM(DRDAConnThread.java:5133)

          at org.apache.derby.impl.drda.DRDAConnThread.processCommands(DRDAConnThread.java:755)

          at org.apache.derby.impl.drda.DRDAConnThread.run(DRDAConnThread.java:295)

          Cleanup action completed

          This passes with 10.5 and presumably without the patch.

          Show
          Kathey Marsden added a comment - I noticed, with the patch, if I add Statement s = cs1.createStatement(); s.executeUpdate("create table foo (i int)"); just before xar.end(xid, XAResource.TMSUCCESS); I get on the subsequent cs1 = xaconn.getConnection(); $ java ReproIsoLost 2010-04-27 00:18:34.201 GMT : Apache Derby Network Server - 10.7.0.0 alpha - (93 8130:938140M) started and ready to accept connections on port 1597 Exception in thread "main" java.sql.SQLNonTransientConnectionException: An error occurred during connect reset and the connection has been terminated. See chai ned exceptions for details. at org.apache.derby.client.am.SQLExceptionFactory40.getSQLException(SQLE xceptionFactory40.java:70) at org.apache.derby.client.am.SqlException.getSQLException(SqlException. java:358) at org.apache.derby.client.ClientPooledConnection.getConnection(ClientPo oledConnection.java:259) at org.apache.derby.client.ClientXAConnection.getConnection(ClientXAConn ection.java:70) at ReproIsoLost.main(ReproIsoLost.java:51) Caused by: org.apache.derby.client.am.DisconnectException: An error occurred dur ing connect reset and the connection has been terminated. See chained exception s for details. at org.apache.derby.client.am.Connection.reset(Connection.java:2175) at org.apache.derby.client.ClientPooledConnection.getConnection(ClientPo oledConnection.java:245) ... 2 more In the derby.log I have: , (SESSIONID = 3), (DATABASE = wombat), (DRDAID = NF000001.GC36-867504605894726310 {2} ), Failed Statement is: SET CURRENT ISOLATION = CS ERROR X0Y77: Cannot issue set transaction isolation statement on a global transaction that is in progress because it would have implicitly committed the global transaction. at org.apache.derby.iapi.error.StandardException.newException(StandardException.java:276) at org.apache.derby.impl.sql.conn.GenericLanguageConnectionContext.setIsolationLevel(GenericLanguageConnectionContext.java:2650) at org.apache.derby.impl.sql.execute.SetTransactionIsolationConstantAction.executeConstantAction(SetTransactionIsolationConstantAction.java:86) at org.apache.derby.impl.sql.execute.MiscResultSet.open(MiscResultSet.java:61) at org.apache.derby.impl.sql.GenericPreparedStatement.executeStmt(GenericPreparedStatement.java:436) at org.apache.derby.impl.sql.GenericPreparedStatement.execute(GenericPreparedStatement.java:317) at org.apache.derby.impl.jdbc.EmbedStatement.executeStatement(EmbedStatement.java:1232) at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:625) at org.apache.derby.impl.jdbc.EmbedStatement.executeUpdate(EmbedStatement.java:175) at org.apache.derby.iapi.jdbc.BrokeredStatement.executeUpdate(BrokeredStatement.java:113) at org.apache.derby.impl.drda.DRDAConnThread.parseEXCSQLIMM(DRDAConnThread.java:5133) at org.apache.derby.impl.drda.DRDAConnThread.processCommands(DRDAConnThread.java:755) at org.apache.derby.impl.drda.DRDAConnThread.run(DRDAConnThread.java:295) Cleanup action completed This passes with 10.5 and presumably without the patch.
          Hide
          Lily Wei added a comment -

          At this point, I am hesitate to keep working on piggy-backing fix due to the fact that we can not execute a statement in the local transaction when we rejoin to the global transaction per Kathey's example( please see ReproIsoLost.java). With DERBY-4314-5 fix, we don't commit when calling setTransactionIsolation(). However, we do have performance issue when connection is not cache. We will have server round-trip when we first call seTransactionIsolation when creating the connection, we will have two round trip when isolation is not set to READ_COMMITTED (check + change) In any case, DERBY-4314-5 do fix setTransctonIsolation() not perform commit issue. Therefore, I am leading toward to submit DERBY-4314-5 change and open a new JIRA for the performance issue. From the new JIRA, we can investigate further about the piggybacking issue and beyond and have conclusion on this issue and DERBY-4343. Is this sound like a reasonable solution for this issue?

          Show
          Lily Wei added a comment - At this point, I am hesitate to keep working on piggy-backing fix due to the fact that we can not execute a statement in the local transaction when we rejoin to the global transaction per Kathey's example( please see ReproIsoLost.java). With DERBY-4314 -5 fix, we don't commit when calling setTransactionIsolation(). However, we do have performance issue when connection is not cache. We will have server round-trip when we first call seTransactionIsolation when creating the connection, we will have two round trip when isolation is not set to READ_COMMITTED (check + change) In any case, DERBY-4314 -5 do fix setTransctonIsolation() not perform commit issue. Therefore, I am leading toward to submit DERBY-4314 -5 change and open a new JIRA for the performance issue. From the new JIRA, we can investigate further about the piggybacking issue and beyond and have conclusion on this issue and DERBY-4343 . Is this sound like a reasonable solution for this issue?
          Hide
          Kristian Waagan added a comment -

          Attached patch 6d, which is a combined patch with some additional XA changes in the client.

          Can someone with more knowledge about XA have a look?
          Briefly described, I have made the client not reset the isolation level when we are doing a join or resume. Instead, we pick up the piggy-backed value from the server. The server, I believe, is keeping track of the correct value for the isolation level .

          The full regression tests are running, I verified that the tests that failed with the previous patch(es) passed.

          Show
          Kristian Waagan added a comment - Attached patch 6d, which is a combined patch with some additional XA changes in the client. Can someone with more knowledge about XA have a look? Briefly described, I have made the client not reset the isolation level when we are doing a join or resume. Instead, we pick up the piggy-backed value from the server. The server, I believe, is keeping track of the correct value for the isolation level . The full regression tests are running, I verified that the tests that failed with the previous patch(es) passed.
          Hide
          Lily Wei added a comment -

          This is the version I intent to submit for this fix.
          This fix avoid assertion like DERBY-4343 by short-circuiting setTransactionIsolation.
          For case as users obtaining the pooled connection for the third time, the variable
          isolation_ is reset Connection.completeReset. If users call setTransactionIsolation and executed, the server does not send any piggybacking update because the isolation level has not changed.
          Isolation_ remain as UNKNOWN until getTransactionIsolation is called
          or a different statement causing a change of the isolation level
          is executed. As mention before, we should think about change this to READ_UNCOMMITTED.
          With introducing getTransactionIsolationX and assertion is never reach. Therefore, no assertion.
          The client driver acts as embedded as without commit action when setTransactionIsolation is called.
          Performance concern: This fix does not improve performance, it will just move the server round-trip. In some cases performance will be worse for the initial setTransactionIsolation call, depending on
          whether isolation_ value being set to READ_COMMITTED or not. If it is, one round-trip (check + short-circuit). If it is not, two round-trip (check + change).

          I don't fully understand piggybacking. Any suggestion is welcome. I simply think this is a good place to address this bug and DERBY-4343. I will suggestion to open a new JIRA for the performance concern.

          I run suits.all and derbyall.

          Show
          Lily Wei added a comment - This is the version I intent to submit for this fix. This fix avoid assertion like DERBY-4343 by short-circuiting setTransactionIsolation. For case as users obtaining the pooled connection for the third time, the variable isolation_ is reset Connection.completeReset. If users call setTransactionIsolation and executed, the server does not send any piggybacking update because the isolation level has not changed. Isolation_ remain as UNKNOWN until getTransactionIsolation is called or a different statement causing a change of the isolation level is executed. As mention before, we should think about change this to READ_UNCOMMITTED. With introducing getTransactionIsolationX and assertion is never reach. Therefore, no assertion. The client driver acts as embedded as without commit action when setTransactionIsolation is called. Performance concern: This fix does not improve performance, it will just move the server round-trip. In some cases performance will be worse for the initial setTransactionIsolation call, depending on whether isolation_ value being set to READ_COMMITTED or not. If it is, one round-trip (check + short-circuit). If it is not, two round-trip (check + change). I don't fully understand piggybacking. Any suggestion is welcome. I simply think this is a good place to address this bug and DERBY-4343 . I will suggestion to open a new JIRA for the performance concern. I run suits.all and derbyall.
          Hide
          Kristian Waagan added a comment -

          Patch 6d ran without failures.

          If I'm not mistaken, Lily's latest patch is a "sub-patch" of 6d, so committing it here and creating a new issue for the piggy-backing is fine with me.

          Show
          Kristian Waagan added a comment - Patch 6d ran without failures. If I'm not mistaken, Lily's latest patch is a "sub-patch" of 6d, so committing it here and creating a new issue for the piggy-backing is fine with me.
          Hide
          Lily Wei added a comment -

          Thanks Kristian for combined patch with some additional XA changes in the client. I believe the fix also address the server does not send any piggybacking update and improve the performance concern since we are getting the right isolation level. We still need to think about whether to check isolation_ value to TRANSACTION_READ_COMMITTED. With this patch, I also add more comment on setTransactionIsolation since that was why we commit when call setTransactionIsolation.

          Suite.all and derby tests passed.

          Show
          Lily Wei added a comment - Thanks Kristian for combined patch with some additional XA changes in the client. I believe the fix also address the server does not send any piggybacking update and improve the performance concern since we are getting the right isolation level. We still need to think about whether to check isolation_ value to TRANSACTION_READ_COMMITTED. With this patch, I also add more comment on setTransactionIsolation since that was why we commit when call setTransactionIsolation. Suite.all and derby tests passed.
          Hide
          Kathey Marsden added a comment -

          I will look at the 7b-combine patch and commit. Thanks Lily and Kristian for this collaborative effort !

          Show
          Kathey Marsden added a comment - I will look at the 7b-combine patch and commit. Thanks Lily and Kristian for this collaborative effort !
          Hide
          Lily Wei added a comment -

          Thanks to Kathey and Kristian, with #940620, setTransactionIsolation will not commit and it behaves the same for network server and embedded server. The nightly tests are coming up.

          Show
          Lily Wei added a comment - Thanks to Kathey and Kristian, with #940620, setTransactionIsolation will not commit and it behaves the same for network server and embedded server. The nightly tests are coming up.
          Hide
          Lily Wei added a comment -

          Change fix version information and add patch available

          Show
          Lily Wei added a comment - Change fix version information and add patch available
          Hide
          Kristian Waagan added a comment -

          The issue DERBY-2064, marked as a duplicate, has Release Note Needed set.
          Is a release note needed?
          If so, it should be written and the issue should be resolved.

          Show
          Kristian Waagan added a comment - The issue DERBY-2064 , marked as a duplicate, has Release Note Needed set. Is a release note needed? If so, it should be written and the issue should be resolved.
          Hide
          Kathey Marsden added a comment -

          Yes, I think we should have a release note saying that the client Driver will no longer commit on setTransactionIsolation if the isolation level does not change. Users relying on the previous behavior to commit their transaction, should now add an explicit commit to their code.

          See Writing a release note at: http://wiki.apache.org/db-derby/ReleaseNoteProcess

          Show
          Kathey Marsden added a comment - Yes, I think we should have a release note saying that the client Driver will no longer commit on setTransactionIsolation if the isolation level does not change. Users relying on the previous behavior to commit their transaction, should now add an explicit commit to their code. See Writing a release note at: http://wiki.apache.org/db-derby/ReleaseNoteProcess
          Hide
          Lily Wei added a comment -

          Thanks to Kathey, I am able to prepare the release note for setTransactionIsolation change. The change was made to client to match the embedded behavior and to improve client performance.

          Show
          Lily Wei added a comment - Thanks to Kathey, I am able to prepare the release note for setTransactionIsolation change. The change was made to client to match the embedded behavior and to improve client performance.
          Hide
          Kristian Waagan added a comment -

          Hi Lily,

          Here are my comments on the release note:
          — Summary of Change
          a) Typo in method name. Maybe use <tt></tt> around method names? Include the API class, i.e. Connection.getTransactionIsolation?
          I.e. "<tt>Connection.setTransactionIsolation</tt> in the Derby client driver will not issue a commit if the isolation level does not change."
          — Symptoms Seen by Applications Affected by Change
          b) "Application" -> "Applications"
          — Incompatibilities with Previous Release
          c) "In previous releases, <tt>Connection.setTransactionIsolation</tt> in the Derby client driver would issue a commit even if the isolation level did not change."
          d) Skip the rest of the paragraph, it is already mentioned under summary of change?
          — Rationale for Change
          e) " ...embedded behavior..." -> "...behavior of the embedded driver..."
          — Application Changes Required
          f) "transactions" -> "transaction"?

          Use your own judgment on the suggestions

          Thanks,

          Show
          Kristian Waagan added a comment - Hi Lily, Here are my comments on the release note: — Summary of Change a) Typo in method name. Maybe use <tt></tt> around method names? Include the API class, i.e. Connection.getTransactionIsolation? I.e. "<tt>Connection.setTransactionIsolation</tt> in the Derby client driver will not issue a commit if the isolation level does not change." — Symptoms Seen by Applications Affected by Change b) "Application" -> "Applications" — Incompatibilities with Previous Release c) "In previous releases, <tt>Connection.setTransactionIsolation</tt> in the Derby client driver would issue a commit even if the isolation level did not change." d) Skip the rest of the paragraph, it is already mentioned under summary of change? — Rationale for Change e) " ...embedded behavior..." -> "...behavior of the embedded driver..." — Application Changes Required f) "transactions" -> "transaction"? Use your own judgment on the suggestions Thanks,
          Hide
          Lily Wei added a comment -

          Thanks Kristian for reviewing the release note. I make the change accordingly.
          a) Typo in method name. Maybe use <tt></tt> around method names? Include the API class, i.e. Connection.getTransactionIsolation?
          I.e. "<tt>Connection.setTransactionIsolation</tt> in the Derby client driver will not issue a commit if the isolation level does not change."
          <tt></tt> has been added to API class for the releaseNote of DERBY-4314.
          — Symptoms Seen by Applications Affected by Change
          b) "Application" -> "Applications"
          This has been changed.
          — Incompatibilities with Previous Release
          c) "In previous releases, <tt>Connection.setTransactionIsolation</tt> in the Derby client driver would issue a commit even if the isolation level did not change."
          The change has been made accordingly.
          d) Skip the rest of the paragraph, it is already mentioned under summary of change?
          As it for me, it is clearer to have it there. I decide to keep it there. I hope that is okay with everybody. If not, we can take it out.
          — Rationale for Change
          e) " ...embedded behavior..." -> "...behavior of the embedded driver..."
          Changed.
          — Application Changes Required
          f) "transactions" -> "transaction"?
          Fix the typo.

          Thank you so much for all the help,
          Lily

          Show
          Lily Wei added a comment - Thanks Kristian for reviewing the release note. I make the change accordingly. a) Typo in method name. Maybe use <tt></tt> around method names? Include the API class, i.e. Connection.getTransactionIsolation? I.e. "<tt>Connection.setTransactionIsolation</tt> in the Derby client driver will not issue a commit if the isolation level does not change." <tt></tt> has been added to API class for the releaseNote of DERBY-4314 . — Symptoms Seen by Applications Affected by Change b) "Application" -> "Applications" This has been changed. — Incompatibilities with Previous Release c) "In previous releases, <tt>Connection.setTransactionIsolation</tt> in the Derby client driver would issue a commit even if the isolation level did not change." The change has been made accordingly. d) Skip the rest of the paragraph, it is already mentioned under summary of change? As it for me, it is clearer to have it there. I decide to keep it there. I hope that is okay with everybody. If not, we can take it out. — Rationale for Change e) " ...embedded behavior..." -> "...behavior of the embedded driver..." Changed. — Application Changes Required f) "transactions" -> "transaction"? Fix the typo. Thank you so much for all the help, Lily
          Hide
          Kathey Marsden added a comment -

          reopen to add label

          Show
          Kathey Marsden added a comment - reopen to add label

            People

            • Assignee:
              Lily Wei
              Reporter:
              Kathey Marsden
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development