-
Notifications
You must be signed in to change notification settings - Fork 100
Asynchronous Send mode. #179
Comments
Makes sense and I think the API you propose is overall reasonable. Some minor thoughts/questions:
|
I have thought about this mucho :) A single way to handle errors makes a simpler API, but there's also a correctness argument: you might be tempted to think that a direct error return from Send() guarantees that the message was not sent. This is a dangerous assumption, an error return from net.Conn.Write() does not guarantee that part (or all) of the buffer wasn't received remotely and the failure happened before the ack got back. So unless all layers of the stack are coded very carefully to differentiate "definitely didn't happen" from "maybe didn't happen" it's much safer to assume "maybe". You have to write the "maybe" code anyway and it's semantically correct even if the failure was definite.
+1 I used "Outcome" in electron but that is equally random. "Settlement" might be a better AMQP-esque term. We can use this to provide more info about the AMQP settlement so folks can handle reject, modify and the like if they want (or ignore them if not and just take the Error()) I defer to your judgement, whatever you feel is in keeping with the project style. |
SGTM. For the result type let's go with |
The current Sender.Send() blocks for the remote disposition. This is fine for many applications, but it does not allow sending at full network throughput because it forces the sender t idle for at least 2x the network latency between each send.
The doc mentions using goroutines, but this is not really workable - you lose control of message ordering. What you really want is an ordered stream of messages going out and an independent ordered stream of dispositions coming in.
One solution is to send the dispositions back on a channel, like https://github.com/apache/qpid-proton/blob/go1/electron/sender.go#L61
However I am currently leaning towards a design like this:
For the sync case this is almost as easy to use as blocking Send():
and it supports the the channel solution for async:
I'd like to hear comments on the design before I submit a PR.
The text was updated successfully, but these errors were encountered: