activemq-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (ARTEMIS-883) Fix OpenWire ProducerFlowControlTest Regression
Date Wed, 21 Dec 2016 16:03:58 GMT

    [ https://issues.apache.org/jira/browse/ARTEMIS-883?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15767412#comment-15767412
] 

ASF GitHub Bot commented on ARTEMIS-883:
----------------------------------------

Github user clebertsuconic commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/907#discussion_r93465898
  
    --- Diff: artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/amq/AMQSession.java
---
    @@ -303,108 +301,103 @@ public void send(final ProducerInfo producerInfo,
              originalCoreMsg.putStringProperty(org.apache.activemq.artemis.api.core.Message.HDR_DUPLICATE_DETECTION_ID.toString(),
messageSend.getMessageId().toString());
           }
     
    -      Runnable runnable;
    -
    -      if (sendProducerAck) {
    -         runnable = new Runnable() {
    -            @Override
    -            public void run() {
    -               try {
    -                  ProducerAck ack = new ProducerAck(producerInfo.getProducerId(), messageSend.getSize());
    -                  connection.dispatchSync(ack);
    -               } catch (Exception e) {
    -                  ActiveMQServerLogger.LOGGER.warn(e.getMessage(), e);
    -                  connection.sendException(e);
    -               }
    -
    -            }
    -         };
    -      } else {
    -         final Connection transportConnection = connection.getTransportConnection();
    -
    -         if (transportConnection == null) {
    -            // I don't think this could happen, but just in case, avoiding races
    -            runnable = null;
    -         } else {
    -            runnable = new Runnable() {
    -               @Override
    -               public void run() {
    -                  transportConnection.setAutoRead(true);
    -               }
    -            };
    -         }
    -      }
    -
    -      internalSend(actualDestinations, originalCoreMsg, runnable);
    -   }
    +      boolean shouldBlockProducer = producerInfo.getWindowSize() > 0 || messageSend.isResponseRequired();
     
    -   private void internalSend(ActiveMQDestination[] actualDestinations,
    -                             ServerMessage originalCoreMsg,
    -                             final Runnable onComplete) throws Exception {
    +      final AtomicInteger count = new AtomicInteger(actualDestinations.length);
     
    -      Runnable runToUse;
    +      final Exception[] anyException = new Exception[] {null};
    --- End diff --
    
    @gaohoward sorry Howard, this looks a hack to me... and it's even harder to debug what's
going on...
    store.checkMemory() will happen outside of the context of the caller's thread...
    
    
    What I meant by the Runnable will throw a Thread, is that you should fix it properly..
not adding this kind of thing that will never meant to run... The thread still set an exception
on a finally that will never happen as the caller's thread is gone...
    
    
    
    Instead of throwing an exception, your runnable should call connection.sendException()
and interrupt execution, instead of having this code in place.



> Fix OpenWire ProducerFlowControlTest Regression
> -----------------------------------------------
>
>                 Key: ARTEMIS-883
>                 URL: https://issues.apache.org/jira/browse/ARTEMIS-883
>             Project: ActiveMQ Artemis
>          Issue Type: Bug
>          Components: OpenWire
>    Affects Versions: 1.5.1
>            Reporter: Howard Gao
>            Assignee: Howard Gao
>             Fix For: 1.5.next
>
>
> ProducerFlowControlTest fails due to regression



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message