Uploaded image for project: 'Apache S4'
  1. Apache S4
  2. S4-109

TCPEmitter#send() should return false when it doesn't send a packet across

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.5.0
    • Fix Version/s: 0.6
    • Labels:
      None

      Description

      In TCPEmitter, send() calls sendMessage() which silently discards message if it can't connect to the Listener. Instead, it should return false.

      1. s4-109.patch
        2 kB
        Karthik Kambatla

        Issue Links

          Activity

          Hide
          kkambatl Karthik Kambatla (Inactive) added a comment -

          Uploading a trivial patch to address the issue.

          Show
          kkambatl Karthik Kambatla (Inactive) added a comment - Uploading a trivial patch to address the issue.
          Hide
          mmorel Matthieu Morel added a comment -

          Thanks Karthik!
          However, we recently merged S4-95 branch into dev so the patch proposed here would not apply. Also, in the new implementation, the emitter first acquires a permit (so it can actually write on the TCP channel), before the message can be sent. The actual write is an asynchronous operation.

          In this case what would be the meaning of returning a boolean? We could return a future though, in case the sender might want to do something about that. In particular, this information could be useful in some implementations of sender executors.

          Show
          mmorel Matthieu Morel added a comment - Thanks Karthik! However, we recently merged S4-95 branch into dev so the patch proposed here would not apply. Also, in the new implementation, the emitter first acquires a permit (so it can actually write on the TCP channel), before the message can be sent. The actual write is an asynchronous operation. In this case what would be the meaning of returning a boolean? We could return a future though, in case the sender might want to do something about that. In particular, this information could be useful in some implementations of sender executors.
          Hide
          kkambatl Karthik Kambatla (Inactive) added a comment -

          Thanks Matthieu. Returning a future makes a lot more sense, and captures best my original intention of being able to communicate potential failures.

          I got confused with the branches, was looking at the piper branch all this while. Let me take a closer look at dev. If we have not already, we should attempt at augmenting the API to return a Future.

          Show
          kkambatl Karthik Kambatla (Inactive) added a comment - Thanks Matthieu. Returning a future makes a lot more sense, and captures best my original intention of being able to communicate potential failures. I got confused with the branches, was looking at the piper branch all this while. Let me take a closer look at dev. If we have not already, we should attempt at augmenting the API to return a Future.
          Hide
          kkambatl Karthik Kambatla (Inactive) added a comment -

          Makes sense. Thanks Matthieu. I ll close this is a part of S4-95.

          Show
          kkambatl Karthik Kambatla (Inactive) added a comment - Makes sense. Thanks Matthieu. I ll close this is a part of S4-95 .

            People

            • Assignee:
              kasha Karthik Kambatla
              Reporter:
              kasha Karthik Kambatla
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development