You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by andytaylor <gi...@git.apache.org> on 2016/02/15 16:23:36 UTC

[GitHub] activemq-artemis pull request: ARTEMIS-398 - AMQP protocol idle ti...

GitHub user andytaylor opened a pull request:

    https://github.com/apache/activemq-artemis/pull/390

    ARTEMIS-398 - AMQP protocol idle timeout issue

    added functionality to tick every n seconds where n is 1/2 the idle timeout
    
    https://issues.apache.org/jira/browse/ARTEMIS-398

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/andytaylor/activemq-artemis master

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/activemq-artemis/pull/390.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #390
    
----
commit bab0fe2850d8d3b00d591821b2b6a1b80d6b4448
Author: Andy Taylor <an...@gmail.com>
Date:   2016-02-15T10:06:49Z

    ARTEMIS-398 - AMQP protocol idle timeout issue
    
    added functionality to tick every n seconds where n is 1/2 the idle timeout
    
    https://issues.apache.org/jira/browse/ARTEMIS-398

----


---
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.
---

[GitHub] activemq-artemis pull request: ARTEMIS-398 - AMQP protocol idle ti...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/activemq-artemis/pull/390


---
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.
---

[GitHub] activemq-artemis pull request: ARTEMIS-398 - AMQP protocol idle ti...

Posted by gemmellr <gi...@git.apache.org>.
Github user gemmellr commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/390#discussion_r52922134
  
    --- Diff: artemis-protocols/artemis-proton-plug/src/main/java/org/proton/plug/context/AbstractConnectionContext.java ---
    @@ -40,28 +44,32 @@
     
     public abstract class AbstractConnectionContext extends ProtonInitializable implements AMQPConnectionContext {
     
    +   public static final Symbol CONNECTION_OPEN_FAILED = Symbol.valueOf("amqp:connection-establishment-failed");
    +
        protected ProtonHandler handler = ProtonHandler.Factory.create();
     
        protected AMQPConnectionCallback connectionCallback;
    +   private final ScheduledExecutorService scheduledPool;
     
        private final Map<Session, AbstractProtonSessionContext> sessions = new ConcurrentHashMap<>();
     
        protected LocalListener listener = new LocalListener();
     
    -   public AbstractConnectionContext(AMQPConnectionCallback connectionCallback) {
    -      this(connectionCallback, DEFAULT_IDLE_TIMEOUT, DEFAULT_MAX_FRAME_SIZE, DEFAULT_CHANNEL_MAX);
    +   public AbstractConnectionContext(AMQPConnectionCallback connectionCallback, ScheduledExecutorService scheduledPool) {
    +      this(connectionCallback, DEFAULT_IDLE_TIMEOUT, DEFAULT_MAX_FRAME_SIZE, DEFAULT_CHANNEL_MAX, scheduledPool);
        }
     
        public AbstractConnectionContext(AMQPConnectionCallback connectionCallback,
                                         int idleTimeout,
                                         int maxFrameSize,
    -                                    int channelMax) {
    +                                    int channelMax,
    +                                    ScheduledExecutorService scheduledPool) {
           this.connectionCallback = connectionCallback;
    +      this.scheduledPool = scheduledPool;
           connectionCallback.setConnection(this);
           Transport transport = handler.getTransport();
           if (idleTimeout > 0) {
    -         transport.setIdleTimeout(idleTimeout);
    -         transport.tick(idleTimeout / 2);
    +         transport.setIdleTimeout(idleTimeout / 2);
    --- End diff --
    
    Meant to comment on this before...did you mean to half the value passed here, it wasn't halved before?
    
    Proton will half the given value before advertising it to the peer since the AMQP spec suggests you should advertise half your actual timeout (proton also pessimistically assumes on the receiving side that the peer did not half it, and so halves it again to ensure it sends more often than the actual timeout).


---
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.
---

[GitHub] activemq-artemis pull request: ARTEMIS-398 - AMQP protocol idle ti...

Posted by andytaylor <gi...@git.apache.org>.
Github user andytaylor commented on the pull request:

    https://github.com/apache/activemq-artemis/pull/390#issuecomment-184306375
  
    leave this for now


---
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.
---

[GitHub] activemq-artemis pull request: ARTEMIS-398 - AMQP protocol idle ti...

Posted by gemmellr <gi...@git.apache.org>.
Github user gemmellr commented on the pull request:

    https://github.com/apache/activemq-artemis/pull/390#issuecomment-184291017
  
    I'd suggest deriving the 'now' ms value passed to Transport#tick() from System#nanoTime() rather than using System#currentTimeInMillis(), since the latter is subject to wall clock change that can throw things off. Similar change made in https://issues.apache.org/jira/browse/AMQ-6031


---
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.
---

[GitHub] activemq-artemis pull request: ARTEMIS-398 - AMQP protocol idle ti...

Posted by andytaylor <gi...@git.apache.org>.
Github user andytaylor commented on the pull request:

    https://github.com/apache/activemq-artemis/pull/390#issuecomment-184294765
  
    cheers Robbie, done


---
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.
---