Skip to content

stress: client logs total num of calls made #6762

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 6, 2023
Merged

stress: client logs total num of calls made #6762

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
88d80b1
stress: client logs total num of calls made
temawi Nov 3, 2023
44a6117
Use int64 and moree efficient incrementing
temawi Nov 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions stress/client/main.go
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can do that, but rather in a later PR so that this commit stays focused on one thing.

Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"strconv"
"strings"
"sync"
"sync/atomic"
"time"

"google.golang.org/grpc"
Expand Down Expand Up @@ -55,7 +56,8 @@ var (
tlsServerName = flag.String("server_host_override", "foo.test.google.fr", "The server name use to verify the hostname returned by TLS handshake if it is not empty. Otherwise, --server_host is used.")
caFile = flag.String("ca_file", "", "The file containing the CA root cert file")

logger = grpclog.Component("stress")
totalNumCalls int64
logger = grpclog.Component("stress")
)

// testCaseWithWeight contains the test case type and its weight.
Expand Down Expand Up @@ -206,7 +208,6 @@ func startServer(server *server, port int) {
s := grpc.NewServer()
metricspb.RegisterMetricsServiceServer(s, server)
s.Serve(lis)

}

// performRPCs uses weightedRandomTestSelector to select test case and runs the tests.
Expand Down Expand Up @@ -241,6 +242,7 @@ func performRPCs(gauge *gauge, conn *grpc.ClientConn, selector *weightedRandomTe
interop.DoCustomMetadata(client, grpc.WaitForReady(true))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Later PR... Same reasons as with moving to under /interop.

}
numCalls++
defer func() { atomic.AddInt64(&totalNumCalls, numCalls) }()
gauge.set(int64(float64(numCalls) / time.Since(startTime).Seconds()))

select {
Expand Down Expand Up @@ -335,6 +337,6 @@ func main() {
close(stop)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(#6763 FYI)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to chat with you a bit more about this, not changing in this PR.

}
wg.Wait()
logger.Infof("Total calls made: %v", totalNumCalls)
logger.Infof(" ===== ALL DONE ===== ")

}