This closes #657 ARTEMIS-650 clarify doc, use TTL default for 0 heartbeat

artemis-protocols/artemis-stomp-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/stomp/v11/StompFrameHandlerV11.java

@@ -95,7 +95,7 @@ public class StompFrameHandlerV11 extends VersionedStompFrameHandler implements
                   response.addHeader(Stomp.Headers.Connected.HEART_BEAT, "0,0");
                }
                else {
-                  response.addHeader(Stomp.Headers.Connected.HEART_BEAT, Long.toString(heartBeater.serverPingPeriod) + "," + Long.toString(heartBeater.clientPingResponse));
+                  response.addHeader(Stomp.Headers.Connected.HEART_BEAT, heartBeater.serverPingPeriod + "," + heartBeater.clientPingResponse);
                }
             }
          }
@@ -116,8 +116,6 @@ public class StompFrameHandlerV11 extends VersionedStompFrameHandler implements
       return response;
    }
 
-   //ping parameters, hard-code for now
-   //the server can support min 20 milliseconds and receive ping at 100 milliseconds (20,100)
    private void handleHeartBeat(String heartBeatHeader) throws ActiveMQStompException {
       String[] params = heartBeatHeader.split(",");
       if (params.length != 2) {
@@ -129,10 +127,8 @@ public class StompFrameHandlerV11 extends VersionedStompFrameHandler implements
       //client receive ping
       long minAcceptInterval = Long.valueOf(params[1]);
 
-      if ((minPingInterval != 0) || (minAcceptInterval != 0)) {
       heartBeater = new HeartBeater(minPingInterval, minAcceptInterval);
    }
-   }
 
    @Override
    public StompFrame onDisconnect(StompFrame frame) {
@@ -266,18 +262,27 @@ public class StompFrameHandlerV11 extends VersionedStompFrameHandler implements
 
       private HeartBeater(final long clientPing, final long clientAcceptPing) {
          connectionEntry = ((RemotingServiceImpl)connection.getManager().getServer().getRemotingService()).getConnectionEntry(connection.getID());
-         clientPingResponse = clientPing;
 
+         if (connectionEntry != null) {
+            String heartBeatToTtlModifierStr = (String) connection.getAcceptorUsed().getConfiguration().get(TransportConstants.HEART_BEAT_TO_CONNECTION_TTL_MODIFIER);
+            double heartBeatToTtlModifier = heartBeatToTtlModifierStr == null ? 2 : Double.valueOf(heartBeatToTtlModifierStr);
+
+            // the default response to the client
+            clientPingResponse = (long) (connectionEntry.ttl / heartBeatToTtlModifier);
+
+            if (clientPing != 0) {
+               clientPingResponse = clientPing;
                String ttlMaxStr = (String) connection.getAcceptorUsed().getConfiguration().get(TransportConstants.CONNECTION_TTL_MAX);
                long ttlMax = ttlMaxStr == null ? Long.MAX_VALUE : Long.valueOf(ttlMaxStr);
 
                String ttlMinStr = (String) connection.getAcceptorUsed().getConfiguration().get(TransportConstants.CONNECTION_TTL_MIN);
                long ttlMin = ttlMinStr == null ? 1000 : Long.valueOf(ttlMinStr);
 
-         String heartBeatToTtlModifierStr = (String) connection.getAcceptorUsed().getConfiguration().get(TransportConstants.HEART_BEAT_TO_CONNECTION_TTL_MODIFIER);
+               /* The connection's TTL should be one of the following:
-         double heartBeatToTtlModifier = heartBeatToTtlModifierStr == null ? 2 : Double.valueOf(heartBeatToTtlModifierStr);
+                *   1) clientPing * heartBeatToTtlModifier
-
+                *   2) ttlMin
-         // The connection's TTL should be clientPing * 2, MIN_CLIENT_PING, or ttlMax set on the acceptor
+                *   3) ttlMax
+                */
                long connectionTtl = (long) (clientPing * heartBeatToTtlModifier);
                if (connectionTtl < ttlMin) {
                   connectionTtl = ttlMin;
@@ -291,6 +296,8 @@ public class StompFrameHandlerV11 extends VersionedStompFrameHandler implements
                   ActiveMQServerLogger.LOGGER.debug("Setting STOMP client TTL to: " + connectionTtl);
                }
                connectionEntry.ttl = connectionTtl;
+            }
+         }
 
          if (clientAcceptPing != 0) {
             serverPingPeriod = clientAcceptPing > MIN_SERVER_PING ? clientAcceptPing : MIN_SERVER_PING;

docs/user-manual/en/protocols-interoperability.md

@@ -271,13 +271,6 @@ subscribes (or unsubscribes) for a destination (using a `SUBSCRIBE` or
 
 ### STOMP heart-beating and connection-ttl
 
-Apache ActiveMQ Artemis specifies a minimum value for both client and server heart-beat
-intervals. The minimum interval for both client and server heartbeats is
-500 milliseconds. That means if a client sends a CONNECT frame with
-heartbeat values lower than 500, the server will defaults the value to
-500 milliseconds regardless the values of the 'heart-beat' header in the
-frame.
-
 Well behaved STOMP clients will always send a DISCONNECT frame before
 closing their connections. In this case the server will clear up any
 server side resources such as sessions and consumers synchronously.
@@ -295,28 +288,33 @@ For example:
 
     <acceptor name="stomp-acceptor">tcp://localhost:61613?protocols=STOMP;connectionTtl=20000</acceptor>
 
-The above configuration will make sure that any stomp connection that is
+The above configuration will make sure that any Stomp connection that is
-created from that acceptor will have its connection-ttl set to 20
+created from that acceptor and does not include a `heart-beat` header
-seconds. The `connectionTtl` set on an acceptor will take precedence over
+or disables client-to-server heart-beats by specifying a `0` value will
-`connection-ttl-override`.
+have its connection-ttl set to 20 seconds. The `connectionTtl` set on an
+acceptor will take precedence over `connection-ttl-override`. The default
+`connectionTtl` is 60,000 milliseconds.
 
-Since Stomp 1.0 doesn't support heart-beating then all connections from
+Since Stomp 1.0 does not support heart-beating then all connections from
 Stomp 1.0 clients will have a connection TTL imposed upon them by the broker
 based on the aforementioned configuration options. Likewise, any Stomp 1.1
-or 1.2 clients that don't specify a heart-beat or disable heart-beating
+or 1.2 clients that don't specify a `heart-beat` header or disable client-to-server
-(e.g. by sending `0,0` in the `heart-beat` header) will have a connection
+heart-beating (e.g. by sending `0,X` in the `heart-beat` header) will have 
-TTL imposed upon them by the broker.
+a connection TTL imposed upon them by the broker.
 
-For Stomp 1.1 and 1.2 clients which send a valid `heart-beat` header then
+For Stomp 1.1 and 1.2 clients which send a non-zero client-to-server `heart-beat`
-their connection TTL will be set accordingly. However, the broker will not
+header value then their connection TTL will be set accordingly. However, the broker
-set the connection TTL to the same value as the specified in the `heart-beat`
+will not strictly set the connection TTL to the same value as the specified in the
-since even small network delays could then cause spurious disconnects. Instead,
+`heart-beat` since even small network delays could then cause spurious disconnects.
-the value in the heart-beat will be multiplied by the `heartBeatConnectionTtlModifer`
+Instead, the client-to-server value in the `heart-beat` will be multiplied by the
-specified on the acceptor. The `heartBeatConnectionTtlModifer` is a decimal 
+`heartBeatConnectionTtlModifer` specified on the acceptor. The
-value that defaults to 2.0 so for example, if a client sends a `heart-beat`
+`heartBeatConnectionTtlModifer` is a decimal value that defaults to `2.0` so for
-frame of `1000,0` the the connection TTL will be set to `2000` so that the
+example, if a client sends a `heart-beat` header of `1000,0` the the connection TTL
-ping frames sent every 1000 milliseconds will have a sufficient cushion so as
+will be set to `2000` so that the data or ping frames sent every 1000 milliseconds will
-not to be considered late and trigger a disconnect.
+have a sufficient cushion so as not to be considered late and trigger a disconnect.
+This is also in accordance with the Stomp 1.1 and 1.2 specifications which both state,
+"because of timing inaccuracies, the receiver SHOULD be tolerant and take into account
+an error margin."
 
 The minimum and maximum connection TTL allowed can also be specified on the
 acceptor via the `connectionTtlMin` and `connectionTtlMax` properties respectively.
@@ -329,12 +327,15 @@ of `2.0` then the connection TTL would be `40000` (i.e. `20000` * `2.0`) which w
 exceed the `connectionTtlMax`. In this case the server would respond to the client
 with a `heart-beat` header of `0,15000` (i.e. `30000` / `2.0`). As described
 previously, this is to make sure there is a sufficient cushion for the client
-heart-beats. The same kind of calculation is done for `connectionTtlMin`.
+heart-beats in accordance with the Stomp 1.1 and 1.2 specifications. The same kind
+of calculation is done for `connectionTtlMin`.
+
+The minimum server-to-client heart-beat value is 500ms.
 
 > **Note**
 >
 > Please note that the STOMP protocol version 1.0 does not contain any
-> heartbeat frame. It is therefore the user's responsibility to make
+> heart-beat frame. It is therefore the user's responsibility to make
 > sure data is sent within connection-ttl or the server will assume the
 > client is dead and clean up server side resources. With `Stomp 1.1`
 > users can use heart-beats to maintain the life cycle of stomp

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/v11/StompV11Test.java

@@ -407,7 +407,7 @@ public class StompV11Test extends StompV11TestBase {
 
       connV11.disconnect();
 
-      //no heart beat for (0,0)
+      //default heart beat for (0,0) which is default connection TTL (60000) / default heartBeatToTtlModifier (2.0) = 30000
       connV11 = StompClientConnectionFactory.createClientConnection("1.1", hostname, port);
       frame = connV11.createFrame("CONNECT");
       frame.addHeader("host", "127.0.0.1");
@@ -420,7 +420,7 @@ public class StompV11Test extends StompV11TestBase {
 
       assertEquals("CONNECTED", reply.getCommand());
 
-      assertEquals("0,0", reply.getHeader("heart-beat"));
+      assertEquals("0,30000", reply.getHeader("heart-beat"));
 
       Thread.sleep(5000);
 
@@ -790,7 +790,7 @@ public class StompV11Test extends StompV11TestBase {
 
       assertEquals("CONNECTED", reply.getCommand());
 
-      assertEquals("0,0", reply.getHeader("heart-beat"));
+      assertEquals("0,500", reply.getHeader("heart-beat"));
 
       Thread.sleep(3000);
 

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/v12/StompV12Test.java

@@ -596,7 +596,7 @@ public class StompV12Test extends StompV11TestBase {
 
       Assert.assertEquals("CONNECTED", reply.getCommand());
 
-      Assert.assertEquals("0,0", reply.getHeader("heart-beat"));
+      Assert.assertEquals("0,30000", reply.getHeader("heart-beat"));
 
       Thread.sleep(5000);