ADR-47 Request Many #228
ADR-47 Request Many #228
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Signed-off-by: Tomasz Pietrek <[email protected]>
This comment was marked as outdated.
This comment was marked as outdated.
|
@ripienaar agree. |
|
|
||
| The maximum number of messages to wait for. | ||
| * Optional | ||
| * If this number of messages is received, the request is complete. |
There was a problem hiding this comment.
seems implementation is leaning towards sub inbox per request so this should be using auto unsub here
There was a problem hiding this comment.
This is a great idea, but would work only on non-muxed request-many's.
There was a problem hiding this comment.
Adding notes about the optionally using immediate unsub when max responses is supplied.
There was a problem hiding this comment.
I see, ok it would be better if the muxed version of request for many responses is the one that is in the library since that has more value added and trickier to get right. Not sure if want to include both implementations (new style + old style) could be that just document that old style approach since not a lot of code in the ADR.
There was a problem hiding this comment.
Is there a significant performance gain that would compensate for the complexity of using muxed?
There was a problem hiding this comment.
the muxed version is better for the interest graph in cluster setups, specially in a super cluster setup with gateways, that is why clients Request defaults to that version
adr/ADR-47.md
Outdated
| If the client supports a sentinel with a callback/predicate that accepts the message and returns a boolean, | ||
| a return of true would mean continue to process and false would mean stop processing. | ||
|
|
||
| If possible, the client should support the "standard sentienl", which is a message with a null/nil or empty payload. |
There was a problem hiding this comment.
| If possible, the client should support the "standard sentienl", which is a message with a null/nil or empty payload. | |
| If possible, the client should support the "standard sentinel", which is a message with a null/nil or empty payload. |
adr/ADR-47.md
Outdated
|
|
||
| ### Cancelling | ||
|
|
||
| If possible, the user should be able to cancel the request. This is not the sentinel. |
There was a problem hiding this comment.
here for example would mean to unsub without a limit set
There was a problem hiding this comment.
This is intended that the dev can cancel the entire request-many arbitrarily. For instance I could envision a request that gives 1 second for responses, gets many, realizes it has all the information it needs and is done. If it's using a manual sentinel pattern with a predicate, they can short circuit that way, but if they are not using a sentinel, this is just another way for manually short circuit.
I will refine the section.
|
|
||
| ### Max messages | ||
|
|
||
| The maximum number of messages to wait for. |
There was a problem hiding this comment.
Depending on the implementation approach, with new style and max responses traffic wise you can get more than the max responses anyway and the client will receive them and not process them. With old style and auto unsub you can short circuit the number of max messages to avoid flooding the client with unnecessary responses, some pros and cons depending on the protocols sent.
New style/mux based approach has more value added though since more difficult to maintain for users on their own.
There was a problem hiding this comment.
Added more notes under a Max Responses Optimization section.
| Theoretically this unsub could be processed after a reply has come in and out of the server, so you still must check the count manually. | ||
|
|
||
| ### Best Practice | ||
| Since the client is in charge of the subscription, it should always unsubscribe upon completion of the request handling instead of leaving it up to the server to time it out. |
There was a problem hiding this comment.
would only happen when not using the mux version/new style, if using old style/non mux it must always clean up to avoid leaking subscriptions, that is a very common issue when users implement their own version of request/response expecting multiple responses
Request Many ADR based on @aricart's implementation and discussion in #215