geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dain Sundstrom <>
Subject Re: [Review] GERONIMO-2277 Remove TransactionContextManager
Date Sun, 06 Aug 2006 04:11:14 GMT
I merged all changes from head back into notcm and fixed the m2  
build.  I also made a few changes noted below based on requests for  
David.  You I suggest you test the notcm branch directly.  If you  
want to try the merge, use the same command as before, and you'll  
have to resolve some conflicts (I have no idea why svn marks them as  
conflicts).  Here are the commands I used:

svn co geronimo
cd geronimo

svn merge -r 427246:HEAD 
branches/dain/notcm .

# resolve the conflicts by taking the files from notcm
mv bootstrap.merge-right* \
svn resolved bootstrap
chmod u+x bootstrap

mv pom.xml.merge-right* \
svn resolved pom.xml

mv m2-assemblies/geronimo-boilerplate-j2ee/src/main/resources/var/ 
security/keystores/geronimo-default.merge-right* \
svn resolved m2-assemblies/geronimo-boilerplate-j2ee/src/main/ 

mv m2-assemblies/geronimo-boilerplate-minimal/src/main/resources/var/ 
security/keystores/geronimo-default.merge-right* \
svn resolved m2-assemblies/geronimo-boilerplate-minimal/src/main/ 

# run the m2 build or run the m1 build
mvn clean install

# maven new

Sorry about the long instructions, but svn screwed me. more comments  

On Aug 4, 2006, at 5:40 PM, David Jencks wrote:

> I have a couple of concerns: most of these were probably present in  
> the unmodified code.
> 1. TransactionManagerImpl.unassociate calls fireThreadAssociated  
> instead of fireThreadUnassociated (dain is fixing this IIUC)


> 2. ConnectorTransactionContext.managedConnections doesn't have any  
> obvious synchronization.  When I wrote this code I think I had an  
> argument why it wasn't needed, but I'd like to find the object that  
> is in fact guarding it and document it.

I synchronized access.  It the data is effectively transaction local,  
and only thread can be associated with a tx at a time. But it is  
safer to synchronize.  It shouldn't hurt performance since there will  
be no contention.

> 3. Am I correct in thinking that you made the connection tracking  
> work without needing instance contexts available?  I think this is  
> a reasonable change, just

Based on my reading of the code, conection tracking need to be  
notified of context changes.  If it switches to a context without  
connector tx data, the tracker has nothing to do.

> 4. I wonder if the client still needs a tm since it is inaccessible.

I don't think it is requires, but we may want to start an  
unrecoverable one anyway.


View raw message