From ef70aa153d193d890c148918175a09fce4efb4c8 Mon Sep 17 00:00:00 2001 From: Yuxuan Li Date: Fri, 13 Oct 2017 16:30:42 -0700 Subject: [PATCH] fix json string indentation, minor fixes --- clientconn.go | 13 +- rpc_util.go | 39 ------ service_config.go | 43 +++++- test/end2end_test.go | 321 ++++++++++++++++++++----------------------- 4 files changed, 195 insertions(+), 221 deletions(-) diff --git a/clientconn.go b/clientconn.go index 3648380b0439..5af7d9f52a14 100644 --- a/clientconn.go +++ b/clientconn.go @@ -793,13 +793,14 @@ func (cc *ClientConn) getTransport(ctx context.Context, failfast bool) (transpor // struct ServiceConfig, and store both the struct and the JSON string in ClientConn. func (cc *ClientConn) handleServiceConfig(js string) error { sc, err := parseServiceConfig(js) - if err == nil { - cc.mu.Lock() - cc.scRaw = js - cc.sc = sc - cc.mu.Unlock() + if err != nil { + return err } - return err + cc.mu.Lock() + cc.scRaw = js + cc.sc = sc + cc.mu.Unlock() + return nil } // Close tears down the ClientConn and all underlying connections. diff --git a/rpc_util.go b/rpc_util.go index d6a8ed13958d..2e5f2cd9c4ff 100644 --- a/rpc_util.go +++ b/rpc_util.go @@ -493,45 +493,6 @@ func Errorf(c codes.Code, format string, a ...interface{}) error { return status.Errorf(c, format, a...) } -// MethodConfig defines the configuration recommended by the service providers for a -// particular method. -// This is EXPERIMENTAL and subject to change. -type MethodConfig struct { - // WaitForReady indicates whether RPCs sent to this method should wait until - // the connection is ready by default (!failfast). The value specified via the - // gRPC client API will override the value set here. - WaitForReady *bool - // Timeout is the default timeout for RPCs sent to this method. The actual - // deadline used will be the minimum of the value specified here and the value - // set by the application via the gRPC client API. If either one is not set, - // then the other will be used. If neither is set, then the RPC has no deadline. - Timeout *time.Duration - // MaxReqSize is the maximum allowed payload size for an individual request in a - // stream (client->server) in bytes. The size which is measured is the serialized - // payload after per-message compression (but before stream compression) in bytes. - // The actual value used is the minimum of the value specified here and the value set - // by the application via the gRPC client API. If either one is not set, then the other - // will be used. If neither is set, then the built-in default is used. - MaxReqSize *int - // MaxRespSize is the maximum allowed payload size for an individual response in a - // stream (server->client) in bytes. - MaxRespSize *int -} - -// ServiceConfig is provided by the service provider and contains parameters for how -// clients that connect to the service should behave. -// This is EXPERIMENTAL and subject to change. -type ServiceConfig struct { - // LB is the load balancer the service providers recommends. The balancer specified - // via grpc.WithBalancer will override this. - LB *string - // Methods contains a map for the methods in this service. - // If there is an exact match for a method (i.e. /service/method) in the map, use the corresponding MethodConfig. - // If there's no exact match, look for the default config for the service (/service/) and use the corresponding MethodConfig if it exists. - // Otherwise, the method has no MethodConfig to use. - Methods map[string]MethodConfig -} - func min(a, b *int) *int { if *a < *b { return a diff --git a/service_config.go b/service_config.go index 469dc7c44c49..caf10ec53686 100644 --- a/service_config.go +++ b/service_config.go @@ -25,6 +25,45 @@ import ( "google.golang.org/grpc/grpclog" ) +// MethodConfig defines the configuration recommended by the service providers for a +// particular method. +// This is EXPERIMENTAL and subject to change. +type MethodConfig struct { + // WaitForReady indicates whether RPCs sent to this method should wait until + // the connection is ready by default (!failfast). The value specified via the + // gRPC client API will override the value set here. + WaitForReady *bool + // Timeout is the default timeout for RPCs sent to this method. The actual + // deadline used will be the minimum of the value specified here and the value + // set by the application via the gRPC client API. If either one is not set, + // then the other will be used. If neither is set, then the RPC has no deadline. + Timeout *time.Duration + // MaxReqSize is the maximum allowed payload size for an individual request in a + // stream (client->server) in bytes. The size which is measured is the serialized + // payload after per-message compression (but before stream compression) in bytes. + // The actual value used is the minimum of the value specified here and the value set + // by the application via the gRPC client API. If either one is not set, then the other + // will be used. If neither is set, then the built-in default is used. + MaxReqSize *int + // MaxRespSize is the maximum allowed payload size for an individual response in a + // stream (server->client) in bytes. + MaxRespSize *int +} + +// ServiceConfig is provided by the service provider and contains parameters for how +// clients that connect to the service should behave. +// This is EXPERIMENTAL and subject to change. +type ServiceConfig struct { + // LB is the load balancer the service providers recommends. The balancer specified + // via grpc.WithBalancer will override this. + LB *string + // Methods contains a map for the methods in this service. + // If there is an exact match for a method (i.e. /service/method) in the map, use the corresponding MethodConfig. + // If there's no exact match, look for the default config for the service (/service/) and use the corresponding MethodConfig if it exists. + // Otherwise, the method has no MethodConfig to use. + Methods map[string]MethodConfig +} + func parseTimeout(t *string) (*time.Duration, error) { if t == nil { return nil, nil @@ -38,7 +77,7 @@ type jsonName struct { Method *string `json:"method,omitempty"` } -func (j jsonName) String() (string, bool) { +func (j jsonName) generatePath() (string, bool) { if j.Service == nil { return "", false } @@ -94,7 +133,7 @@ func parseServiceConfig(js string) (ServiceConfig, error) { MaxRespSize: m.MaxResponseMessageBytes, } for _, n := range *m.Name { - if path, valid := n.String(); valid { + if path, valid := n.generatePath(); valid { sc.Methods[path] = mc } } diff --git a/test/end2end_test.go b/test/end2end_test.go index 2a1a7ca212a3..a9ec10f038fa 100644 --- a/test/end2end_test.go +++ b/test/end2end_test.go @@ -1238,39 +1238,36 @@ func TestGetMethodConfig(t *testing.T) { cc := te.clientConn() r.NewAddress([]resolver.Address{{Addr: te.srvAddr}}) r.NewServiceConfig(`{ - "methodConfig": [ - { - "name": [ - { - "service": "grpc.testing.TestService", - "method": "EmptyCall" - } - ], - "waitForReady": true, - "timeout": "1ms" - }, - { - "name": [ - { - "service": "grpc.testing.TestService" - } - ], - "waitForReady": false - } - ] + "methodConfig": [ + { + "name": [ + { + "service": "grpc.testing.TestService", + "method": "EmptyCall" + } + ], + "waitForReady": true, + "timeout": "1ms" + }, + { + "name": [ + { + "service": "grpc.testing.TestService" + } + ], + "waitForReady": false + } + ] }`) tc := testpb.NewTestServiceClient(cc) // Make sure service config has been processed by grpc. - mc := cc.GetMethodConfig("/grpc.testing.TestService/EmptyCall") for { - if mc.WaitForReady == nil { - time.Sleep(time.Millisecond) - mc = cc.GetMethodConfig("/grpc.testing.TestService/EmptyCall") - continue + if cc.GetMethodConfig("/grpc.testing.TestService/EmptyCall").WaitForReady != nil { + break } - break + time.Sleep(time.Millisecond) } // The following RPCs are expected to become non-fail-fast ones with 1ms deadline. @@ -1280,37 +1277,34 @@ func TestGetMethodConfig(t *testing.T) { } r.NewServiceConfig(`{ - "methodConfig": [ - { - "name": [ - { - "service": "grpc.testing.TestService", - "method": "UnaryCall" - } - ], - "waitForReady": true, - "timeout": "1ms" - }, - { - "name": [ - { - "service": "grpc.testing.TestService" - } - ], - "waitForReady": false - } - ] + "methodConfig": [ + { + "name": [ + { + "service": "grpc.testing.TestService", + "method": "UnaryCall" + } + ], + "waitForReady": true, + "timeout": "1ms" + }, + { + "name": [ + { + "service": "grpc.testing.TestService" + } + ], + "waitForReady": false + } + ] }`) // Make sure service config has been processed by grpc. - mc = cc.GetMethodConfig("/grpc.testing.TestService/EmptyCall") for { - if mc.WaitForReady == nil || *mc.WaitForReady { - time.Sleep(time.Millisecond) - mc = cc.GetMethodConfig("/grpc.testing.TestService/EmptyCall") - continue + if mc := cc.GetMethodConfig("/grpc.testing.TestService/EmptyCall"); mc.WaitForReady != nil && !*mc.WaitForReady { + break } - break + time.Sleep(time.Millisecond) } // The following RPCs are expected to become fail-fast. if _, err = tc.EmptyCall(context.Background(), &testpb.Empty{}); grpc.Code(err) != codes.Unavailable { @@ -1330,35 +1324,32 @@ func TestServiceConfigWaitForReady(t *testing.T) { cc := te.clientConn() r.NewAddress([]resolver.Address{{Addr: te.srvAddr}}) r.NewServiceConfig(`{ - "methodConfig": [ - { - "name": [ - { - "service": "grpc.testing.TestService", - "method": "EmptyCall" - }, - { - "service": "grpc.testing.TestService", - "method": "FullDuplexCall" - } - ], - "waitForReady": false, - "timeout": "1ms" - } - ] + "methodConfig": [ + { + "name": [ + { + "service": "grpc.testing.TestService", + "method": "EmptyCall" + }, + { + "service": "grpc.testing.TestService", + "method": "FullDuplexCall" + } + ], + "waitForReady": false, + "timeout": "1ms" + } + ] }`) tc := testpb.NewTestServiceClient(cc) // Make sure service config has been processed by grpc. - mc := cc.GetMethodConfig("/grpc.testing.TestService/FullDuplexCall") for { - if mc.WaitForReady == nil { - time.Sleep(time.Millisecond) - mc = cc.GetMethodConfig("/grpc.testing.TestService/FullDuplexCall") - continue + if cc.GetMethodConfig("/grpc.testing.TestService/FullDuplexCall").WaitForReady != nil { + break } - break + time.Sleep(time.Millisecond) } // The following RPCs are expected to become non-fail-fast ones with 1ms deadline. @@ -1373,33 +1364,30 @@ func TestServiceConfigWaitForReady(t *testing.T) { // Generate a service config update. // Case2:Client API set failfast to be false, and service config set wait_for_ready to be true, and the rpc will wait until deadline exceeds. r.NewServiceConfig(`{ - "methodConfig": [ - { - "name": [ - { - "service": "grpc.testing.TestService", - "method": "EmptyCall" - }, - { - "service": "grpc.testing.TestService", - "method": "FullDuplexCall" - } - ], - "waitForReady": true, - "timeout": "1ms" - } - ] + "methodConfig": [ + { + "name": [ + { + "service": "grpc.testing.TestService", + "method": "EmptyCall" + }, + { + "service": "grpc.testing.TestService", + "method": "FullDuplexCall" + } + ], + "waitForReady": true, + "timeout": "1ms" + } + ] }`) // Wait for the new service config to take effect. - mc = cc.GetMethodConfig("/grpc.testing.TestService/EmptyCall") for { - if mc.WaitForReady == nil || !*mc.WaitForReady { - time.Sleep(time.Millisecond) - mc = cc.GetMethodConfig("/grpc.testing.TestService/EmptyCall") - continue + if mc := cc.GetMethodConfig("/grpc.testing.TestService/EmptyCall"); mc.WaitForReady != nil && *mc.WaitForReady { + break } - break + time.Sleep(time.Millisecond) } // The following RPCs are expected to become non-fail-fast ones with 1ms deadline. if _, err := tc.EmptyCall(context.Background(), &testpb.Empty{}); grpc.Code(err) != codes.DeadlineExceeded { @@ -1422,35 +1410,32 @@ func TestServiceConfigTimeout(t *testing.T) { cc := te.clientConn() r.NewAddress([]resolver.Address{{Addr: te.srvAddr}}) r.NewServiceConfig(`{ - "methodConfig": [ - { - "name": [ - { - "service": "grpc.testing.TestService", - "method": "EmptyCall" - }, - { - "service": "grpc.testing.TestService", - "method": "FullDuplexCall" - } - ], - "waitForReady": true, - "timeout": "1h" - } - ] + "methodConfig": [ + { + "name": [ + { + "service": "grpc.testing.TestService", + "method": "EmptyCall" + }, + { + "service": "grpc.testing.TestService", + "method": "FullDuplexCall" + } + ], + "waitForReady": true, + "timeout": "1h" + } + ] }`) tc := testpb.NewTestServiceClient(cc) // Make sure service config has been processed by grpc. - mc := cc.GetMethodConfig("/grpc.testing.TestService/FullDuplexCall") for { - if mc.Timeout == nil { - time.Sleep(time.Millisecond) - mc = cc.GetMethodConfig("/grpc.testing.TestService/FullDuplexCall") - continue + if cc.GetMethodConfig("/grpc.testing.TestService/FullDuplexCall").Timeout != nil { + break } - break + time.Sleep(time.Millisecond) } // The following RPCs are expected to become non-fail-fast ones with 1ns deadline. @@ -1470,33 +1455,30 @@ func TestServiceConfigTimeout(t *testing.T) { // Generate a service config update. // Case2: Client API sets timeout to be 1hr and ServiceConfig sets timeout to be 1ns. Timeout should be 1ns (min of 1ns and 1hr) and the rpc will wait until deadline exceeds. r.NewServiceConfig(`{ - "methodConfig": [ - { - "name": [ - { - "service": "grpc.testing.TestService", - "method": "EmptyCall" - }, - { - "service": "grpc.testing.TestService", - "method": "FullDuplexCall" - } - ], - "waitForReady": true, - "timeout": "1ns" - } - ] + "methodConfig": [ + { + "name": [ + { + "service": "grpc.testing.TestService", + "method": "EmptyCall" + }, + { + "service": "grpc.testing.TestService", + "method": "FullDuplexCall" + } + ], + "waitForReady": true, + "timeout": "1ns" + } + ] }`) // Wait for the new service config to take effect. - mc = cc.GetMethodConfig("/grpc.testing.TestService/FullDuplexCall") for { - if mc.Timeout == nil || *mc.Timeout != time.Nanosecond { - time.Sleep(time.Millisecond) - mc = cc.GetMethodConfig("/grpc.testing.TestService/FullDuplexCall") - continue + if mc := cc.GetMethodConfig("/grpc.testing.TestService/FullDuplexCall"); mc.Timeout != nil && *mc.Timeout == time.Nanosecond { + break } - break + time.Sleep(time.Millisecond) } ctx, cancel = context.WithTimeout(context.Background(), time.Hour) @@ -1537,22 +1519,22 @@ func TestServiceConfigMaxMsgSize(t *testing.T) { } scjs := `{ - "methodConfig": [ - { - "name": [ - { - "service": "grpc.testing.TestService", - "method": "UnaryCall" - }, - { - "service": "grpc.testing.TestService", - "method": "FullDuplexCall" - } - ], - "maxRequestMessageBytes": 2048, - "maxResponseMessageBytes": 2048 - } - ] + "methodConfig": [ + { + "name": [ + { + "service": "grpc.testing.TestService", + "method": "UnaryCall" + }, + { + "service": "grpc.testing.TestService", + "method": "FullDuplexCall" + } + ], + "maxRequestMessageBytes": 2048, + "maxResponseMessageBytes": 2048 + } + ] }` // Case1: sc set maxReqSize to 2048 (send), maxRespSize to 2048 (recv). @@ -1574,14 +1556,11 @@ func TestServiceConfigMaxMsgSize(t *testing.T) { Payload: smallPayload, } - mc := cc1.GetMethodConfig("/grpc.testing.TestService/FullDuplexCall") for { - if mc.MaxReqSize == nil { - time.Sleep(time.Millisecond) - mc = cc1.GetMethodConfig("/grpc.testing.TestService/FullDuplexCall") - continue + if cc1.GetMethodConfig("/grpc.testing.TestService/FullDuplexCall").MaxReqSize != nil { + break } - break + time.Sleep(time.Millisecond) } // Test for unary RPC recv. @@ -1643,14 +1622,11 @@ func TestServiceConfigMaxMsgSize(t *testing.T) { r.NewServiceConfig(scjs) tc = testpb.NewTestServiceClient(cc2) - mc = cc2.GetMethodConfig("/grpc.testing.TestService/FullDuplexCall") for { - if mc.MaxReqSize == nil { - time.Sleep(time.Millisecond) - mc = cc2.GetMethodConfig("/grpc.testing.TestService/FullDuplexCall") - continue + if cc2.GetMethodConfig("/grpc.testing.TestService/FullDuplexCall").MaxReqSize != nil { + break } - break + time.Sleep(time.Millisecond) } // Test for unary RPC recv. @@ -1708,14 +1684,11 @@ func TestServiceConfigMaxMsgSize(t *testing.T) { r.NewServiceConfig(scjs) tc = testpb.NewTestServiceClient(cc3) - mc = cc3.GetMethodConfig("/grpc.testing.TestService/FullDuplexCall") for { - if mc.MaxReqSize == nil { - time.Sleep(time.Millisecond) - mc = cc3.GetMethodConfig("/grpc.testing.TestService/FullDuplexCall") - continue + if cc3.GetMethodConfig("/grpc.testing.TestService/FullDuplexCall").MaxReqSize != nil { + break } - break + time.Sleep(time.Millisecond) } // Test for unary RPC recv.