-
Notifications
You must be signed in to change notification settings - Fork 21
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
[new] ADR-40 Request Many #228
base: main
Are you sure you want to change the base?
Changes from 3 commits
8284e98
2af68cf
a2e4c72
199b1e3
c9ab060
8a85c82
0aabefa
d4deecd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
# Request Many | ||
|
||
| Metadata | Value | | ||
|----------|-----------------------| | ||
| Date | 2023-06-26 | | ||
| Author | @aricart, @scottf | | ||
| Status | Partially Implemented | | ||
| Tags | client | | ||
|
||
## Problem Statement | ||
Have the client support receiving multiple replies from a single request, instead of limiting the client to the first reply. | ||
|
||
## Design | ||
|
||
Making the request and handling multiple replies is straightforward. Much like a simplified fetch, the developer | ||
will provide some basic strategy information that tell how many messages to wait for and how long to wait for those messages. | ||
|
||
## Options | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want to have some common defaults across clients around max time or gap time? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can but reallly that value should be based on the worse rtt for the furthest service There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Timeout shouldn't necessarily be dictated by RTT (although tbh it is in most cases). There are cases where there's value in determining that SLAs cannot be met and a hard timeout would indicate that - particularly around time dependent data where business value decreases over time or may even be harmful/misleading if stale. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using the worst |
||
|
||
#### Count | ||
|
||
The number of responses to allow. | ||
|
||
* Responses are accepted until the count is reached. | ||
* Must be used in combination with a time as the count may never be reached. | ||
|
||
#### Sentinel | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As discussed on calls should we be clear that this is not something to add to the clients but rather a pattern to document for now? like, this isnt hard and its more flexible: MultiSub(subj, func(m *nats.Msg)) {
// sentinal that can be based on nil body, headers or anything
if len(m.Data)==0 || m.Header.Get("EOF") != "" {
m.Sub.Unsubscribe()
return
}
// handle msg
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It depends, right - if you are yielding an iterator, you could break, and that would close it. So for example if you are using the mux subscription to handle the responses, you cannot unsubscribe there, and perhaps you want to cleanup there. Not sure that not implementing it is not helpful. Imho, if it is a pattern, the question is whether the sentinel has any meaning, if it doesn't, the client shouldn't even see it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The concern is the sentinel can be anything - server supports nil in one specific case but users might other things. So to have it built in the se final detector should be an injectable dependency so we provide a nil detecting one and users can do their own detector? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As usual, maybe lets remove it then. We can always extend it later. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am going to keep it in mine, because in the mux I wouldn't have a way of cleaning. Other clients can expose the other patterns. |
||
|
||
A sentinel is a message with an empty payload. | ||
|
||
* Must be used in combination with a time as a sentinel may never be sent. | ||
|
||
#### Max Time | ||
|
||
The amount of time to wait for responses. | ||
|
||
* Responses are accepted until the max time is reached. | ||
|
||
#### First Max / Gap Time | ||
|
||
A combination time... | ||
|
||
* The first response must be received by the first max time or the request expires. | ||
* Subsequent messages must be received in the gap time, the time since the last message was received or the request expires. | ||
* Each time a message is received the gap timer is reset. | ||
|
||
### Combinations | ||
* Sentinel can be used with any other option. | ||
* Both time options can be used together. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The last statement is vague. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did some additional survey of my code, and realized, that I don't issue a timeout ever, I simply return no messages (the iterator will stop and yield nothing) if the request is not answered in time. Note that in JavaScript this returns an iterator always, the iterator may not yield any messages, but if a message is received, it yields it immediately
With the above said, possibilities are:
AND:
The client doesn't assume success or failure - only that it might receive messages - The various options are there to short circuit the length of the wait.
The jitter value allows for waiting for the service with the slowest latency. (scatter gather)
The message count allows for waiting for some count of messages or a timer (scatter gather)
The sentinel strategy allows for waiting for chunked data, order is not required since the client will see all messages and can use its internal protocol to determine the order of the messages or if additional request for missing/corrupt data should be done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It has similar semantics to
fetch
from JS API.Waiting for all messages or timeout, but just closing iterator when timeout is hit is a valid use case, however I think it should be client-specific, following language patterns, so may vary.
In general - all additional options - how many messages, how long, should just add additional triggers to "close", "fuse" the iterator, or do a proper thing in other patterns.