-
1. Re: Transactional support for JAX RS based applications
mmusgrov Mar 8, 2012 12:44 PM (in response to radeshrao)1 of 1 people found this helpfulHi, it is great that you are interested in this project.
I believe anyone is at liberty to raise a JIRA against the code but it is probably best to get them discussed on these forums first.
It is still very much an active project, the lack of activity is more indicative of priorities rather than lack of interest. What issues have you found so far?
-
2. Re: Transactional support for JAX RS based applications
radeshrao Mar 8, 2012 1:24 PM (in response to mmusgrov)That's great!
I did find few issues. I'll post them in individual posts for clarity.
java.util.ConcurrentModificationException
at java.util.HashMap$HashIterator.nextEntry(HashMap.java:793)
at java.util.HashMap$KeyIterator.next(HashMap.java:828)
at org.jboss.jbossts.star.service.Coordinator.terminateTransaction(Coordinator.java:331)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at java.lang.reflect.Method.invoke(Method.java:597)
The iteration over participants in the terminateTransaction is not wrapped in a synchronized block
for (Iterator<String> i = participants.keySet().iterator(); i.hasNext(); )
if (enlistmentIds.contains(i.next()))
i.remove();
-
3. Re: Transactional support for JAX RS based applications
radeshrao Mar 8, 2012 1:52 PM (in response to mmusgrov)When transactions timeout (when the client abandons them or errors out) the org.jboss.jbossts.star.service.Coordinator holds references to the org.jboss.jbossts.star.resource.Transaction in the transactions Map. So eventually the Coordinator goes OOM.
To test it out I added a listener using TransactionReaper.transactionReaper() on startup and clearing the timed out transactions from the Map and it seemed to work.
Also while I was testing this found that TxSupport.getTransactions() returns a List of size 1 when there were 0 transactions. The content.split on an empty string in getTransactions() returns and array with 1 "" element.
-
4. Re: Transactional support for JAX RS based applications
radeshrao Mar 8, 2012 2:01 PM (in response to mmusgrov)I see this exception when there are large number of transactions in progress:
Caused by: org.jboss.jbossts.star.provider.HttpResponseException: java.net.SocketException: No buffer space available (maximum connections reached?): connect
at org.jboss.jbossts.star.util.TxSupport.httpRequest(TxSupport.java:261)
at org.jboss.jbossts.star.util.TxSupport.startTx(TxSupport.java:179)
...............................
Caused by: java.net.SocketException: No buffer space available (maximum connections reached?): connect
at java.net.PlainSocketImpl.socketConnect(Native Method)
at java.net.PlainSocketImpl.doConnect(PlainSocketImpl.java:351)
at java.net.PlainSocketImpl.connectToAddress(PlainSocketImpl.java:213)
at java.net.PlainSocketImpl.connect(PlainSocketImpl.java:200)
at java.net.SocksSocketImpl.connect(SocksSocketImpl.java:366)
at java.net.Socket.connect(Socket.java:529)
at java.net.Socket.connect(Socket.java:478)
Closing streams in TxSupport getContent() and openConnection() methods seems to fix this one.
-
5. Re: Transactional support for JAX RS based applications
mmusgrov Mar 8, 2012 2:47 PM (in response to radeshrao)Interesting. I declared the map as a synchronisedMap but this is not sufficient to protect against concurrent modification whilst iterating. The latest codebase now supports JDK 6 so I think it is better if we go for a ConcurrentHashMap rather than explicit sychronisation:
Try replacing the declaration of participants with:
participants = new ConcurrentHashMap<String, LinkHolder> ();
And the removal operation (during iteration) in terminateTransaction with:
for (String enlistmentId : enlistmentIds)
participants.remove(enlistmentId);
which is guaranteed to be thread safe (and does nothing if the map does not contain enlistmentId). I will try it out myself too tomorrow.
Thanks again for spotting these problems,
Regards, Mike
-
6. Re: Transactional support for JAX RS based applications
mmusgrov Mar 8, 2012 2:49 PM (in response to radeshrao)Good catch and fix
-
7. Re: Transactional support for JAX RS based applications
mmusgrov Mar 8, 2012 2:52 PM (in response to radeshrao)Radesh Rao wrote:
When transactions timeout (when the client abandons them or errors out) the org.jboss.jbossts.star.service.Coordinator holds references to the org.jboss.jbossts.star.resource.Transaction in the transactions Map. So eventually the Coordinator goes OOM.
To test it out I added a listener using TransactionReaper.transactionReaper() on startup and clearing the timed out transactions from the Map and it seemed to work.
Also while I was testing this found that TxSupport.getTransactions() returns a List of size 1 when there were 0 transactions. The content.split on an empty string in getTransactions() returns and array with 1 "" element.
This is superb Radesh, keep them comming. You have certainly exposed some issues.
-
8. Re: Transactional support for JAX RS based applications
radeshrao Mar 8, 2012 3:36 PM (in response to mmusgrov)Michael Musgrove wrote:
Interesting. I declared the map as a synchronisedMap but this is not sufficient to protect against concurrent modification whilst iterating. The latest codebase now supports JDK 6 so I think it is better if we go for a ConcurrentHashMap rather than explicit sychronisation:
Try replacing the declaration of participants with:
participants = new ConcurrentHashMap<String, LinkHolder> ();
And the removal operation (during iteration) in terminateTransaction with:
for (String enlistmentId : enlistmentIds)
participants.remove(enlistmentId);
which is guaranteed to be thread safe (and does nothing if the map does not contain enlistmentId). I will try it out myself too tomorrow.
Thanks again for spotting these problems,
Regards, Mike
I just tested this out and it worked.
The transactions Map is also iterated over in getAllTransactions while new transactions are being added to it.
-
9. Re: Transactional support for JAX RS based applications
radeshrao Mar 8, 2012 4:31 PM (in response to mmusgrov)Not sure if this is correct or important but Coordinator.replaceParticipant does not copy over the txId to the replaced participant. I found this when trying out my changes to the OOM issue and one of the unit tests in CoordinatorTest (or maybe it was SpecTest not sure) failed.
-
10. Re: Transactional support for JAX RS based applications
mmusgrov Mar 12, 2012 7:39 AM (in response to radeshrao)Hi Radesh, I see three ways we can get these issues fixed and integrated. You decide how you'd like to proceed:
1) Get an account on JIRA (our bug tracking tool), sign the JBoss contributor agreement, raise JIRAs for each issue, go to github and fork our project, fix the issues in a branch and raise a pull request. I will then merge in your fixes into the master branch
2) Get a JIRA account raise JIRAs. Attach patches for each issue and I will apply them to the master branch
3) Get an account on JIRA and raise JIRAs and say how you would fix the issues (like you did on the forum post). I will then implement your fixes in the master branch
I can identify four JIRAs for the issues you have raised:
1: The coordinator can throw a ConcurrentModificationException during transaction termination
2: The coordinator does not flush its cache of transactions as they timeout eventually resulting in OOM
3. TxSupport helper method getTransactions() returns 1 when there are no transactions
4: TxSupport helper getContent() and openConnection() leaves I/O streams open
-
11. Re: Transactional support for JAX RS based applications
radeshrao Mar 12, 2012 2:48 PM (in response to mmusgrov)Hi Michael,
Michael Musgrove wrote:
Hi Radesh, I see three ways we can get these issues fixed and integrated. You decide how you'd like to proceed:
1) Get an account on JIRA (our bug tracking tool), sign the JBoss contributor agreement, raise JIRAs for each issue, go to github and fork our project, fix the issues in a branch and raise a pull request. I will then merge in your fixes into the master branch
2) Get a JIRA account raise JIRAs. Attach patches for each issue and I will apply them to the master branch
3) Get an account on JIRA and raise JIRAs and say how you would fix the issues (like you did on the forum post). I will then implement your fixes in the master branch
(1) sounds alright to me. Let me start by creating the JIRA tickets. I haven't worked a lot on github (or git) but I looked at the https://community.jboss.org/wiki/HackingOnAS7 and I'd like to give this a shot.
-
12. Re: Transactional support for JAX RS based applications
radeshrao Mar 12, 2012 3:04 PM (in response to mmusgrov)Michael Musgrove wrote:
I can identify four JIRAs for the issues you have raised:
1: The coordinator can throw a ConcurrentModificationException during transaction termination
2: The coordinator does not flush its cache of transactions as they timeout eventually resulting in OOM
3. TxSupport helper method getTransactions() returns 1 when there are no transactions
4: TxSupport helper getContent() and openConnection() leaves I/O streams open
I have created the JIRA tickets. Unit test for (3) is trivial, not sure about consistently repeatable tests for the rest. Any advice?
-
13. Re: Transactional support for JAX RS based applications
mmusgrov Mar 14, 2012 6:34 AM (in response to radeshrao)Radesh Rao wrote:
Michael Musgrove wrote:
I can identify four JIRAs for the issues you have raised:
1: The coordinator can throw a ConcurrentModificationException during transaction termination
2: The coordinator does not flush its cache of transactions as they timeout eventually resulting in OOM
3. TxSupport helper method getTransactions() returns 1 when there are no transactions
4: TxSupport helper getContent() and openConnection() leaves I/O streams open
I have created the JIRA tickets. Unit test for (3) is trivial, not sure about consistently repeatable tests for the rest. Any advice?
I agree its not easy to provide unit tests for 1 and 4, ideally the problem would fall under the remit of system tests . We could do something in our qa test suite but to be honnest I would just fix it and indicate in the JIRA how you generated the failure conditions.
Problem 2 depends on how you fix it. I discussed it with Jonathan and he recommends using an after completion synchronisation to do the clean up. Although after completion synchronisations are advisory this will work with the current implementation since the transaction never flows into other transaction coordinators.
So the unit test for 2 is to start a txn with timeout. Wait for the timeout period and check that the transaction has gone. The behavour in this case is covered by the following quote from the spec:
If the transaction is terminated because of a timeout, the resources representing the created
transaction are deleted. All further invocations on the transaction-coordinator or any of its related
URIs MAY return 410 if the implementation records information about transactions that have
rolled back, (not necessary for presumed rollback semantics) but at a minimum MUST return 404.
-
14. Re: Transactional support for JAX RS based applications
radeshrao Mar 14, 2012 11:00 AM (in response to mmusgrov)Michael Musgrove wrote:
Hi Radesh, I see three ways we can get these issues fixed and integrated. You decide how you'd like to proceed:
1) Get an account on JIRA (our bug tracking tool), sign the JBoss contributor agreement, raise JIRAs for each issue, go to github and fork our project, fix the issues in a branch and raise a pull request. I will then merge in your fixes into the master branch
Is https://cla.jboss.org/index.seam where I sign the Contributer license?