activemq-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From tabish121 <...@git.apache.org>
Subject [GitHub] activemq-artemis pull request #1102: Two Performance and TX changes
Date Fri, 17 Mar 2017 20:29:04 GMT
Github user tabish121 commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1102#discussion_r106739208
  
    --- Diff: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/transaction/ProtonTransactionHandler.java
---
    @@ -78,44 +82,68 @@ public void onMessage(Delivery delivery) throws ActiveMQAMQPException
{
                 Binary txID = sessionSPI.newTransaction();
                 Declared declared = new Declared();
                 declared.setTxnId(txID);
    -            delivery.disposition(declared);
    +            synchronized (connection.getLock()) {
    +               delivery.disposition(declared);
    +            }
              } else if (action instanceof Discharge) {
                 Discharge discharge = (Discharge) action;
     
                 Binary txID = discharge.getTxnId();
    -            sessionSPI.dischargeTx(txID);
    +            ProtonTransactionImpl tx = (ProtonTransactionImpl)sessionSPI.getTransaction(txID,
true);
    +            tx.discharge();
    +
                 if (discharge.getFail()) {
                    try {
    -                  sessionSPI.rollbackTX(txID, true);
    -                  delivery.disposition(new Accepted());
    +                  tx.rollback();
    +                  synchronized (connection.getLock()) {
    +                     delivery.disposition(new Accepted());
    +                  }
    +                  connection.flush();
                    } catch (Exception e) {
    +                  log.warn(e.getMessage(), e);
                       throw ActiveMQAMQPProtocolMessageBundle.BUNDLE.errorRollingbackCoordinator(e.getMessage());
                    }
                 } else {
                    try {
    -                  sessionSPI.commitTX(txID);
    -                  delivery.disposition(new Accepted());
    +                  tx.commit();
    +                  synchronized (connection.getLock()) {
    +                     delivery.disposition(new Accepted());
    +                  }
    +                  connection.flush();
                    } catch (ActiveMQAMQPException amqpE) {
    +                  log.warn(amqpE.getMessage(), amqpE);
                       throw amqpE;
                    } catch (Exception e) {
    +                  log.warn(e.getMessage(), e);
                       throw ActiveMQAMQPProtocolMessageBundle.BUNDLE.errorCommittingCoordinator(e.getMessage());
                    }
                 }
     
                 // Replenish coordinator receiver credit on exhaustion so sender can continue
                 // transaction declare and discahrge operations.
    -            if (receiver.getCredit() == 0) {
    -               receiver.flow(DEFAULT_COORDINATOR_CREDIT);
    +            synchronized (connection.getLock()) {
    --- End diff --
    
    I was going to submit a change here but since you've modified things I'll just see if
we can get this into this commit.  I inadvertently put this into the handler block for discharge
but really it should be checked in either case.  Also it makes sense to move this up to top
after the receiver reference is obtained so that credit is granted before any work is done
that might throw an exception and cause the check to be missed.  Can possibly do it in the
fix block where the connection lock is taken to avoid undue lock contention.  


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message