benchmark: add latency/MTU/bandwidth into testcases #1304

ZhouyihaiDing · 2017-06-13T22:15:48Z

Add latency, MTU and bandwidth into benchmark cases.

make benchmark only run all testcases matches "trace, MTU=infinite, latency=infinite" in order to reduce the run time to 5 minutes.

To run exact one testcase, the command is
go test benchmark/benchmark17_test.go benchmark/benchmark.go -benchmem -bench=BenchmarkClient/Stream-Tracing-kbps_0-MTU_0-maxConcurrentCalls_512-reqSize_1024-respSize_1-latency_0s

Because the benchmark uses the simulated network implemented by latency.go which contains extra operation, the simulated no delay is slower than ideal no delay.

menghanl · 2017-06-16T22:48:32Z

benchmark/benchmark.go

@@ -137,8 +138,11 @@ type ServerInfo struct {

 // StartServer starts a gRPC server serving a benchmark service according to info.
 // It returns its listen address and a function to stop the server.
-func StartServer(info ServerInfo, opts ...grpc.ServerOption) (string, func()) {
+func StartServer(nw *latency.Network, info ServerInfo, opts ...grpc.ServerOption) (string, func()) {


Instead of adding one more parameter to the function, why not add Network into struct ServeInfo?
So you don't need to change all the caller of StartServer

menghanl · 2017-06-17T00:10:01Z

benchmark/benchmark.go

 	s := stats.AddStats(b, 38)
+	nw := &latency.Network{Kbps: kbps, Latency: ltc, MTU: mtu}
+	nw.Kbps = -1


What's the purpose of this?
This function should always set nw.Kbps to the parameter kbps passed in.

If we want to run benchmarks only with kbps -1, we should make sure this function is called with kbps=-1, instead of setting nw.Kbps to -1 inside the function.

Forgot to delete when I was testing

menghanl · 2017-06-17T00:14:46Z

benchmark/benchmark.go

+		grpc.WithInsecure(),
+		grpc.WithDialer(
+			func(address string, timeout time.Duration) (net.Conn, error) {
+				clientConn, err := nw.TimeoutDialer(net.DialTimeout)("tcp", address, timeout)


I would prefer to name the returned conn just conn. clientConn usually means a gRPC client.

menghanl · 2017-06-17T00:15:28Z

benchmark/benchmark.go

+			func(address string, timeout time.Duration) (net.Conn, error) {
+				clientConn, err := nw.TimeoutDialer(net.DialTimeout)("tcp", address, timeout)
+				if err != nil {
+					b.Fatalf("Unexpected error dialing: %v", err)


Return nil, err instead of fatal?

menghanl · 2017-06-17T00:16:14Z

benchmark/benchmark.go

+		grpc.WithInsecure(),
+		grpc.WithDialer(
+			func(address string, timeout time.Duration) (net.Conn, error) {
+				clientConn, err := nw.TimeoutDialer(net.DialTimeout)("tcp", address, timeout)


Change this dialer, too.

menghanl · 2017-06-17T00:18:42Z

benchmark/server/main.go

@@ -28,7 +29,8 @@ func main() {
 			grpclog.Fatalf("Failed to serve: %v", err)
 		}
 	}()
-	addr, stopper := benchmark.StartServer(benchmark.ServerInfo{Addr: ":0", Type: "protobuf"}) // listen on all interfaces
+	nw := &latency.Network{}
+	addr, stopper := benchmark.StartServer(nw, benchmark.ServerInfo{Addr: ":0", Type: "protobuf"}) // listen on all interfaces


Pass a nil network instead.
If you move nw into ServerInfo as what I suggested in previous comment, this line doesn't need to be changed.

menghanl · 2017-06-17T00:19:41Z

benchmark/worker/benchmark_server.go

 	if config.PayloadConfig != nil {
 		switch payload := config.PayloadConfig.Payload.(type) {
 		case *testpb.PayloadConfig_BytebufParams:
 			opts = append(opts, grpc.CustomCodec(byteBufCodec{}))
-			addr, closeFunc = benchmark.StartServer(benchmark.ServerInfo{
+			addr, closeFunc = benchmark.StartServer(nw, benchmark.ServerInfo{


See my comment in server/main.go.

menghanl · 2017-06-17T00:21:32Z

benchmark/stats/stats.go

@@ -85,7 +85,8 @@ func (stats *Stats) maybeUpdate() {
 	stats.histogram = NewHistogram(HistogramOptions{
 		NumBuckets: numBuckets,
 		// max-min(lower bound of last bucket) = (1 + growthFactor)^(numBuckets-2) * baseBucketSize.
-		GrowthFactor:   math.Pow(float64(stats.max-stats.min), 1/float64(numBuckets-2)) - 1,
+		// GrowthFactor can't be 0
+		GrowthFactor:   math.Pow(float64(stats.max-stats.min), 1/float64(numBuckets-2)) - 0.999,


Changing 1 to 0.999 seems arbitrary.

In what situation will this be 0?

When (stat.max-stat.min = 1). There is a parameter oneOverLogOnePlusGrowthFactor: 1 / math.Log(1+opts.GrowthFactor), which mean GrowthFactor can't be 0. I can also add a 0.001 to oneOverLogOnePlusGrowthFactor but it is the same.
The reason it happens is because the latency is divided by a int64 time unit. For example, If adding 40ms latency to the network, it may take 80.xxx ms for a round trip, so the final result only contains 80ms(stat.min) and 81ms(stat.max).

menghanl · 2017-06-21T00:38:56Z

benchmark/benchmark.go

+		grpc.WithInsecure(),
+		grpc.WithDialer(
+			func(address string, timeout time.Duration) (net.Conn, error) {
+				connTimeout, err := nw.TimeoutDialer(net.DialTimeout)("tcp", address, timeout)


Can return directly here:

return nw.TimeoutDialer(net.DialTimeout)("tcp", address, timeout)

Also, nit, I would do this in remove some of the indentation

conn := NewClientConn( target, grpc.WithInsecure(), grpc.WithDialer(func(address string, timeout time.Duration) (net.Conn, error) { return nw.TimeoutDialer(net.DialTimeout)("tcp", address, timeout) }), )

menghanl · 2017-06-21T00:39:41Z

benchmark/benchmark.go

+		grpc.WithInsecure(),
+		grpc.WithDialer(
+			func(address string, timeout time.Duration) (net.Conn, error) {
+				connTimeout, err := nw.TimeoutDialer(net.DialTimeout)("tcp", address, timeout)


Same as runUnary

return nw.TimeoutDialer(net.DialTimeout)("tcp", address, timeout)

menghanl · 2017-06-21T00:40:20Z

benchmark/benchmark.go

 }

 // StartServer starts a gRPC server serving a benchmark service according to info.
 // It returns its listen address and a function to stop the server.
 func StartServer(info ServerInfo, opts ...grpc.ServerOption) (string, func()) {
 	lis, err := net.Listen("tcp", info.Addr)
+	nw := info.Network
+	if nw != nil {


Check the error before this.

ZhouyihaiDing added 2 commits June 13, 2017 15:08

benchmark: add latency/MTU/bandwidth into testcases

5ecae41

update benchmark16_test.go

460c5f0

menghanl self-assigned this Jun 16, 2017

menghanl reviewed Jun 17, 2017

View reviewed changes

ZhouyihaiDing added 2 commits June 19, 2017 16:01

update

f46616e

update formate

0dd7e74

menghanl reviewed Jun 21, 2017

View reviewed changes

ZhouyihaiDing added 6 commits June 21, 2017 14:59

update

757d94a

pass golint

fcba863

change var name

8d768d2

adding 1M to request/response size

f68529d

format

d076282

update

cef2696

menghanl approved these changes Jun 23, 2017

View reviewed changes

menghanl merged commit f0c566b into grpc:master Jun 23, 2017

lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

benchmark: add latency/MTU/bandwidth into testcases #1304

benchmark: add latency/MTU/bandwidth into testcases #1304

ZhouyihaiDing commented Jun 13, 2017

menghanl Jun 16, 2017

menghanl Jun 17, 2017

ZhouyihaiDing Jun 19, 2017

menghanl Jun 17, 2017

menghanl Jun 17, 2017

menghanl Jun 17, 2017

menghanl Jun 17, 2017

menghanl Jun 17, 2017

menghanl Jun 17, 2017

ZhouyihaiDing Jun 19, 2017

menghanl Jun 21, 2017

menghanl Jun 21, 2017

menghanl Jun 21, 2017

benchmark: add latency/MTU/bandwidth into testcases #1304

benchmark: add latency/MTU/bandwidth into testcases #1304

Conversation

ZhouyihaiDing commented Jun 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment