Qpid
  1. Qpid
  2. QPID-2796

[Java] implement support for heartbeats following IO changes to the 0-8/0-9/0-9-1/0-10 broker and 0-8/0-9/0-9-1 client

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.15
    • Fix Version/s: 0.20, 0.21
    • Component/s: Java Broker, Java Client
    • Labels:
      None

      Description

      The Java broker and client are not able to generate AMQP heartbeats following the updates to their IO layer. The 0-8/0-9/0-9-1 codepaths previously relied on the Mina IO layer to support this, and as such can no longer do so. The 0-10 path of the Java broker leverages much of the 0-10 implementation in common originally developed for the 0-10 Java client, which simply reflects heartbeats it receives from a broker and as such no heartbeats will ever be sent by broker or the client despite its support.

        Activity

        Hide
        Rob Godfrey added a comment -

        Merged to 0.20 as r1414927.

        Show
        Rob Godfrey added a comment - Merged to 0.20 as r1414927.
        Hide
        Justin Ross added a comment -

        Reviewed by Robbie. Approved for 0.20.

        Show
        Justin Ross added a comment - Reviewed by Robbie. Approved for 0.20.
        Justin Ross made changes -
        Fix Version/s 0.20 [ 12323548 ]
        Robbie Gemmell made changes -
        Status Ready To Review [ 10006 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Robbie Gemmell added a comment -

        Looks good to me.

        Show
        Robbie Gemmell added a comment - Looks good to me.
        Rob Godfrey made changes -
        Assignee Rob Godfrey [ rgodfrey ] Robbie Gemmell [ gemmellr ]
        Hide
        Rob Godfrey added a comment -

        I've made a further check-in to address Robbie's review comments:

        http://svn.apache.org/viewvc?rev=1413549&view=rev

        Show
        Rob Godfrey added a comment - I've made a further check-in to address Robbie's review comments: http://svn.apache.org/viewvc?rev=1413549&view=rev
        Hide
        Rob Godfrey added a comment -
        Show
        Rob Godfrey added a comment - Added a system test: http://svn.apache.org/viewvc?rev=1413539&view=rev
        Rob Godfrey made changes -
        Status In Progress [ 3 ] Ready To Review [ 10006 ]
        Rob Godfrey made changes -
        Status Open [ 1 ] In Progress [ 3 ]
        Robbie Gemmell made changes -
        Summary implement support for heartbeats following IO changes to the broker and 0-8/0-9/0-9-1 client [Java] implement support for heartbeats following IO changes to the 0-8/0-9/0-9-1/0-10 broker and 0-8/0-9/0-9-1 client
        Hide
        Robbie Gemmell added a comment -

        Changes look good so far. A couple of nitpicks and a posible problem:

        In the 0-10 Connection class there was a private implementation of TransportActivity added, but it isnt used. The updated connect() call passes null, which looks unsafe at first when following the subsequent method calls in IoNetowrkTransport but ultimately ends up being ok (because we dont set the reader/writer idle times at all on the 0-10 client side and rely on reflecting the brokers heartbeats if any) but possibly means we are doing socket timesouts with no effect. Given the IoReceiver ignores the ticker if it is null, could we just not create one if we arent going to really use it (as happens in the existing but now-unused IoNetworkConnection constructor)?

        In IdleTimeoutTicker it might be nicer if instead of checking !=0 it used >0 as happens elsewhere.

        Since IdleTimeoutTicker.tick() is only called when we get a SocketTimeoutException in IoReceiver, doesnt that mean its possible we can be in a writer-idle situation and yet never signal so (if the reader keeps getting data within its timeout, which is twice as long as the senders)? The old 0-8/9/9-1 clients will happily kill the connection in this situation if the broker doenst send them anything.

        Show
        Robbie Gemmell added a comment - Changes look good so far. A couple of nitpicks and a posible problem: In the 0-10 Connection class there was a private implementation of TransportActivity added, but it isnt used. The updated connect() call passes null, which looks unsafe at first when following the subsequent method calls in IoNetowrkTransport but ultimately ends up being ok (because we dont set the reader/writer idle times at all on the 0-10 client side and rely on reflecting the brokers heartbeats if any) but possibly means we are doing socket timesouts with no effect. Given the IoReceiver ignores the ticker if it is null, could we just not create one if we arent going to really use it (as happens in the existing but now-unused IoNetworkConnection constructor)? In IdleTimeoutTicker it might be nicer if instead of checking !=0 it used >0 as happens elsewhere. Since IdleTimeoutTicker.tick() is only called when we get a SocketTimeoutException in IoReceiver, doesnt that mean its possible we can be in a writer-idle situation and yet never signal so (if the reader keeps getting data within its timeout, which is twice as long as the senders)? The old 0-8/9/9-1 clients will happily kill the connection in this situation if the broker doenst send them anything.
        Hide
        Rob Godfrey added a comment -

        Added support for sending heartbeats to the Java Broker (all 0-x protocol versions) and Java Client (0-8/9/9-1 protocols).

        http://svn.apache.org/viewvc?rev=1413376&view=rev

        Will be adding a system test before resolving the JIRA

        Show
        Rob Godfrey added a comment - Added support for sending heartbeats to the Java Broker (all 0-x protocol versions) and Java Client (0-8/9/9-1 protocols). http://svn.apache.org/viewvc?rev=1413376&view=rev Will be adding a system test before resolving the JIRA
        Rob Godfrey made changes -
        Component/s Java Client [ 12311389 ]
        Rob Godfrey made changes -
        Fix Version/s 0.21 [ 12323549 ]
        Fix Version/s Future [ 12315490 ]
        Rob Godfrey made changes -
        Assignee Rob Godfrey [ rgodfrey ]
        Hide
        Robbie Gemmell added a comment -

        Issue updated to reflect scope widening from only the0-10 path being unable to use heartbeats to all protocols paths currently being unable to support heartbeats

        Show
        Robbie Gemmell added a comment - Issue updated to reflect scope widening from only the0-10 path being unable to use heartbeats to all protocols paths currently being unable to support heartbeats
        Robbie Gemmell made changes -
        Summary broker cant generate 0-10 heartbeats implement support for heartbeats following IO changes to the broker and 0-8/0-9/0-9-1 client
        Fix Version/s Future [ 12315490 ]
        Affects Version/s 0.15 [ 12319043 ]
        Affects Version/s 0.6 [ 12313728 ]
        Description The Java broker is not able to generate 0-10 AMQP heartbeats. The broker implementation leverages much of the 0-10 implementation in common originally developed for the client, which simply reflects heartbeats it receives from a broker and as such no heartbeats will ever be sent when it is connected to the Java broker. The Java broker and client are not able to generate AMQP heartbeats following the updates to their IO layer. The 0-8/0-9/0-9-1 codepaths previously relied on the Mina IO layer to support this, and as such can no longer do so. The 0-10 path of the Java broker leverages much of the 0-10 implementation in common originally developed for the 0-10 Java client, which simply reflects heartbeats it receives from a broker and as such no heartbeats will ever be sent by broker or the client despite its support.
        Robbie Gemmell made changes -
        Field Original Value New Value
        Fix Version/s 0.7 [ 12314455 ]
        Hide
        Robbie Gemmell added a comment -

        Updating 'Fix For' to Unknown on issues not targeted for 0.8

        Show
        Robbie Gemmell added a comment - Updating 'Fix For' to Unknown on issues not targeted for 0.8
        Robbie Gemmell created issue -

          People

          • Assignee:
            Robbie Gemmell
            Reporter:
            Robbie Gemmell
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development