Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JBTM-1479. JTA, IronJacamar, and Tomcat quickstart #74

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
5 participants
@gytis
Copy link
Member

commented Apr 22, 2013

No description provided.

@jbosstm-bot

This comment has been minimized.

Copy link

commented Apr 22, 2013

@jbosstm-bot

This comment has been minimized.

Copy link

commented Apr 22, 2013

Tests Failed

@gytis

This comment has been minimized.

Copy link
Member Author

commented Apr 22, 2013

REST-AT test failed

@jbosstm-bot

This comment has been minimized.

Copy link

commented Apr 22, 2013

@jbosstm-bot

This comment has been minimized.

Copy link

commented Apr 22, 2013

Tests Failed

@gytis

This comment has been minimized.

Copy link
Member Author

commented Apr 22, 2013

@jbosstm-bot

This comment has been minimized.

Copy link

commented Apr 24, 2013

@jbosstm-bot

This comment has been minimized.

Copy link

commented Apr 24, 2013

Tests Failed

@jbosstm-bot

This comment has been minimized.

Copy link

commented Apr 24, 2013

@jbosstm-bot

This comment has been minimized.

Copy link

commented Apr 24, 2013

Tests Passed

@jbosstm-bot

This comment has been minimized.

Copy link

commented Apr 25, 2013

@jbosstm-bot

This comment has been minimized.

Copy link

commented Apr 25, 2013

Tests Passed

@jbosstm-bot

This comment has been minimized.

Copy link

commented Apr 25, 2013

@jbosstm-bot

This comment has been minimized.

Copy link

commented Apr 25, 2013

Tests Failed

@gytis

This comment has been minimized.

Copy link
Member Author

commented Apr 25, 2013

IndirectTXManagementTest failed. REST-AT coordinator missing

@mmusgrov

This comment has been minimized.

Copy link
Contributor

commented Apr 25, 2013

The quickstarts expect http://172.17.131.2/view/Narayana+BlackTie/job/narayana/TESTS=MAIN,jdk=jdk7.latest,label=linux/lastSuccessfulBuild to exist. So we should make sure the quickstart job depends on this particular narayana job

@jbosstm-bot

This comment has been minimized.

Copy link

commented Apr 25, 2013

@jbosstm-bot

This comment has been minimized.

Copy link

commented Apr 25, 2013

Tests Failed

@tomjenkinson

This comment has been minimized.

Copy link
Contributor

commented Apr 25, 2013

@mmusgrov and @gytis , @paulrobinson is looking into the issue where the multi job config is no longer archiving the last successful build. I don't think adding the dependency is necessary but we do need to resolve it so narayana starts archiving the last successul build. I asked Paul if maybe using the jenkins publisher would work. @paulrobinson perhaps you can raise a jira so that it raises the visibility of the current issue?

@paulrobinson

This comment has been minimized.

Copy link
Contributor

commented Apr 25, 2013

@gytis

This comment has been minimized.

Copy link
Member Author

commented Apr 25, 2013

Moved IronJacamar quickstart module to the top in parent's pom.xml

@jbosstm-bot

This comment has been minimized.

Copy link

commented Apr 25, 2013

@jbosstm-bot

This comment has been minimized.

Copy link

commented Apr 25, 2013

Tests Failed

@jbosstm-bot

This comment has been minimized.

Copy link

commented Apr 25, 2013

@jbosstm-bot

This comment has been minimized.

Copy link

commented Apr 25, 2013

Tests Failed

@paulrobinson

This comment has been minimized.

Copy link
Contributor

commented Apr 25, 2013

I think I've fixed JBTM-1653, but this PR seems to be failing for another reason. Can you take a look? Let me know if you think it's related to JBTM-1653 or maybe the WildFly renaming.

@gytis

This comment has been minimized.

Copy link
Member Author

commented Apr 25, 2013

It is not related to JBTM-1653. Fungal kernel cannot delete a file on Windows and therefore cannot shutdown and restart the JCA container properly.

@jbosstm-bot

This comment has been minimized.

Copy link

commented May 30, 2013

@jbosstm-bot

This comment has been minimized.

Copy link

commented May 30, 2013

Tests Failed

@gytis

This comment has been minimized.

Copy link
Member Author

commented May 30, 2013

REST-AT quickstart failed. Tests for this pull request were successful.

@gytis

This comment has been minimized.

Copy link
Member Author

commented Jun 7, 2013

Updated readme

@jbosstm-bot

This comment has been minimized.

Copy link

commented Jun 7, 2013

@jbosstm-bot

This comment has been minimized.

Copy link

commented Jun 7, 2013

Tests Failed

@gytis

This comment has been minimized.

Copy link
Member Author

commented Jun 10, 2013

Required tests passed

$TOMCAT_HOME/bin/catalina.sh run

Steps 3 will register Byteman rule which will kill Tomcat server after you will try to add customer in step 4. It will lead to the uncompleted two phase commit protocol.
After you will restart Tomcat in step 5, recovery system will kick in and finish the transaction. You will see the TRACE logging in Tomcat console. New customer entry should be available in around 30 seconds after restart i.e. after second recovery cycle. You can find customers list here: <http://localhost:8080/jca-and-tomcat/customers.xhtml>

This comment has been minimized.

Copy link
@tomjenkinson

tomjenkinson Jun 10, 2013

Contributor

The new customer is actually visible about 2 minutes after the server starts. The recovery cycle is every 2 minutes. I think we need to update that text as it causes uncertainty. It might be worth including the first warning message that the customer will see too:

2013-06-10 11:35:30,497 [Periodic Recovery] WARN com.arjuna.ats.jta - ARJUNA016037: Could not find new XAResource to use for recovering non-serializable XAResource XAResourceRecord < resource:null, txid:< formatId=131077, gtrid_length=29, bqual_length=36, tx_uid=0:ffff7f000001:94bc:51b5ab83:10, node_name=1, branch_uid=0:ffff7f000001:94bc:51b5ab83:15, subordinatenodename=null, eis_name=0 >, heuristic: TwoPhaseOutcome.FINISH_OK com.arjuna.ats.internal.jta.resources.arjunacore.XAResourceRecord@253de8da >
2013-06-10 11:35:30,498 [Periodic Recovery] TRACE org.jboss.narayana.quickstart.jca.xa.DummyXAResource - DummyXAResource.commit(xid=< formatId=131077, gtrid_length=29, bqual_length=36, tx_uid=0:ffff7f000001:94bc:51b5ab83:10, node_name=1, branch_uid=0:ffff7f000001:94bc:51b5ab83:11, subordinatenodename=null, eis_name=0 >, onePhase=false)
2013-06-10 11:35:30,499 [Periodic Recovery] WARN com.arjuna.ats.jta - ARJUNA016038: No XAResource to recover < formatId=131077, gtrid_length=29, bqual_length=36, tx_uid=0:ffff7f000001:94bc:51b5ab83:10, node_name=1, branch_uid=0:ffff7f000001:94bc:51b5ab83:15, subordinatenodename=null, eis_name=0 >

@gytis

This comment has been minimized.

Copy link
Member Author

commented Jun 10, 2013

I've changed recovery period to 30 seconds in transaction.xml just not to make user to wait too long.

@gytis

This comment has been minimized.

Copy link
Member Author

commented Jun 10, 2013

I'll add the warnings to the readme

@tomjenkinson

This comment has been minimized.

Copy link
Contributor

commented Jun 10, 2013

hmm, either I didn't follow the readme correctly or that setting didn't take effect for me as I definitely had to wait 120 seconds after the first "failure to find XAResource" message....

@jbosstm-bot

This comment has been minimized.

Copy link

commented Jun 10, 2013

@jbosstm-bot

This comment has been minimized.

Copy link

commented Jun 10, 2013

@jbosstm-bot

This comment has been minimized.

Copy link

commented Jun 10, 2013

Tests Failed

@gytis

This comment has been minimized.

Copy link
Member Author

commented Jun 10, 2013

Required tests passed

LOG.trace("CustomerManager.addCustomer(name=" + name + ")");
}

beginTransaction();

This comment has been minimized.

Copy link
@tomjenkinson

tomjenkinson Jun 10, 2013

Contributor

Personally, I feel that putting this code in a beginTransaction method spoils the readability of the code, especially as beginTransaction is only called from here, same for commitTransaction and rollbackTransaction, wdyt?

This comment has been minimized.

Copy link
@gytis

gytis Jun 10, 2013

Author Member

My intention was to separate tasks of each method and make them compact. At least I have read it in a couple of books that it's a good practice not to make methods too chunky. beginTransaction, commitTransaction, and rollbackTransaction does multiple tasks (get transaction manager, start/finish transaction, enlist dummy participant, catch exception) which are not directly related to adding customer to the database.

This comment has been minimized.

Copy link
@tomjenkinson

tomjenkinson Jun 10, 2013

Contributor

I think it would aid readability to do these inline, perhaps you could have a utility method to get the transaction, but the begin/commit/rollback would read easier inline. Also, I think those methods should throw an exception if something goes wrong (the exception from the transactionManager is fine) rather than just log a warning.

@@ -0,0 +1,41 @@
#

This comment has been minimized.

Copy link
@tomjenkinson

tomjenkinson Jun 10, 2013

Contributor

is this better in src/test/resources?

This comment has been minimized.

Copy link
@gytis

gytis Jun 10, 2013

Author Member

I didn't add it to the test, because it isn't used by any of the test classes. Do you think it belongs in there?

This comment has been minimized.

Copy link
@tomjenkinson

tomjenkinson Jun 10, 2013

Contributor

I think it could be better as it is a test to illustrate the behaviour. it is subjective though but logically it feels more like a test than part of the app

$TOMCAT_HOME/bin/catalina.sh run

Steps 3 will register Byteman rule which will kill Tomcat server after you will try to add customer in step 4. It will lead to the uncompleted two phase commit protocol.
After you will restart Tomcat in step 5, recovery system will kick in and finish the transaction. You will see the TRACE logging in Tomcat console. New customer entry should be available in around 40 seconds after restart i.e. after second recovery cycle. You can find customers list here: <http://localhost:8080/jca-and-tomcat/customers.xhtml>

This comment has been minimized.

Copy link
@tomjenkinson

tomjenkinson Jun 10, 2013

Contributor

you have used the word trace here, when they are warnings

This comment has been minimized.

Copy link
@gytis

gytis Jun 10, 2013

Author Member

Dummy participant will write trace logs once its 2PC methods are invoked. Those warnings should not be in there at all, so they are mentioned in separate notice

This comment has been minimized.

Copy link
@tomjenkinson

tomjenkinson Jun 10, 2013

Contributor

I see now, maybe I was too familiar with the quickstart and overlooked the separation, feel feel to leave the wording of this as trace (unless you promote that logging to INFO - see below)


You will be presented with a simple form for adding customers to a database.

When you enter a name and click "Add" that customer, you will see the TRACE level logging in the Tomcat console. In the browser you will see the list of customer added so far and number of successfully commited transactions since the server was started.

This comment has been minimized.

Copy link
@tomjenkinson

tomjenkinson Jun 10, 2013

Contributor

I am not sure what is being illustrated by outputting the test code at trace level? If its not too important I would disable this (i.e. remove the log4jproperties). The example then has the benefit that the src/main can be used as is as almost an archetype should someone want to build an app on top of it.

This comment has been minimized.

Copy link
@gytis

gytis Jun 10, 2013

Author Member

But most of the quickstarts write some sort of the logs to show to the user that something is happening there

This comment has been minimized.

Copy link
@tomjenkinson

tomjenkinson Jun 10, 2013

Contributor

Maybe you can output at INFO level then? if it is pertinent to the quickstart?


public void commit(Xid xid, boolean onePhase) throws XAException {
if (LOG.isTraceEnabled()) {
LOG.trace("DummyXAResource.commit(xid=" + xid + ", onePhase=" + onePhase + ")");

This comment has been minimized.

Copy link
@tomjenkinson

tomjenkinson Jun 10, 2013

Contributor

Can be info


public int prepare(Xid xid) throws XAException {
if (LOG.isTraceEnabled()) {
LOG.trace("DummyXAResource.prepare(xid=" + xid + ")");

This comment has been minimized.

Copy link
@tomjenkinson

tomjenkinson Jun 10, 2013

Contributor

can be info


public void rollback(Xid xid) throws XAException {
if (LOG.isTraceEnabled()) {
LOG.trace("DummyXAResource.rollback(xid=" + xid + ")");

This comment has been minimized.

Copy link
@tomjenkinson

tomjenkinson Jun 10, 2013

Contributor

can be info


public Xid[] recover(int flag) throws XAException {
if (LOG.isTraceEnabled()) {
LOG.trace("DummyXAResource.recover(flag=" + flag + ")");

This comment has been minimized.

Copy link
@tomjenkinson

tomjenkinson Jun 10, 2013

Contributor

can be info

@Override
public void contextInitialized(final ServletContextEvent servletContextEvent) {
if (LOG.isTraceEnabled()) {
LOG.trace("ServletContextListenerImpl.contextInitialized()");

This comment has been minimized.

Copy link
@tomjenkinson

tomjenkinson Jun 10, 2013

Contributor

can be info

@Override
public void contextDestroyed(final ServletContextEvent servletContextEvent) {
if (LOG.isTraceEnabled()) {
LOG.trace("ServletContextListenerImpl.contextDestroyed()");

This comment has been minimized.

Copy link
@tomjenkinson

tomjenkinson Jun 10, 2013

Contributor

can be info

@jbosstm-bot

This comment has been minimized.

Copy link

commented Jun 10, 2013

@jbosstm-bot

This comment has been minimized.

Copy link

commented Jun 10, 2013

Tests Failed

@jbosstm-bot

This comment has been minimized.

Copy link

commented Jun 10, 2013

@jbosstm-bot

This comment has been minimized.

Copy link

commented Jun 10, 2013

Tests Failed

@tomjenkinson

This comment has been minimized.

Copy link
Contributor

commented Jun 10, 2013

merged

@gytis gytis deleted the gytis:master-JBTM-1479 branch Aug 29, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.