Make the CSub GetCollectEntries() RPC response streaming #1865
Conversation
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
- The deps.dev API proto file was removed in a6c67d3 Signed-off-by: Narsimham Chelluri (Narsa) <narsa@kusari.dev>
- also fix typo in CollectSubscriberServiceClient name Signed-off-by: Narsimham Chelluri (Narsa) <narsa@kusari.dev>
nchelluri
force-pushed
the
nchelluri/csub-datasources-trimming
branch
from
de2ed32
to
ecdb818
Compare
- In my testing the message size gets too big at ~55K pURLs. - So I bumped message size up from 1K entries to 15K entries. This should allow plenty of room for large data source entries and also still result in far fewer roundtrips. Signed-off-by: Narsimham Chelluri (Narsa) <narsa@kusari.dev>
jeffmendoza
approved these changes
Apr 26, 2024
pxp928
approved these changes
Apr 29, 2024
Signed-off-by: Narsimham Chelluri (Narsa) <narsa@kusari.dev>
Signed-off-by: Narsimham Chelluri (Narsa) <narsa@kusari.dev>
arorasoham9
pushed a commit
to arorasoham9/guac
that referenced
this pull request
May 17, 2024
* Fix Makefile proto target - The deps.dev API proto file was removed in a6c67d3 Signed-off-by: Narsimham Chelluri (Narsa) <narsa@kusari.dev> * Make GetCollectEntries() rpc have a streaming response - also fix typo in CollectSubscriberServiceClient name Signed-off-by: Narsimham Chelluri (Narsa) <narsa@kusari.dev> * Bump up response message size - In my testing the message size gets too big at ~55K pURLs. - So I bumped message size up from 1K entries to 15K entries. This should allow plenty of room for large data source entries and also still result in far fewer roundtrips. Signed-off-by: Narsimham Chelluri (Narsa) <narsa@kusari.dev> * Add missing header comments Signed-off-by: Narsimham Chelluri (Narsa) <narsa@kusari.dev> * Make cb error more descriptive Signed-off-by: Narsimham Chelluri (Narsa) <narsa@kusari.dev> --------- Signed-off-by: Narsimham Chelluri (Narsa) <narsa@kusari.dev> Signed-off-by: Soham Arora <arorasoham9@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description of the PR
As I demonstrated in #661 (comment), with the current
mainit is possible to quickly gather enough pURLs or other CSub data sources that the gRPC response size for aGetCollectEntries()call grows too big and so the call will error out. We decided that until some sort of CSub data source pubsub system is implemented (that will likely be the long term solution we go with), we will make this call have a streaming response, and I have done that in this PR.A couple of notes:
maintheGetCollectEntries()calls fails after < 60K pURLs are in the response. With this PR, I get to over 160K pURLs in the (now-streaming) response and things still look fine.protoc-gen-go v1.28.1I used here is less than the previously used version which wasprotoc-gen-go v1.31.0. I can't explain this; I followed the instructions on https://grpc.io/docs/languages/go/quickstart/PR Checklist
-sflag togit commit.make generatehas been runmake generatehas been runcollectsubprotobuf has been changed,make protohas been run