ghz icon indicating copy to clipboard operation
ghz copied to clipboard

Improve file and stdin data to support streaming input ?

Open bojand opened this issue 5 years ago • 7 comments

Currently when data is provided using a file or stdin we read the full data. We chould improve this and support reading and parsing as a stream, probably using json.Decoder. However this may cause some breakage or limitation with supporting a bit more flexible data as we do now. For example presently if a single JSON object is passed in for a client streaming request and bidi request, we use that for all writes to the client. This may not be possible with a streaming input. Similarly for client streaming calls, it would have to be an array input. And also not sure how that should work. We should probably send + record until the end of the stream, and then replay the payload as writes for all subsequent calls.

bojand avatar Jul 26 '18 00:07 bojand

I am guessing this is talking about support for Streaming Services too. Are you guys looking into this?

snktagarwal avatar Jan 25 '19 05:01 snktagarwal

Hello, thanks for the question. No this issue has nothing really to do with streaming calls per se, just how we take the data within the CLI. It's a suggestion for an alternate way to read data within CLI, but it has pros and cons.

With regards to actual gRPC calls we support data for all 4 call mechanisms include streaming and bi-directional calls. For example client streaming data can be sent as an array, each element representing a single message:

ghz -proto ./greeter.proto \
  -call helloworld.Greeter.SayHelloCS \
  -d '[{"name":"Joe"},{"name":"Kate"},{"name":"Sara"}]' \
  0.0.0.0:50051

If a single object is given for data it is sent as every message until the end.

Hope this helps clarify things.

bojand avatar Jan 25 '19 13:01 bojand

That is pretty cool. One last question, we often have a case where the streams have some rate of messages. Consider audio streaming, you send one packet every 100ms. Is there a way to simulate that?

Also, if the server is streaming as well -- does the ghz library simply ignore the output until it gets an EOF?

snktagarwal avatar Jan 25 '19 16:01 snktagarwal

We know what kind of call this is (Unary, Client Streaming, Server Streaming or BiDi) from the proto file and perform call action(s) specifically based on the type.

In case of client streaming we send all the messages in the input array and then we close and receive .

In case of server streaming we just read messaged until EOF.

In case of bidi we send all the messages in the array first and close the send portion; then read the messages from the server until EOF and then the call is finished.

Currently we do not have a way to specify a timeout interval in between the message sends inside the stream. Feel free to open an feature request issue for this with some specifics and details on use-case and desired behaviour.

bojand avatar Jan 25 '19 17:01 bojand

@snktagarwal Is #58 like something to what you have in mind?

bojand avatar Jan 26 '19 16:01 bojand

Wow. Yeah, that's exactly what would be helpful.

Now taking this conversation to the next step -- why not have a way to put expectations? The reason is because with streaming rpcs it's important to know if the results are coming back in regular time.

I am actually curious, does GHz assume that if the return status is OK then the rpc succeeded?

On Sat, Jan 26, 2019, 8:59 AM Bojan <[email protected] wrote:

@snktagarwal https://github.com/snktagarwal Is #58 https://github.com/bojand/ghz/pull/58 like something to what you have in mind?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bojand/ghz/issues/24#issuecomment-457846857, or mute the thread https://github.com/notifications/unsubscribe-auth/AAG_Hyk1E81q6OrXihMyeYbtKYn1MW8Lks5vHIljgaJpZM4Vg_7b .

snktagarwal avatar Jan 26 '19 18:01 snktagarwal

Hello, with regards to status, yes with gRPC only OK status is a "successful" RPC call. I believe all other status codes result in an error on client-side. We record all status codes, but we only keep the latencies for successful calls and use them for the calculation of different metrics. This is really a design decision I had to make. I did consider recording all latencies and using them for calculations but in my experience and I think most people would ideally have only successful responses in benchmark testing and only care about the latencies of successful calls.

With regard to expectations do you mean actually measuring the time between message receives in streaming calls? I have added an issue for that and feel free to add more context, details and ideas there to further continue the discussion.

In addition, by expectations did you mean thresholds for response times? This is something that I do have planned for the complementary web application and I created an issue for it now. I did in fact have this feature in an original prototype, but I was not happy with it and rebuilt the app and have not gotten around to adding this feature yet. I am just not sure how thresholds this would work in the CLI context, but feel free to create an issue with some ideas.

bojand avatar Jan 27 '19 00:01 bojand