[
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.
