Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Receive semantics #454

Closed
mirjak opened this issue Jan 17, 2020 · 12 comments
Closed

Receive semantics #454

mirjak opened this issue Jan 17, 2020 · 12 comments
Labels

Comments

@mirjak
Copy link
Contributor

mirjak commented Jan 17, 2020

In the examples at the beginning I already didn't really like the fact that we have this "empty" received call there as many application have a response request pattern (and only want complete messages). I wondering if we should provide a SendAndReceive function or even make the send function having an implicit receive for complete message and specify a separate SendOnly function...?

I know these are rather implementation details but the goal was to optimise this interface for what application usually need/do and I'm not sure we reached this goal here.

@mwelzl
Copy link
Contributor

mwelzl commented Jan 17, 2020

I think that binding the functionality of the Receive call (flow control) with a Send as you propose isn't clean design - just because "Send and Receive" is common, I don't see this as necessary (then you tie exactly 1 send to exactly 1 receive ... or invent parameters for a number of sends/receives... this all just strikes me as unnecessary complexity added to the API).

I do suspect, from the way you phrase it ("the 'empty' received call"), that you stumbled over the syntax here - just the way it looks to issue what seems like an 'empty' receive call.

So: perhaps it would be better to make the real functionality of Receive() more explicit, by renaming it into EnqueueReceiveEvent() ?

@mwelzl mwelzl added the API label Jan 17, 2020
@mirjak
Copy link
Contributor Author

mirjak commented Jan 17, 2020

So: perhaps it would be better to make the real functionality of Receive() more explicit, by renaming it into EnqueueReceiveEvent() ?

That would make at least the example at the very beginning of this doc much clearer.

Btw. maybe we should also say explicitly somewhere what happens if a message is received before the received event is enqueued.

@mwelzl
Copy link
Contributor

mwelzl commented Jan 17, 2020

I agree on both accounts. Let's see what others think!

@abrunstrom
Copy link
Contributor

I agree with @mwelzl to keep Send and Receive separate as the most basic calls.

I do not really like the EnqueueReceiveEvent suggestion. The API is event-based so we have many calls of this type. Should Listen then be EnqueueConnectionReceivedEvent or why single out the Receive event to make the Enqueue-Event explicit?

Making explicitly what happens if a message is received before the received event is enqueued sounds good.

@tfpauly
Copy link
Contributor

tfpauly commented Jan 18, 2020

I really don't like bundling a send and receive; that's an application choice to call them in sequence if they want. I also don't think EnqueueReceiveEvent adds much. I agree more with @abrunstrom here.

@MaxF12
Copy link
Collaborator

MaxF12 commented Jan 19, 2020

I also agree with @abrunstrom and that we need to be explicit about what happens to messages that arrive before the first receive() call (if there never is one, what kind of error would it be when messages need to get dropped because the buffer is full?).
However, as mentioned before, I think that the fact that you have to enqueue single receives is inconvenient in general. For example, in our implementation, there is a new receive() call in every received callback as it is required if you want to receive more than one message.
I think one way to solve this is to add a transport property that specifies wether or not the application wants to just receive all (complete) messages as soon as they are available or not.

@tfpauly
Copy link
Contributor

tfpauly commented Jan 19, 2020

@MaxF12 there is a very good reason to keep individual receive calls—that is the mechanism for achieving meaningful backpressure. While it is convenient, sure, for an application to be able to say "give me everything all the time", that practically means that the implementation is always buffering up to its memory limits. If the convenience to ignore flow control/backpressure is present, you would see the majority of adopters use that method, which would increase the amount of queuing in a TAPS layer. We would certainly not implement such a function in our framework.

@mwelzl
Copy link
Contributor

mwelzl commented Jan 20, 2020

I agree with @abrunstrom and I agree with things that were said after her post.

Now, a question. We all seem to agree that it's good to be explicit about what happens when a Message arrives before a Receive has been issued. My question is: what should really happen?

I think that the default behavior should be to hand it on, because the code may just not have reached the first "Receive" yet. Else, to listen and immediately be able to receive a single message, one must always write "listen" and "receive" directly one after another... but whenever I call "Listen", surely it means I'm ready to receive something? So I guess it would be better if a call to "Listen" should really translate into one "Listen" and one "Receive" call. If we agree, then we should state this in the description of "Listen".

@mirjak
Copy link
Contributor Author

mirjak commented Jan 20, 2020

Maybe I'm confusing myself now but let's assume you don't call receive immediately and, as your connection is fast, the transport received already a whole bunch of small messages before the first receive. receive will then provide me the first complete message. How do I now how many times I have to call received again to get all enqueued messages?

@MaxF12
Copy link
Collaborator

MaxF12 commented Jan 20, 2020

Now, a question. We all seem to agree that it's good to be explicit about what happens when a Message arrives before a Receive has been issued. My question is: what should really happen?

I think the message should just sit in the queue of the taps implementation until the application calls receive. If there is a framer it may already deframe it.

Else, to listen and immediately be able to receive a single message, one must always write "listen" and "receive" directly one after another... but whenever I call "Listen", surely it means I'm ready to receive something? So I guess it would be better if a call to "Listen" should really translate into one "Listen" and one "Receive" call. If we agree, then we should state this in the description of "Listen".

Would that be one automatic receive per incoming connection or just one automatic receive for the first new connection?

How do I now how many times I have to call received again to get all enqueued messages?

With the current API you would just have to keep calling receive() until you no longer get any received events. This is what I meant when I said that in many practical cases there will a receive() call in every received() callback.

Has there ever been any consideration to change the receive semantics to something closer to how its done for framers?
I.e. the application gets a received/receivedPartial event, which does not include message data, whenever there is a new message/ data. It can then call receive() to get a complete/ partial message. I think that would get rid of some of the issues like not knowing if there are any new mesaages while also keeping backpressure.

@theri
Copy link
Contributor

theri commented Jan 20, 2020

Agree with @MaxF12 about the expected behavior.

I don't think we should change Listen() to automatically do one Receive() call for the same reason that we don't want to couple Send() and Receive(). To me it sounds like bad design and also quite unintuitive. If the application wants to receive messages right away after Listen() (and ConnectionReceived<>, right, because otherwise there is no Connection to receive messages on?), why not just call Receive() right away?

I think adding more calls, like make receive events more similar to framers, makes our interface even more complex, which we should really avoid.

@mwelzl
Copy link
Contributor

mwelzl commented Jan 22, 2020

I agree with @theri, and with @MaxF12 about the expected behavior: the data should just sit there and wait for Receive to be issued. I take back my proposed ListenAndReceive behavior, that was a bad idea.

mwelzl added a commit that referenced this issue Mar 4, 2020
#450: removed the confusing sentence;
#494: carried out replacements suggested by @gorryfair
mwelzl added a commit that referenced this issue Mar 9, 2020
API receive semantics and DSCP text fixes; closes minor issues #454 #450 #494
@britram britram closed this as completed Apr 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants