db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kathey Marsden <kmarsdende...@sbcglobal.net>
Subject Re: [jira] Commented: (DERBY-339) Network client XA should only keep XA state for transaction branch association, to track whether to send commit in autocommit mode. All other state and state related decisions should be deferred to the server.
Date Tue, 07 Jun 2005 20:21:30 GMT
Mamta Satoor wrote:

> Hi Kathey,
>
> First of all, let me admit, I am no XA expert but here are my comments
> on the review package anyways.(I used the patch attached to the Jira.
> That's the latest patch, correct?)
>
You know This sounds like an old patch.  I must have attached the wrong
thing to Jira somehow.  I will repost a new patch to Jira and delete
this one. Below comments for your issues.

> 1)I noticed that you added a new sql state to SqlState.java as follows 

> 2)SqlState.java has following comment
>
> 3)ClientXAConnection::checkXANotAssociated() is new to this patch but
> call to this method in the same java class from getConnection() has
> been commented out. This change must be for Derby-341 and looks like
> that is why it is commented out.

1-3 were part of a potential patch for 341 which should not be in this
patch.

> 4)The patch has new file xaSimplePositive_derby.properties but
> everything in that file is commented out. Is it a place holder for
> future properties? Also, I am not too fluent with svn, but I don't see
> svn add for that file in the patch? Maybe committer of the patch needs
> to be in manually?
>
Yes, Just useful for debugging to have a derby properties for this test
that I left it there for future use.

> 5)Maybe this a silly question, but why has the code to associate the
> connection to Connection.XA_T0_NOT_ASSOCIATED been moved from
> NetXAResource::commit() to NetXAResource::end()?
>
No not silly at all. It  is in end in the latest patch.

> 6)In NetXAResource::throwXAException(int rc), it looks like following
> is added but it looks incomplete since there is nothing inside switch(rc)
> +        // Set transaction association appropriately for the error.
> +        switch (rc){
> +        
> +  }
>
> 7)In NetXAResource, a new method by name setXaStateForXAException(int
> rc) has been added but I don't see any call to this method.

will be resolved in new patch.

>  
> 8)There is no new test for this patch but that may be because the code
> is cleaning up existing code and not adding/changing any existing
> functionality.
>

The changes were made in an effort to bring the xaSimplePositive test
into the Network Server test suite.  Even with these changes there is
still one issue with the the test that has to be addressed before I can
add it to the suite.  But that test now gets through all but the last
case which fails with a failure setting holdability.


Kathey



Mime
View raw message