stress: client logs total num of calls made #6762
Conversation
|
@dfawley, who could review this? |
| case "status_code_and_message": | ||
| interop.DoStatusCodeAndMessage(client, grpc.WaitForReady(true)) | ||
| case "custom_metadata": | ||
| interop.DoCustomMetadata(client, grpc.WaitForReady(true)) |
There was a problem hiding this comment.
I don't think we want WaitForReady anymore. There are probably hysterical raisins for why it was once there, but it can be removed now. Or in a later PR; either way.
There was a problem hiding this comment.
Later PR... Same reasons as with moving to under /interop.
There was a problem hiding this comment.
Can you move all of this stuff under /interop (i.e. cd grpc-go; mv stress interop/)? I don't think it really wants to be a top-level thing.
There was a problem hiding this comment.
I can do that, but rather in a later PR so that this commit stays focused on one thing.
| go startServer(metricsServer, *metricsPort) | ||
| if *testDurationSecs > 0 { | ||
| time.Sleep(time.Duration(*testDurationSecs) * time.Second) | ||
| close(stop) |
There was a problem hiding this comment.
Optional for this PR but would be nice to fix up soon if we are reviving this:
More natural and simpler would be:
ctx := context.Background
if *testDurationSecs > 0 {
ctx, cancel = context.WithTimeout(ctx, *testDurationSecs * time.Second)
defer cancel()
}
........
performRPCs(ctx, g, conn, testSelector)
......
func performRPCs() {
....
for ctx.Err() == nil {
....
}
}Even better would be to then pass ctx to interop.DoBlah but that probably affects other things too. But it's following a bad practice and should be cleaned up one day. I'll file an issue for that.
There was a problem hiding this comment.
I need to chat with you a bit more about this, not changing in this PR.
2 participants
Needed for an internal, DirectPath related, stress test.
RELEASE NOTES: none