You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-dev@db.apache.org by "Kristian Waagan (JIRA)" <ji...@apache.org> on 2010/05/12 10:14:52 UTC
[jira] Created: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Avoid unnecessary round-trip for commit/rollback in the client driver
---------------------------------------------------------------------
Key: DERBY-4653
URL: https://issues.apache.org/jira/browse/DERBY-4653
Project: Derby
Issue Type: Improvement
Components: JDBC, Network Client
Affects Versions: 10.7.0.0
Reporter: Kristian Waagan
Priority: Minor
The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
>From Connection.flowCommit:
// Per JDBC specification (see javadoc for Connection.commit()):
// "This method should be used only when auto-commit mode has been disabled."
// However, some applications do this anyway, it is harmless, so
// if they ask to commit, we could go ahead and flow a commit.
// But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
// This behavior is subject to further review.
// if (!this.inUnitOfWork)
// return;
// We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
// regardless of whether or not we're in a unit of work or in auto-commit mode.
//
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-4653) Avoid unnecessary round-trip for
commit in the client driver
Posted by "Lily Wei (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Lily Wei updated DERBY-4653:
----------------------------
Attachment: DERBY-4653-7_withflowcommittest_comment_update_diff_followup.txt
This patch take out the unused variable expectednumtransaction and set the test case to run with autocommit off so we will flow on first commit and does not flow the second commit.
> Avoid unnecessary round-trip for commit in the client driver
> -------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: _sds_0, DERBY-4653-1.diff, DERBY-4653-2.diff, DERBY-4653-3_withrollback.diff, DERBY-4653-4_withcommitrollbacktest.diff, DERBY-4653-5_withflowcommitrollback.diff, DERBY-4653-6_withflowcommitrollback.diff, DERBY-4653-7_withflowcommittest.diff, DERBY-4653-7_withflowcommittest_comment_update_diff.txt, DERBY-4653-7_withflowcommittest_comment_update_diff.txt, DERBY-4653-7_withflowcommittest_comment_update_diff_followup.txt, ReproTransInProgressAttempt.java, SaveRoundClientDS.java, SaveRoundClientDS.java
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Posted by "Lily Wei (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Lily Wei updated DERBY-4653:
----------------------------
Attachment: DERBY-4653-4_withcommitrollbacktest.diff
The fix is the same as before, we flowcommit() when it is safe and we flowrollback() when we are in XAConnection. I run it against Suites.All and Derbyall and they all passed.
With Kathey's help, I add new test to test just Connection.flowcommit() and Connection.flowrollback(). The different signature testConnectionFlowCommitRollback() is added to J2EEDataSourceTest.
In this test, when we flow commit, we check the transaction increase by 1.
When we are testing rollback, we rollback the transaction and execute an insert statement, the transaction increase by 2.
With the little repro, manual check the trace file to see the DRDA protocol trace save the round trip for flowcommit() and flowrollback() is also done.
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: _sds_0, DERBY-4653-1.diff, DERBY-4653-2.diff, DERBY-4653-3_withrollback.diff, DERBY-4653-4_withcommitrollbacktest.diff, SaveRoundClientDS.java, SaveRoundClientDS.java
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Posted by "Lily Wei (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12880277#action_12880277 ]
Lily Wei commented on DERBY-4653:
---------------------------------
Derbyall runs without an error either. The DERBY-4653-7_withflowcommittest.diff is ready for review or commit.
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: _sds_0, DERBY-4653-1.diff, DERBY-4653-2.diff, DERBY-4653-3_withrollback.diff, DERBY-4653-4_withcommitrollbacktest.diff, DERBY-4653-5_withflowcommitrollback.diff, DERBY-4653-6_withflowcommitrollback.diff, DERBY-4653-7_withflowcommittest.diff, ReproTransInProgressAttempt.java, SaveRoundClientDS.java, SaveRoundClientDS.java
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Posted by "Lily Wei (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Lily Wei updated DERBY-4653:
----------------------------
Attachment: DERBY-4653-5_withflowcommitrollback.diff
Thanks Kathey for showing me around and making this patch to this stage and explain all the issues to me.
With this patch, it has the intention fix for Connection.flowcommit() and Connection.flowrollback(). However, when running suites.All, some tests failed. i.e. jdbcapi.StatementPollingTest
The change of the patch which answer the issue Kristian point out. Thanks Kristian for the looking at the patch and beyond.
J2EEDataSourceTest
1. The test gets added to getClientSuite(). The test is only running on regular client, pooled, and xa datasources.
2. Change to javadoc as Kristian point out.
3. Please see comment in BaseJdbcTestCase.java
4. No need to use the system table as part of test anymore.
BaseJdbcTestCase.java
5. getClientTransactonID(Connection conn) is in BaseJdbcTestCase.java so it is closer in suites than J2EEDataSourceTest and the comment is as following
"If we are not in a transaction, we don't want to flow commit. We just return"
6. For flowRollback: Like Kathey said, the original change is only for XAConnection which does not make sense. With the new change, server flow rollback if isUnitOfWork_ indicates we are not in a transaction, we return. However, with this change, StatementPoolingTest failed
at org.apache.derbyTesting.functionTests.tests.jdbcapi.StatementPoolingTest.doTestResultSetCloseForHoldability(StatementPoolingTest.java:591)
at org.apache.derbyTesting.functionTests.tests.jdbcapi.StatementPoolingTest.resTestHoldCursorsOverCommit(StatementPoolingTest.java:522)
I am in the middle of evaluating StatementPoolingTest failed and what change the value of isUnitOfWork_ when the test trying to close the connection and whether it has to with CachingLogicalConnection40. Will the test close the connection that is not the connection it performs rollback? What could be change the value of isUnitOfWork_ within StatementPoolingTest.
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: _sds_0, DERBY-4653-1.diff, DERBY-4653-2.diff, DERBY-4653-3_withrollback.diff, DERBY-4653-4_withcommitrollbacktest.diff, DERBY-4653-5_withflowcommitrollback.diff, SaveRoundClientDS.java, SaveRoundClientDS.java
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Posted by "Lily Wei (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12871444#action_12871444 ]
Lily Wei commented on DERBY-4653:
---------------------------------
Hi Knut:
You are right. It does say 10 tests. I was hoping to see the list of the 10 tests. However, it may not be necessary. Thanks.
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: DERBY-4653-1.diff
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-4653) Avoid unnecessary round-trip for
commit in the client driver
Posted by "Kathey Marsden (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Kathey Marsden updated DERBY-4653:
----------------------------------
Summary: Avoid unnecessary round-trip for commit in the client driver (was: Avoid unnecessary round-trip for commit/rollback in the client driver)
Lily recommends putting in the change first for commit and has filed a second issue for rollback, That fix is perhaps not worth the change, because all open result sets would need to be checked before optimizing out the rollback flow.
She filed another issue for rollback DERBY-4687
, so I am changing the summary for this issue to cover just commit. I looked at the patch DERBY-4653-7_withflowcommittest.diff and I think it looks very good except a couple of stale comments. I will attach the revised patch and will commit Monday unless there are more comments.
> Avoid unnecessary round-trip for commit in the client driver
> -------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: _sds_0, DERBY-4653-1.diff, DERBY-4653-2.diff, DERBY-4653-3_withrollback.diff, DERBY-4653-4_withcommitrollbacktest.diff, DERBY-4653-5_withflowcommitrollback.diff, DERBY-4653-6_withflowcommitrollback.diff, DERBY-4653-7_withflowcommittest.diff, ReproTransInProgressAttempt.java, SaveRoundClientDS.java, SaveRoundClientDS.java
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12871239#action_12871239 ]
Knut Anders Hatlen commented on DERBY-4653:
-------------------------------------------
Hi Lily. From the output you posted above, it looks like the test is running successfully (it says OK - 10 tests). Or is there some other problem you are seeing?
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: DERBY-4653-1.diff
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-4653) Avoid unnecessary round-trip for
commit in the client driver
Posted by "Kathey Marsden (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Kathey Marsden updated DERBY-4653:
----------------------------------
Attachment: DERBY-4653-7_withflowcommittest_comment_update_diff.txt
Here is the slightly revised patch. There was just a comment in the test that it ran with embedded, but it has been changed to run only with client.
> Avoid unnecessary round-trip for commit in the client driver
> -------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: _sds_0, DERBY-4653-1.diff, DERBY-4653-2.diff, DERBY-4653-3_withrollback.diff, DERBY-4653-4_withcommitrollbacktest.diff, DERBY-4653-5_withflowcommitrollback.diff, DERBY-4653-6_withflowcommitrollback.diff, DERBY-4653-7_withflowcommittest.diff, DERBY-4653-7_withflowcommittest_comment_update_diff.txt, ReproTransInProgressAttempt.java, SaveRoundClientDS.java, SaveRoundClientDS.java
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (DERBY-4653) Avoid unnecessary
round-trip for commit/rollback in the client driver
Posted by "Lily Wei (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12876962#action_12876962 ]
Lily Wei edited comment on DERBY-4653 at 6/9/10 1:41 AM:
---------------------------------------------------------
Thanks Kathey for showing me around and making this patch to this stage and explain all the issues to me.
With this patch, it has the intention fix for Connection.flowcommit() and Connection.flowrollback(). However, when running suites.All, some tests failed. i.e. jdbcapi.StatementPollingTest
The change of the patch which answer the issue Kristian point out. Thanks Kristian for the looking at the patch and beyond.
J2EEDataSourceTest
1. The test gets added to getClientSuite(). The test is only running on regular client, pooled, and xa datasources.
2. Change to javadoc as Kristian point out.
3. Please see comment in BaseJdbcTestCase.java
4. No need to use the system table as part of test anymore.
BaseJdbcTestCase.java
5. getClientTransactonID(Connection conn) is in BaseJdbcTestCase.java so it is closer in suites than J2EEDataSourceTest and the comment is as following
"If we are not in a transaction, we don't want to flow commit. We just return"
6. For flowRollback: Like Kathey said, the original change is only for XAConnection which does not make sense. With the new change, server flow rollback if isUnitOfWork_ indicates we are not in a transaction, we return. However, with this change, StatementPoolingTest failed
at org.apache.derbyTesting.functionTests.tests.jdbcapi.StatementPoolingTest.doTestResultSetCloseForHoldability(StatementPoolingTest.java:591)
at org.apache.derbyTesting.functionTests.tests.jdbcapi.StatementPoolingTest.resTestHoldCursorsOverCommit(StatementPoolingTest.java:522)
I am in the middle of evaluating StatementPoolingTest failed and what change the value of isUnitOfWork_ when the test trying to close the connection and whether it has to with CachingLogicalConnection40. Will the test close the connection that is not the connection it performs rollback? What could be change the value of isUnitOfWork_ within StatementPoolingTest?
was (Author: lilywei):
Thanks Kathey for showing me around and making this patch to this stage and explain all the issues to me.
With this patch, it has the intention fix for Connection.flowcommit() and Connection.flowrollback(). However, when running suites.All, some tests failed. i.e. jdbcapi.StatementPollingTest
The change of the patch which answer the issue Kristian point out. Thanks Kristian for the looking at the patch and beyond.
J2EEDataSourceTest
1. The test gets added to getClientSuite(). The test is only running on regular client, pooled, and xa datasources.
2. Change to javadoc as Kristian point out.
3. Please see comment in BaseJdbcTestCase.java
4. No need to use the system table as part of test anymore.
BaseJdbcTestCase.java
5. getClientTransactonID(Connection conn) is in BaseJdbcTestCase.java so it is closer in suites than J2EEDataSourceTest and the comment is as following
"If we are not in a transaction, we don't want to flow commit. We just return"
6. For flowRollback: Like Kathey said, the original change is only for XAConnection which does not make sense. With the new change, server flow rollback if isUnitOfWork_ indicates we are not in a transaction, we return. However, with this change, StatementPoolingTest failed
at org.apache.derbyTesting.functionTests.tests.jdbcapi.StatementPoolingTest.doTestResultSetCloseForHoldability(StatementPoolingTest.java:591)
at org.apache.derbyTesting.functionTests.tests.jdbcapi.StatementPoolingTest.resTestHoldCursorsOverCommit(StatementPoolingTest.java:522)
I am in the middle of evaluating StatementPoolingTest failed and what change the value of isUnitOfWork_ when the test trying to close the connection and whether it has to with CachingLogicalConnection40. Will the test close the connection that is not the connection it performs rollback? What could be change the value of isUnitOfWork_ within StatementPoolingTest.
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: _sds_0, DERBY-4653-1.diff, DERBY-4653-2.diff, DERBY-4653-3_withrollback.diff, DERBY-4653-4_withcommitrollbacktest.diff, DERBY-4653-5_withflowcommitrollback.diff, SaveRoundClientDS.java, SaveRoundClientDS.java
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Assigned: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Posted by "Lily Wei (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Lily Wei reassigned DERBY-4653:
-------------------------------
Assignee: Lily Wei
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Posted by "Lily Wei (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12870721#action_12870721 ]
Lily Wei commented on DERBY-4653:
---------------------------------
I notice with java 1.6.0_13, test BooleanValuesTest does not run.
Is this expected behavior?
$ java junit.textui.TestRunner org.apache.derbyTesting.functionTests.tests.lang.B
ooleanValuesTest
Time: 0.001
OK (0 tests)
$ java -version
java version "1.6.0_13"
Java(TM) SE Runtime Environment (build 1.6.0_13-b03)
Java HotSpot(TM) Client VM (build 11.3-b02, mixed mode)
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: DERBY-4653-1.diff
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-4653) Avoid unnecessary round-trip for
commit in the client driver
Posted by "Kristian Waagan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12881674#action_12881674 ]
Kristian Waagan commented on DERBY-4653:
----------------------------------------
Sorry, I missed the fact that DERBY-4709 was logged for the alternative test.
Deleted the attachment and will upload it to the correct issue.
> Avoid unnecessary round-trip for commit in the client driver
> -------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: _sds_0, DERBY-4653-1.diff, DERBY-4653-2.diff, DERBY-4653-3_withrollback.diff, DERBY-4653-4_withcommitrollbacktest.diff, DERBY-4653-5_withflowcommitrollback.diff, DERBY-4653-6_withflowcommitrollback.diff, DERBY-4653-7_withflowcommittest.diff, DERBY-4653-7_withflowcommittest_comment_update_diff.txt, DERBY-4653-7_withflowcommittest_comment_update_diff.txt, DERBY-4653-7_withflowcommittest_comment_update_diff_followup.txt, ReproTransInProgressAttempt.java, SaveRoundClientDS.java, SaveRoundClientDS.java
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Posted by "Lily Wei (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Lily Wei updated DERBY-4653:
----------------------------
Attachment: DERBY-4653-7_withflowcommittest.diff
This patch is without Connection.flowrollback performance improvement fix.
With the test, it is using the same approach as transaction id for Connection.flowcommit save round-trip fix. Move the getClientTransactionID to more test suite level - BaseJDBCTestCase.java
It is ready to commit. Running Suites.All passed. I am running derbyall test suite now.
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: _sds_0, DERBY-4653-1.diff, DERBY-4653-2.diff, DERBY-4653-3_withrollback.diff, DERBY-4653-4_withcommitrollbacktest.diff, DERBY-4653-5_withflowcommitrollback.diff, DERBY-4653-6_withflowcommitrollback.diff, DERBY-4653-7_withflowcommittest.diff, ReproTransInProgressAttempt.java, SaveRoundClientDS.java, SaveRoundClientDS.java
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Posted by "Kristian Waagan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12875737#action_12875737 ]
Kristian Waagan commented on DERBY-4653:
----------------------------------------
Sorry for the short post (it's late here now), but I think there are some issues with the patch.
Feel free to have another look at it right away, I'll have a look on Monday in any case.
Also, I'll be happy to help you write the code parsing the trace file (it's good to keep the existing test) , i.e. something like:
- obtain first connection, run test sequence without redundant commits
- parse trace file to obtain baseline for number of commits
- obtain second connection, run test sequence with many of extra commits
- parse trace file to obtain number of commits
- compare the commit counts
In this case it might be good to factor out the test sequence code into a method with a boolean telling wheter or not to do the extra commits.
It's fine if you don't implement this test, I may have a go at it if you don't :)
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: _sds_0, DERBY-4653-1.diff, DERBY-4653-2.diff, DERBY-4653-3_withrollback.diff, DERBY-4653-4_withcommitrollbacktest.diff, SaveRoundClientDS.java, SaveRoundClientDS.java
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-4653) Avoid unnecessary round-trip for
commit in the client driver
Posted by "Kathey Marsden (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12881408#action_12881408 ]
Kathey Marsden commented on DERBY-4653:
---------------------------------------
I committed Kristian's version of DERBY-4653 and will leave to Lily to submit a follow up patch for the issues pointed out in his last comment. I think the trace file parsing test might be pretty fragile, so am not such a big fan.
I agree that rollback is important. I will post a thought to issue DERBY-4687,
> Avoid unnecessary round-trip for commit in the client driver
> -------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: _sds_0, DERBY-4653-1.diff, DERBY-4653-2.diff, DERBY-4653-3_withrollback.diff, DERBY-4653-4_withcommitrollbacktest.diff, DERBY-4653-5_withflowcommitrollback.diff, DERBY-4653-6_withflowcommitrollback.diff, DERBY-4653-7_withflowcommittest.diff, DERBY-4653-7_withflowcommittest_comment_update_diff.txt, DERBY-4653-7_withflowcommittest_comment_update_diff.txt, ReproTransInProgressAttempt.java, SaveRoundClientDS.java, SaveRoundClientDS.java
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Posted by "Lily Wei (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12871194#action_12871194 ]
Lily Wei commented on DERBY-4653:
---------------------------------
Hi Knut:
Thank you so much for the prompt response. I did svn update and up to revision 948072 and upgrade my jdk to 1.6.0_20.
For some reasons, the test in BooleanValuesTest is not running.
$ java junit.textui.TestRunner org.apache.derbyTesting.functionTests.tests.lang.B
ooleanValuesTest
..........
Time: 8.245
OK (10 tests)
$ java -version
java version "1.6.0_20"
Java(TM) SE Runtime Environment (build 1.6.0_20-b02)
Java HotSpot(TM) Client VM (build 16.3-b01, mixed mode, sharing)
$ svn update
At revision 948072.
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: DERBY-4653-1.diff
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Posted by "Lily Wei (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Lily Wei updated DERBY-4653:
----------------------------
Attachment: ReproTransInProgressAttempt.java
Thanks to Kathey. I got to know more in turn of why Connection.close() after Connection.flowrollback() will cause error "Cannot close a connection while a transaction is still active" for StatementPoolingTest. As the ReproTransInProgressAttempt.java shows, if we have commit between we are in ResultSet, we should not flowrollback. If flowrollback remains unchanged, it will detect the state of the statement and resultSet and keep all the flag as inUnitOfWork etc in the right place and Connection.close() will not run into error as "Cannot close a connection while a transaction is still active".
Due to this reason, I am leading toward to not fix flowrollback() to save round trip and just keep the change for flowcommit() to improve performance.
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: _sds_0, DERBY-4653-1.diff, DERBY-4653-2.diff, DERBY-4653-3_withrollback.diff, DERBY-4653-4_withcommitrollbacktest.diff, DERBY-4653-5_withflowcommitrollback.diff, DERBY-4653-6_withflowcommitrollback.diff, ReproTransInProgressAttempt.java, SaveRoundClientDS.java, SaveRoundClientDS.java
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Posted by "Kathey Marsden (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12874083#action_12874083 ]
Kathey Marsden commented on DERBY-4653:
---------------------------------------
I think that sounds like a good idea to check the file for the RDBCMM string and to make the name more predictable, you might want to use traceFile instead of traceDirectory. Also I think it makes sense to check in the commit part of this patch while working through the rollback issues in another patch, but it would be good to do so with the regression test for the commit portion.
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: _sds_0, DERBY-4653-1.diff, DERBY-4653-2.diff, SaveRoundClientDS.java
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Posted by "Kristian Waagan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12873907#action_12873907 ]
Kristian Waagan commented on DERBY-4653:
----------------------------------------
Would it be good enough to simply iterate over the lines in the trace file, counting occurrences of RDBCMM and RDBRLLBCK?
I modified your repro, adding some more commits and rollbacks. When doing a manual grep on the trace file, I got 50 matches without your patch, and 17 with.
To parse the file you should use SupportFilesSetup and PrivilegedFileOpsForTests.
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: _sds_0, DERBY-4653-1.diff, DERBY-4653-2.diff, SaveRoundClientDS.java
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-4653) Avoid unnecessary round-trip for
commit in the client driver
Posted by "Kristian Waagan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Kristian Waagan updated DERBY-4653:
-----------------------------------
Attachment: derby-4653-8a-alternative_test.diff
Attached patch 8a, which implements an alternative test parsing the client connection trace file.
It's a bit more extensive than the existing test, but may not add any extra value.
Uploaded for reference, as it may come in useful for debugging purposes.
> Avoid unnecessary round-trip for commit in the client driver
> -------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: _sds_0, DERBY-4653-1.diff, DERBY-4653-2.diff, DERBY-4653-3_withrollback.diff, DERBY-4653-4_withcommitrollbacktest.diff, DERBY-4653-5_withflowcommitrollback.diff, DERBY-4653-6_withflowcommitrollback.diff, DERBY-4653-7_withflowcommittest.diff, DERBY-4653-7_withflowcommittest_comment_update_diff.txt, DERBY-4653-7_withflowcommittest_comment_update_diff.txt, DERBY-4653-7_withflowcommittest_comment_update_diff_followup.txt, derby-4653-8a-alternative_test.diff, ReproTransInProgressAttempt.java, SaveRoundClientDS.java, SaveRoundClientDS.java
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Posted by "Lily Wei (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Lily Wei updated DERBY-4653:
----------------------------
Attachment: DERBY-4653-3_withrollback.diff
SaveRoundClientDS.java
Thanks to Kristian and Kathey.
I fix the avoid traffic for flowrollback(). We need to just flow rollback when we are in XAConnection.
The patch passed suites.allpackages and derbyall.
To test the fix:
1. Will it be okay if we just reflect the method in ..am.Connection.getTransactionID() and make sure the next transaction is after first commit()()/rollback is a predictable number. After the test is run, there will always be a trace file hanging around for manual inspection. It could be cheating a little bit. However, it is a little easier than test open the trace file and search for the number of occurrences of RDBCMM or something like that.
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: _sds_0, DERBY-4653-1.diff, DERBY-4653-2.diff, DERBY-4653-3_withrollback.diff, SaveRoundClientDS.java, SaveRoundClientDS.java
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (DERBY-4653) Avoid unnecessary round-trip for
commit in the client driver
Posted by "Lily Wei (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Lily Wei resolved DERBY-4653.
-----------------------------
Fix Version/s: 10.7.0.0
Resolution: Fixed
The code to save performance on Connection.flowcommit has been added. A short version test has also been added. Please also see DERBY-4709 and DERBY-4687
> Avoid unnecessary round-trip for commit in the client driver
> -------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Fix For: 10.7.0.0
>
> Attachments: _sds_0, DERBY-4653-1.diff, DERBY-4653-2.diff, DERBY-4653-3_withrollback.diff, DERBY-4653-4_withcommitrollbacktest.diff, DERBY-4653-5_withflowcommitrollback.diff, DERBY-4653-6_withflowcommitrollback.diff, DERBY-4653-7_withflowcommittest.diff, DERBY-4653-7_withflowcommittest_comment_update_diff.txt, DERBY-4653-7_withflowcommittest_comment_update_diff.txt, DERBY-4653-7_withflowcommittest_comment_update_diff_followup.txt, ReproTransInProgressAttempt.java, SaveRoundClientDS.java, SaveRoundClientDS.java
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Posted by "Lily Wei (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Lily Wei updated DERBY-4653:
----------------------------
Issue & fix info: [Patch Available]
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: DERBY-4653-1.diff
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Posted by "Kristian Waagan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12874144#action_12874144 ]
Kristian Waagan commented on DERBY-4653:
----------------------------------------
Removed unused method Connection.setInUnitOfWork(boolean) with revision 950185.
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: _sds_0, DERBY-4653-1.diff, DERBY-4653-2.diff, SaveRoundClientDS.java
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Posted by "Kristian Waagan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12871528#action_12871528 ]
Kristian Waagan commented on DERBY-4653:
----------------------------------------
Hi Lily,
If you want to see the test names, you can run with the Swing (or awt) test runner [1], or specify the trace property [2] understood by the Derby test framework.
[1] junit.swingui.TestRunner -noloading
[2] -Dderby.tests.trace=true
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: DERBY-4653-1.diff
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (DERBY-4653) Avoid unnecessary
round-trip for commit/rollback in the client driver
Posted by "Lily Wei (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12876962#action_12876962 ]
Lily Wei edited comment on DERBY-4653 at 6/11/10 1:16 AM:
----------------------------------------------------------
Thanks Kathey for showing me around and making this patch to this stage and explain all the issues to me.
With this patch, it has the intention fix for Connection.flowcommit() and Connection.flowrollback(). However, when running suites.All, some tests failed. i.e. jdbcapi.StatementPollingTest
The change of the patch which answer the issue Kristian point out. Thanks Kristian for the looking at the patch and beyond.
J2EEDataSourceTest
1. The test gets added to getClientSuite(). The test is only running on regular client, pooled, and xa datasources.
2. Change to javadoc as Kristian point out.
3. Please see comment in BaseJdbcTestCase.java
4. No need to use the system table as part of test anymore.
BaseJdbcTestCase.java
5. getClientTransactonID(Connection conn) is in BaseJdbcTestCase.java so it is closer in suites than J2EEDataSourceTest and the comment is as following
"If we are not in a transaction, we don't want to flow commit. We just return"
6. For flowRollback: Like Kathey said, the original change is only for XAConnection which does not make sense. With the new change, server flow rollback if inUnitOfWork_ indicates we are not in a transaction, we return. However, with this change, StatementPoolingTest failed
at org.apache.derbyTesting.functionTests.tests.jdbcapi.StatementPoolingTest.doTestResultSetCloseForHoldability(StatementPoolingTest.java:591)
at org.apache.derbyTesting.functionTests.tests.jdbcapi.StatementPoolingTest.resTestHoldCursorsOverCommit(StatementPoolingTest.java:522)
I am in the middle of evaluating StatementPoolingTest failed and what change the value of inUnitOfWork_ when the test trying to close the connection and whether it has to with CachingLogicalConnection40. Will the test close the connection that is not the connection it performs rollback? What could be change the value of inUnitOfWork_ within StatementPoolingTest?
was (Author: lilywei):
Thanks Kathey for showing me around and making this patch to this stage and explain all the issues to me.
With this patch, it has the intention fix for Connection.flowcommit() and Connection.flowrollback(). However, when running suites.All, some tests failed. i.e. jdbcapi.StatementPollingTest
The change of the patch which answer the issue Kristian point out. Thanks Kristian for the looking at the patch and beyond.
J2EEDataSourceTest
1. The test gets added to getClientSuite(). The test is only running on regular client, pooled, and xa datasources.
2. Change to javadoc as Kristian point out.
3. Please see comment in BaseJdbcTestCase.java
4. No need to use the system table as part of test anymore.
BaseJdbcTestCase.java
5. getClientTransactonID(Connection conn) is in BaseJdbcTestCase.java so it is closer in suites than J2EEDataSourceTest and the comment is as following
"If we are not in a transaction, we don't want to flow commit. We just return"
6. For flowRollback: Like Kathey said, the original change is only for XAConnection which does not make sense. With the new change, server flow rollback if isUnitOfWork_ indicates we are not in a transaction, we return. However, with this change, StatementPoolingTest failed
at org.apache.derbyTesting.functionTests.tests.jdbcapi.StatementPoolingTest.doTestResultSetCloseForHoldability(StatementPoolingTest.java:591)
at org.apache.derbyTesting.functionTests.tests.jdbcapi.StatementPoolingTest.resTestHoldCursorsOverCommit(StatementPoolingTest.java:522)
I am in the middle of evaluating StatementPoolingTest failed and what change the value of isUnitOfWork_ when the test trying to close the connection and whether it has to with CachingLogicalConnection40. Will the test close the connection that is not the connection it performs rollback? What could be change the value of isUnitOfWork_ within StatementPoolingTest?
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: _sds_0, DERBY-4653-1.diff, DERBY-4653-2.diff, DERBY-4653-3_withrollback.diff, DERBY-4653-4_withcommitrollbacktest.diff, DERBY-4653-5_withflowcommitrollback.diff, DERBY-4653-6_withflowcommitrollback.diff, SaveRoundClientDS.java, SaveRoundClientDS.java
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Posted by "Kristian Waagan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12876239#action_12876239 ]
Kristian Waagan commented on DERBY-4653:
----------------------------------------
Hi Lily,
Here are my comments on patch 'DERBY-4653-4_withcommitrollbacktest.diff':
--- J2EEDataSourceTest
1) The test should be added to the client suite, since it is only relevant for the client driver.
2) The method comment could well be changed to a JavaDoc comment (/* -> /**)
3) Can the logic for skipping the test (i.e. wheter Connection.getTransactionID exists)
be moved into the suite method?)
If you create a static method to check if the method exists, if can be used both in the
suite method and in J2EEDataSourceTest.getConnectionID. There is one complication, and that is
that you cannot (or should not) obtain a connection in the suite method. Is passing the ClientDriver
class good enough?
If this gets too messy, just let it be. My concern with short-circuiting the tests themselves, is that
the will be listed as run and passed, even though they haven't really been run.
Does anyone have any options on this? Have we solved this problem earlier?
4) Instead of checking for a specific number of rows (X -14 >=0) ) in the system table
(which we don't really know), is it good enough to assert
rs.next() == true, rs.getInt() >= 0, rs.next() == false ?
--- Connection
5) Wrong comment? If we are in a connection, we do want to commit, right?
6) Can you explain the reasoning behind the changes in flowRollback?
I don't understand them, as I'm not that familiar with XA.
--- LogicalConnection
7) I don't understand the comment about embedded, nor why we have to add this method.
If it is to enable testing with logical connections (i.e. with XADataSource or CPDataSource),
maybe it is better to parse the trace file? In any case it would be nice to test a
"normal connection", since there is some special code for when the connection is originating
from CP-|XADataSource.
Possible improvements:
a) Add test parsing the trace file.
b) Test with XA as well (since there is special code for it).
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: _sds_0, DERBY-4653-1.diff, DERBY-4653-2.diff, DERBY-4653-3_withrollback.diff, DERBY-4653-4_withcommitrollbacktest.diff, SaveRoundClientDS.java, SaveRoundClientDS.java
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Posted by "Lily Wei (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Lily Wei updated DERBY-4653:
----------------------------
Attachment: DERBY-4653-2.diff
Thank you so much, Kristian. That is such a good suggestion. I will give it a try.
I add the patch for Connection.flowcommit() so it won't flow if we don't need to perform the unnecessary round trip.
Please see DERBY-4653-2.diff. I also try to add the test for testing saving commit. I add an extra commit on J2EEDataSourceTest to test the fix. However, if the behavior go back to the round-trip scenario, the test has no way knowing other than functionality is working. Does anyone has a good suggest to test the DRDA protocol for commit making round trip or not?
About making Connection.flowrollback not doing the round trip, It is not as straight forward as detecting isUnitofWork_ only. I think the check has to happen along with isXAConnection_ and where we are in the rollback state. I will keep working on that today. However, I do think that it is safe to commit DERBY-4653-2.diff. I will suggest to open a new JIRA for rollback only depend on how to test Connection.flowcommit DRDA related protocol.
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: DERBY-4653-1.diff, DERBY-4653-2.diff
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12871073#action_12871073 ]
Knut Anders Hatlen commented on DERBY-4653:
-------------------------------------------
Hi Lily,
The problems with BooleanValuesTest were fixed in DERBY-4674. You probably ran with java 1.6.0_13 after my initial check-in that disabled the test if Xalan was not available. Rick later re-enabled the test and made it run cleanly on platforms that didn't have the required XML libraries, so I believe it should be running fine now if you do an svn update first.
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: DERBY-4653-1.diff
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Posted by "Lily Wei (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12875790#action_12875790 ]
Lily Wei commented on DERBY-4653:
---------------------------------
Thank you so much, Kristian. It is late over there. We are so international. Not a problem about the short post. I was not totally clear in turn of the issues with the patch.
It will be great to have your help on code parsing and run test sequence with all the necessary steps for commits. This will be more tests on top of the existing test. And, it will be very complete tests.
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: _sds_0, DERBY-4653-1.diff, DERBY-4653-2.diff, DERBY-4653-3_withrollback.diff, DERBY-4653-4_withcommitrollbacktest.diff, SaveRoundClientDS.java, SaveRoundClientDS.java
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-4653) Avoid unnecessary round-trip for
commit in the client driver
Posted by "Kristian Waagan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Kristian Waagan updated DERBY-4653:
-----------------------------------
Attachment: (was: derby-4653-8a-alternative_test.diff)
> Avoid unnecessary round-trip for commit in the client driver
> -------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: _sds_0, DERBY-4653-1.diff, DERBY-4653-2.diff, DERBY-4653-3_withrollback.diff, DERBY-4653-4_withcommitrollbacktest.diff, DERBY-4653-5_withflowcommitrollback.diff, DERBY-4653-6_withflowcommitrollback.diff, DERBY-4653-7_withflowcommittest.diff, DERBY-4653-7_withflowcommittest_comment_update_diff.txt, DERBY-4653-7_withflowcommittest_comment_update_diff.txt, DERBY-4653-7_withflowcommittest_comment_update_diff_followup.txt, ReproTransInProgressAttempt.java, SaveRoundClientDS.java, SaveRoundClientDS.java
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Posted by "Kathey Marsden (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12876902#action_12876902 ]
Kathey Marsden commented on DERBY-4653:
---------------------------------------
wrt to some of Kristian's comments and a few things I talked to Lily about today.
I too think it would be good to test just for client and with regular, pooled, and xa datasources.
I think if the test runs just for client we should always count on the getTransactionID() method being there and can take out the skipping logic all together. I think the system table query can come out all together. I tend to think the getTransactionID() check is good enough, but the protocol trace parsing would be ok too.
I agree the comment about embedded should come out of am.LogicalConnection.
For the flowRollback() change I think the condition to return if inUnitOfWork should not be in the if block. I think the StatementPoolingTest failures you were seeing with it set that way may be because the state of inUnitOfWork may be wrong somehow where hold cursors and pooled connections are involved, probably an existing issue that needs to be tracked down before committing this change. Lily and I tried to reproduce it outside the test with the code but we were not quite able to get in the same state as StatementPoolingTest.
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: _sds_0, DERBY-4653-1.diff, DERBY-4653-2.diff, DERBY-4653-3_withrollback.diff, DERBY-4653-4_withcommitrollbacktest.diff, SaveRoundClientDS.java, SaveRoundClientDS.java
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Posted by "Lily Wei (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12877716#action_12877716 ]
Lily Wei commented on DERBY-4653:
---------------------------------
>From the debugger, we actually call Connection.completeTransactionStart(). This is the stack trace from conn.close->StatementCacheInterator.closeOpenLogicStatements ... Connection.completeTransactionStart()
===>
NetConnection40(Connection).completeTransactionStart() line: 2082
NetConnection40(Connection).readTransactionStart() line: 2078
NetConnection40(NetConnection).readTransactionStart() line: 1581
NetAgent(Agent).beginReadChain(Statement) line: 240
NetAgent.beginReadChain(Statement) line: 488
NetAgent(Agent).flow(Statement) line: 185
PreparedStatement40(Statement).flowClose() line: 1682
PreparedStatement40(Statement).resetForReuse() line: 334
PreparedStatement40(PreparedStatement).resetForReuse() line: 127
LogicalPreparedStatement40(LogicalStatementEntity).close() line: 175
LogicalPreparedStatement40.close() line: 41
StatementCacheInteractor.closeOpenLogicalStatements() line: 224
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: _sds_0, DERBY-4653-1.diff, DERBY-4653-2.diff, DERBY-4653-3_withrollback.diff, DERBY-4653-4_withcommitrollbacktest.diff, DERBY-4653-5_withflowcommitrollback.diff, DERBY-4653-6_withflowcommitrollback.diff, SaveRoundClientDS.java, SaveRoundClientDS.java
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Posted by "Lily Wei (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Lily Wei updated DERBY-4653:
----------------------------
Attachment: DERBY-4653-6_withflowcommitrollback.diff
I was looking at why StatementPoolingTest.test.doTestResultSetCloseorHoldability failure issue.
When the first time it gets to this signature, conn.rollback() will flow. At that time CachingLoginalConnection40.physicalConnection_.isUnitofWork_ is false which is expected.
However, when it gets to conn.close() which call StatementCacheInteractor.closeOpenLogicalStatements line 224 logicalStatement.close() the variable isUnitofWork_ become true. I am not familiar with closeOpenLogicalStatements synchronized method. Does this only get change in some sort of multi-thread situation?
When StatementPoolingTest get to doTestResultSetCloseorHoldability() the second time, the variable of physicalConnection_.isUnitofWork_ remains false when close are completely done.
What need to change so we can rely on isUnitofWork_ to be the flag to decide whether we can flow rollback or not?
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: _sds_0, DERBY-4653-1.diff, DERBY-4653-2.diff, DERBY-4653-3_withrollback.diff, DERBY-4653-4_withcommitrollbacktest.diff, DERBY-4653-5_withflowcommitrollback.diff, DERBY-4653-6_withflowcommitrollback.diff, SaveRoundClientDS.java, SaveRoundClientDS.java
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-4653) Avoid unnecessary round-trip for
commit in the client driver
Posted by "Kristian Waagan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Kristian Waagan updated DERBY-4653:
-----------------------------------
Attachment: DERBY-4653-7_withflowcommittest_comment_update_diff.txt
I attached an updated version of 'DERBY-4653-7_withflowcommittest_comment_update_diff.txt', with the following changes:
- simplified the reflection code somewhat
- JavaDoc edits
- replaced tabs with spaces and some formatting changes (long lines)
The patch looks okay, I have the following comments:
a) I'm not thrilled by adding the LogicalConnection.getTransactionID method just for testing purposes (when we have another mechanism we could use), but I won't object.
b) The argument 'expectednumtransaction' isn't used.
c) The comment in the actual test is slightly inaccurate. Since the test case is run with autocommit on, the commit will be flowed at rs.close() such that none of the calls to commit() will cause a flow to the server.
I'm +0.9 for commit :)
I'll need to read up on the problems regarding rollback. I suppose most connection pools will issue a rollback rather than a commit to be sure the connection state is "clean", which is why it would be good to find an optimization for rollback as well.
> Avoid unnecessary round-trip for commit in the client driver
> -------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: _sds_0, DERBY-4653-1.diff, DERBY-4653-2.diff, DERBY-4653-3_withrollback.diff, DERBY-4653-4_withcommitrollbacktest.diff, DERBY-4653-5_withflowcommitrollback.diff, DERBY-4653-6_withflowcommitrollback.diff, DERBY-4653-7_withflowcommittest.diff, DERBY-4653-7_withflowcommittest_comment_update_diff.txt, DERBY-4653-7_withflowcommittest_comment_update_diff.txt, ReproTransInProgressAttempt.java, SaveRoundClientDS.java, SaveRoundClientDS.java
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Posted by "Lily Wei (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Lily Wei updated DERBY-4653:
----------------------------
Attachment: SaveRoundClientDS.java
_sds_0
Thanks Dag and Kristian.
I did use ClientDataSource and ClientDataSource.setTraceDirectory to see that the change for save round trip for the commit. I manually reading the trace file "_sds_0" to see the second commit() does not have that many trace information like SEND BUFFER: RDBCMN ... ENDDUOWRM ... Thanks Kathey for helping me understand those complicated message. There is SaveRoundClientDS.java and trace file "_sds_0" in the attachment to show the test code and trace information. To parse the trace file in the test suite, is there something available already? Can someone elaborate on this idea?
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: _sds_0, DERBY-4653-1.diff, DERBY-4653-2.diff, SaveRoundClientDS.java
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-4653) Avoid unnecessary round-trip for
commit in the client driver
Posted by "Kristian Waagan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12881648#action_12881648 ]
Kristian Waagan commented on DERBY-4653:
----------------------------------------
Thanks Kathey and Lily.
I committed the follow-up patch with revision 957164.
Since Connection.setAutoCommit(boolean) may theoretically issue a commit, I moved it up before the call to fetch the transaction id.
> Avoid unnecessary round-trip for commit in the client driver
> -------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: _sds_0, DERBY-4653-1.diff, DERBY-4653-2.diff, DERBY-4653-3_withrollback.diff, DERBY-4653-4_withcommitrollbacktest.diff, DERBY-4653-5_withflowcommitrollback.diff, DERBY-4653-6_withflowcommitrollback.diff, DERBY-4653-7_withflowcommittest.diff, DERBY-4653-7_withflowcommittest_comment_update_diff.txt, DERBY-4653-7_withflowcommittest_comment_update_diff.txt, DERBY-4653-7_withflowcommittest_comment_update_diff_followup.txt, ReproTransInProgressAttempt.java, SaveRoundClientDS.java, SaveRoundClientDS.java
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Posted by "Lily Wei (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Lily Wei updated DERBY-4653:
----------------------------
Attachment: DERBY-4653-1.diff
Other than the two failure tests, suites.AllPackages and derbyall passed with DERBY-4653-1.diff.
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: DERBY-4653-1.diff
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Posted by "Lily Wei (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12871781#action_12871781 ]
Lily Wei commented on DERBY-4653:
---------------------------------
The patch pass suites.Allpackages and derbyall test suites.
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: DERBY-4653-1.diff, DERBY-4653-2.diff
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (DERBY-4653) Avoid unnecessary
round-trip for commit/rollback in the client driver
Posted by "Lily Wei (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12877314#action_12877314 ]
Lily Wei edited comment on DERBY-4653 at 6/11/10 1:17 AM:
----------------------------------------------------------
I was looking at why StatementPoolingTest.test.doTestResultSetCloseorHoldability failure issue.
When the first time it gets to this signature, conn.rollback() will flow. At that time CachingLoginalConnection40.physicalConnection_.inUnitofWork_ is false which is expected.
However, when it gets to conn.close() which call StatementCacheInteractor.closeOpenLogicalStatements line 224 logicalStatement.close() the variable inUnitofWork_ become true. I am not familiar with closeOpenLogicalStatements synchronized method. Does this only get change in some sort of multi-thread situation?
When StatementPoolingTest get to doTestResultSetCloseorHoldability() the second time, the variable of physicalConnection_.inUnitofWork_ remains false when close are completely done.
What need to change so we can rely on inUnitofWork_ to be the flag to decide whether we can flow rollback or not?
was (Author: lilywei):
I was looking at why StatementPoolingTest.test.doTestResultSetCloseorHoldability failure issue.
When the first time it gets to this signature, conn.rollback() will flow. At that time CachingLoginalConnection40.physicalConnection_.isUnitofWork_ is false which is expected.
However, when it gets to conn.close() which call StatementCacheInteractor.closeOpenLogicalStatements line 224 logicalStatement.close() the variable isUnitofWork_ become true. I am not familiar with closeOpenLogicalStatements synchronized method. Does this only get change in some sort of multi-thread situation?
When StatementPoolingTest get to doTestResultSetCloseorHoldability() the second time, the variable of physicalConnection_.isUnitofWork_ remains false when close are completely done.
What need to change so we can rely on isUnitofWork_ to be the flag to decide whether we can flow rollback or not?
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: _sds_0, DERBY-4653-1.diff, DERBY-4653-2.diff, DERBY-4653-3_withrollback.diff, DERBY-4653-4_withcommitrollbacktest.diff, DERBY-4653-5_withflowcommitrollback.diff, DERBY-4653-6_withflowcommitrollback.diff, SaveRoundClientDS.java, SaveRoundClientDS.java
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Posted by "Dag H. Wanvik (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12871866#action_12871866 ]
Dag H. Wanvik commented on DERBY-4653:
--------------------------------------
Maybe you can trace the protocol and check the trace file contents, cf.
http://db.apache.org/derby/docs/10.6/adminguide/cadminappsclienttracing.html
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: DERBY-4653-1.diff, DERBY-4653-2.diff
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Posted by "Lily Wei (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12870236#action_12870236 ]
Lily Wei commented on DERBY-4653:
---------------------------------
Change the code in Connection.flowcommit() to save the round trip
if (!this.inUnitOfWork_)
return;
Run suites.AllPackages and test_04_undefinedAndIllegal and test_04_undefinedAndIllegal fails.
1) test_04_undefinedAndIllegal(org.apache.derbyTesting.functionTests.tests.lang.Bo
oleanValuesTest)junit.framework.ComparisonFailure: getBoolean() on BLOB_COL expect
ed:<...2005> but was:<...4000>
at org.apache.derbyTesting.junit.BaseJDBCTestCase.assertSQLState(BaseJDBCT
estCase.java:762)
at org.apache.derbyTesting.functionTests.tests.lang.BooleanValuesTest.vet_
getBooleanException(BooleanValuesTest.java:430)
at org.apache.derbyTesting.functionTests.tests.lang.BooleanValuesTest.vet_
getBooleanIsIllegal(BooleanValuesTest.java:410)
at org.apache.derbyTesting.functionTests.tests.lang.BooleanValuesTest.test
_04_undefinedAndIllegal(BooleanValuesTest.java:332)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.ja
va:39)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccesso
rImpl.java:25)
at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:10
9)
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)
Caused by: java.sql.SQLException: Invalid cursor state - no current row.
However, these two tests fail when I don't have Connect.flowcommit() change either.
On BooleanTestsValue.java,
public void test_04_undefinedAndIllegal() throws Exception
{
Connection conn = getConnection();
vet_getBooleanIsIllegal( conn, "BLOB_COL" ); <<<====This is the failure comes from
I will keep looking to see why these two tests failed with or without Connection.flowcommit(() change.
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (DERBY-4653) Avoid unnecessary
round-trip for commit/rollback in the client driver
Posted by "Kathey Marsden (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12876902#action_12876902 ]
Kathey Marsden edited comment on DERBY-4653 at 6/8/10 8:38 PM:
---------------------------------------------------------------
wrt to some of Kristian's comments and a few things I talked to Lily about today.
I too think it would be good to test just for client and with regular, pooled, and xa datasources.
I think if the test runs just for client we should always count on the getTransactionID() method being there and can take out the skipping logic all together. I think the system table query can come out all together. I tend to think the getTransactionID() check is good enough, but the protocol trace parsing would be ok too.
I agree the comment about embedded should come out of am.LogicalConnection.
For the flowRollback() change I think the condition to return if not inUnitOfWork should not be in the if block. I think the StatementPoolingTest failures you were seeing with it set that way may be because the state of inUnitOfWork may be wrong somehow where hold cursors and pooled connections are involved, probably an existing issue that needs to be tracked down before committing this change. Lily and I tried to reproduce it outside the test with the code but we were not quite able to get in the same state as StatementPoolingTest.
was (Author: kmarsden):
wrt to some of Kristian's comments and a few things I talked to Lily about today.
I too think it would be good to test just for client and with regular, pooled, and xa datasources.
I think if the test runs just for client we should always count on the getTransactionID() method being there and can take out the skipping logic all together. I think the system table query can come out all together. I tend to think the getTransactionID() check is good enough, but the protocol trace parsing would be ok too.
I agree the comment about embedded should come out of am.LogicalConnection.
For the flowRollback() change I think the condition to return if inUnitOfWork should not be in the if block. I think the StatementPoolingTest failures you were seeing with it set that way may be because the state of inUnitOfWork may be wrong somehow where hold cursors and pooled connections are involved, probably an existing issue that needs to be tracked down before committing this change. Lily and I tried to reproduce it outside the test with the code but we were not quite able to get in the same state as StatementPoolingTest.
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: _sds_0, DERBY-4653-1.diff, DERBY-4653-2.diff, DERBY-4653-3_withrollback.diff, DERBY-4653-4_withcommitrollbacktest.diff, SaveRoundClientDS.java, SaveRoundClientDS.java
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-4653) Avoid unnecessary round-trip for
commit/rollback in the client driver
Posted by "Kristian Waagan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4653?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12871847#action_12871847 ]
Kristian Waagan commented on DERBY-4653:
----------------------------------------
Lily wrote:
-----
However, if the behavior go back to the round-trip scenario, the test has no way knowing other than functionality is working. Does anyone has a good suggest to test the DRDA protocol for commit making round trip or not?
-----
A few suggestions (none of them really optimal):
- add different trace points on the client for the cases where a commit is flowed or not, then parse the trace file
(this change could actually help debugging in production environments)
- enable statement logging on the server and parse the log looking for the number of commits (assuming an "unnecessary" commit produces a log message)
- some kind of timed test doing as many commits as possible (hard to set the thresholds, probably not a viable solution?)
- add special debug code, or exploit poor state encapsulation in the client driver
(for instance, can the DRDA "sequence number" be used, or more likely, maybe the client side "transaction id"?)
Another related task would be to inspect the code to make sure the variable 'inUnitOfWork_' is in sync with reality.
> Avoid unnecessary round-trip for commit/rollback in the client driver
> ---------------------------------------------------------------------
>
> Key: DERBY-4653
> URL: https://issues.apache.org/jira/browse/DERBY-4653
> Project: Derby
> Issue Type: Improvement
> Components: JDBC, Network Client
> Affects Versions: 10.7.0.0
> Reporter: Kristian Waagan
> Assignee: Lily Wei
> Priority: Minor
> Attachments: DERBY-4653-1.diff, DERBY-4653-2.diff
>
>
> The methods Connection.commit() and Connection.rollback() in the client driver cause a round-trip to the server even if the commit/rollback is unnecessary (i.e. there is nothing to commit or roll back).
> Comments suggest (see below) that this can be optimized, such that the commands are flowed to the server only when required. It can be seen that this optimization has been used other places in the client driver. Never the less, it must be checked that this optimization doesn't have side-effects.
> This issue came up in connection with connection pooling, where a pool implementation always issued a rollback to make sure there was no active transaction on the connection handed out.
> From Connection.flowCommit:
> // Per JDBC specification (see javadoc for Connection.commit()):
> // "This method should be used only when auto-commit mode has been disabled."
> // However, some applications do this anyway, it is harmless, so
> // if they ask to commit, we could go ahead and flow a commit.
> // But note that rollback() is less harmless, rollback() shouldn't be used in auto-commit mode.
> // This behavior is subject to further review.
> // if (!this.inUnitOfWork)
> // return;
> // We won't try to be "too smart", if the user requests a commit, we'll flow a commit,
> // regardless of whether or not we're in a unit of work or in auto-commit mode.
> //
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.