[ADDED] Add support for multi-get directly from a stream. #5107
Conversation
derekcollison
requested review from
ripienaar,
neilalexander and
Jarema
and removed request for
a team
There was a problem hiding this comment.
Behaviour wise I find the batch/unbatched behaviour weird as pointed out in comments - it should just always be a batch imo.
To continue should we hit batch limits I think we need a bit more info.
Imagine I ask for USER.1234.> and this user has a ton of fields, more than 1000 or maxBytes, it would unexpectedly have to continue with another get to get the full user.
But how do I continue in a consistent way? I have no idea what the time or seq is to get it relative against?
So imagine if the EOB message included the sseq pass to UpToSeq for the next multi get.
In this case the sseq would be either the last message in the stream at the time that you gathered the seqs (as in the comment for MultiLastSeq in file store) or whatever the UpToSeq was or whatever UpToTime resolved the seq to. This way if there is a EOB and I need to get more keys to get the whole thing I immediately know that the next Multi get should have UpToSeq: sseq and all the many reads needed to resolve the entire USER.1234.> would be done around a consistent time.
|
|
||
| // Multiple response support. Will get the last msgs matching the subjects. These can include wildcards. | ||
| MultiLastFor []string `json:"multi_last,omitempty"` | ||
| // Only return messages up to this sequence. If not set, will be last sequence for the stream. |
There was a problem hiding this comment.
worth clarifying that these following items are only aplicable to multi? Especially since the error Bad Request is opaque
There was a problem hiding this comment.
You mean more so that line 645?
server/jetstream_test.go
Outdated
| } | ||
| } | ||
|
|
||
| sub := sendRequest(&JSApiMsgGetRequest{Seq: 1, MultiLastFor: []string{"foo.*"}}) |
There was a problem hiding this comment.
why is Seq:1 required here when we are specifically asking for the last for subjects? Whats the seq being used for?
There was a problem hiding this comment.
If it is a one-shot can be left alone and we start with 1, which is how far back should I look for a match. These is important one paged responses.
There was a problem hiding this comment.
Got what you meant, will fix.
derekcollison
force-pushed
the
multi-get
branch
2 times, most recently
from
a7266cc
to
ccc1a58
Compare
|
ADR update that attempts to capture the behaviour here nats-io/nats-architecture-and-design#268 |
This allows a client to get multiple results based on multiple filters, each which can include a wildcard. You can optionally specify and UpToSeq or UpToTime. The results will be returned in sequential order. They will have headers matching direct get batch as well, with num pending being how many more results are coming. Currently we limit to 64 results maximum, and may revisit this in the future. Signed-off-by: Derek Collison <[email protected]>
Also made sure to honor MaxBytes and Batch same as direct get batch. Left this here for easier comparison but will squash when merging. Signed-off-by: Derek Collison <[email protected]>
…me UpToSeq even if it was not set on original request. Also removed need for Seq to be set on simple multi-requests. Signed-off-by: Derek Collison <[email protected]>
derekcollison
force-pushed
the
multi-get
branch
from
ccc1a58
to
5eb4be4
Compare
This allows a client to get multiple results based on multiple filters, each which can include a wildcard. You can optionally specify an UpToSeq or UpToTime.
The results will be returned in sequential order.
They will have headers matching direct get batch as well, with num pending being how many more results are coming. Currently we limit to 64 results maximum, and may revisit this in the future.
Signed-off-by: Derek Collison [email protected]