-
Notifications
You must be signed in to change notification settings - Fork 20
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
Open
scottf
wants to merge
6
commits into
main
Choose a base branch
from
request-many
base: main
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
8284e98
[new] ADR-40 Request Many
scottf 2af68cf
[new] ADR-40 Request Many
scottf a2e4c72
[new] ADR-40 Request Many
scottf 199b1e3
review comments 1
scottf c9ab060
Merge branch 'main' into request-many
scottf 8a85c82
Improve request many ADR
Jarema File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
# Request Many | ||
|
||
| Metadata | Value | | ||
|----------|-----------------------| | ||
| Date | 2023-06-26 | | ||
| Author | @aricart, @scottf, @Jarema | | ||
| 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. | ||
|
||
* 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 Subsequent Max 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) | ||
|
||
## Config | ||
|
||
#### Timeout | ||
|
||
**Default: same as default for standard request or 1 second** | ||
If client supports global timeout config, it should be reused as a default here. | ||
|
||
The maximum amount of time to wait for responses. | ||
* Responses are accepted until the max time is reached. | ||
|
||
#### Muxing | ||
By default, request many does use client request muxer. | ||
|
||
### Strategies | ||
|
||
#### None | ||
Only timeout applies | ||
|
||
#### Interval / Max Jitter | ||
**Default: disabled** | ||
|
||
Max interval that can be encountered between messages. | ||
If interval between any messages is larger than set, client will not wait for more messages. | ||
|
||
#### Count | ||
**Default: No default. Count is required to use this strategy** | ||
|
||
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. | ||
|
||
## Regarding Time | ||
It's possible that there is a connectivity issue that prevents messages from reaching the requestor so | ||
a request timeout, versus serving the responses being terminated by meeting a time condition. | ||
It would be useful to be able to differentiate between these, but this may not be possible. Do the best you can. | ||
|
||
## Additional Discussion | ||
Is there some level of implementation that should be considered as basic core behavior, such as the most simple multiple reply with a max wait? | ||
Or should the entire implementation be dealt with like an add-on, similar to KV, OS | ||
|
||
## Beyond Scope, for future consideration | ||
|
||
#### Sentinel strategy | ||
|
||
A sentinel is a message with an empty payload. | ||
|
||
* The sentinel strategy allows for waiting for chunked data, | ||
for instance a message that represents a large file that is larger than the max payload. | ||
* It's up to the developer to understand the data, not the client's responsibility. | ||
* For ordering for the chunks, maybe use a header. | ||
* If any chunks were missing, re-issue the request indicating the missing chunk ids. | ||
* Must be used in combination with a time as a sentinel may never be sent or missed. |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.