Skip to content
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

Jump to bottom

update grpc-go version to v1.32.0 which has some breaking api changes #12398

Closed
wants to merge 4 commits into from
Closed

update grpc-go version to v1.32.0 which has some breaking api changes #12398

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2859e9f
update grpc-go version to v1.32.0 which has some breaking api changes…
skyao Oct 15, 2020
4d66d4b
downgrade github.com/golang/protobuf version to 1.3.5
skyao Oct 22, 2020
b1d600e
add doc for method
skyao Oct 22, 2020
5ba58f2
update to pass fmt checking
skyao Oct 22, 2020
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
39 changes: 31 additions & 8 deletions clientv3/balancer/balancer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,15 +136,34 @@ type baseBalancer struct {
connectivityRecorder connectivity.Recorder

picker picker.Picker

resolverErr error // the last error reported by the resolver; cleared on successful resolution
}

// HandleResolvedAddrs implements "grpc/balancer.Balancer" interface.
// gRPC sends initial or updated resolved addresses from "Build".
func (bb *baseBalancer) HandleResolvedAddrs(addrs []resolver.Address, err error) {
if err != nil {
bb.lg.Warn("HandleResolvedAddrs called with error", zap.String("balancer-id", bb.id), zap.Error(err))
func (bb *baseBalancer) ResolverError(err error) {
skyao marked this conversation as resolved.
Show resolved Hide resolved
bb.resolverErr = err
if len(bb.addrToSc) == 0 {
bb.connectivityRecorder.RecordTransition(bb.connectivityRecorder.GetCurrentState(), grpcconnectivity.TransientFailure)
}

if bb.connectivityRecorder.GetCurrentState() != grpcconnectivity.TransientFailure {
// The picker will not change since the balancer does not currently
// report an error.
return
}
bb.updatePicker()
bb.currentConn.UpdateState(balancer.State{
ConnectivityState: bb.connectivityRecorder.GetCurrentState(),
Picker: bb.picker,
})
}

// UpdateClientConnState implements "grpc/balancer.Balancer" interface.
func (bb *baseBalancer) UpdateClientConnState(state balancer.ClientConnState) error {
addrs := state.ResolverState.Addresses
// Successful resolution; clear resolver error and ensure we return nil.
bb.resolverErr = nil

bb.lg.Info("resolved",
zap.String("picker", bb.picker.String()),
zap.String("balancer-id", bb.id),
Expand Down Expand Up @@ -191,10 +210,14 @@ func (bb *baseBalancer) HandleResolvedAddrs(addrs []resolver.Address, err error)
// (DO NOT) delete(bb.scToSt, sc)
}
}

return nil
}

// HandleSubConnStateChange implements "grpc/balancer.Balancer" interface.
func (bb *baseBalancer) HandleSubConnStateChange(sc balancer.SubConn, s grpcconnectivity.State) {
// UpdateSubConnState implements "grpc/balancer.Balancer" interface.
func (bb *baseBalancer) UpdateSubConnState(sc balancer.SubConn, state balancer.SubConnState) {
s := state.ConnectivityState

bb.mu.Lock()
defer bb.mu.Unlock()

Expand Down Expand Up @@ -247,7 +270,7 @@ func (bb *baseBalancer) HandleSubConnStateChange(sc balancer.SubConn, s grpcconn
bb.updatePicker()
}

bb.currentConn.UpdateBalancerState(bb.connectivityRecorder.GetCurrentState(), bb.picker)
bb.currentConn.UpdateState(balancer.State{ConnectivityState: bb.connectivityRecorder.GetCurrentState(), Picker: bb.picker})
}

func (bb *baseBalancer) updatePicker() {
Expand Down
6 changes: 2 additions & 4 deletions clientv3/balancer/picker/err.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
package picker

import (
"context"

"google.golang.org/grpc/balancer"
)

Expand All @@ -34,6 +32,6 @@ func (ep *errPicker) String() string {
return ep.p.String()
}

func (ep *errPicker) Pick(context.Context, balancer.PickInfo) (balancer.SubConn, func(balancer.DoneInfo), error) {
return nil, nil, ep.err
func (ep *errPicker) Pick(balancer.PickInfo) (balancer.PickResult, error) {
return balancer.PickResult{}, ep.err
}
7 changes: 3 additions & 4 deletions clientv3/balancer/picker/roundrobin_balanced.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package picker

import (
"context"
"sync"

"go.uber.org/zap"
Expand Down Expand Up @@ -52,12 +51,12 @@ type rrBalanced struct {
func (rb *rrBalanced) String() string { return rb.p.String() }

// Pick is called for every client request.
func (rb *rrBalanced) Pick(ctx context.Context, opts balancer.PickInfo) (balancer.SubConn, func(balancer.DoneInfo), error) {
func (rb *rrBalanced) Pick(opts balancer.PickInfo) (balancer.PickResult, error) {
rb.mu.RLock()
n := len(rb.scs)
rb.mu.RUnlock()
if n == 0 {
return nil, nil, balancer.ErrNoSubConnAvailable
return balancer.PickResult{}, balancer.ErrNoSubConnAvailable
}

rb.mu.Lock()
Expand Down Expand Up @@ -91,5 +90,5 @@ func (rb *rrBalanced) Pick(ctx context.Context, opts balancer.PickInfo) (balance
rb.lg.Warn("balancer failed", fss...)
}
}
return sc, doneFunc, nil
return balancer.PickResult{SubConn: sc, Done: doneFunc}, nil
}
27 changes: 13 additions & 14 deletions clientv3/naming/grpc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,10 @@ import (
"context"
"encoding/json"
"fmt"

Copy link
Contributor

Choose a reason for hiding this comment

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

@skyao can we mark this package as deprecated? Better, we should move it into the grpcproxy package as an internal implementation detail.

There is no point to expose this anymore since gRPC already stop supporting it. When our users bump gRPC the same version as the etcd client, they cannot use this package anyway. If they use an older version of gRPC, they should also be using an older version of etcd client with naming support.

So we will lose nothing anyway.

etcd "go.etcd.io/etcd/v3/clientv3"
gnaming "go.etcd.io/etcd/v3/clientv3/naming/grpcnaming"

"google.golang.org/grpc/codes"
"google.golang.org/grpc/naming"
"google.golang.org/grpc/status"
)

Expand All @@ -34,23 +33,23 @@ type GRPCResolver struct {
Client *etcd.Client
}

func (gr *GRPCResolver) Update(ctx context.Context, target string, nm naming.Update, opts ...etcd.OpOption) (err error) {
func (gr *GRPCResolver) Update(ctx context.Context, target string, nm gnaming.Update, opts ...etcd.OpOption) (err error) {
switch nm.Op {
case naming.Add:
case gnaming.Add:
var v []byte
if v, err = json.Marshal(nm); err != nil {
return status.Error(codes.InvalidArgument, err.Error())
}
_, err = gr.Client.KV.Put(ctx, target+"/"+nm.Addr, string(v), opts...)
case naming.Delete:
case gnaming.Delete:
_, err = gr.Client.Delete(ctx, target+"/"+nm.Addr, opts...)
default:
return status.Error(codes.InvalidArgument, "naming: bad naming op")
}
return err
}

func (gr *GRPCResolver) Resolve(target string) (naming.Watcher, error) {
func (gr *GRPCResolver) Resolve(target string) (gnaming.Watcher, error) {
ctx, cancel := context.WithCancel(context.Background())
w := &gRPCWatcher{c: gr.Client, target: target + "/", ctx: ctx, cancel: cancel}
return w, nil
Expand All @@ -68,7 +67,7 @@ type gRPCWatcher struct {
// Next gets the next set of updates from the etcd resolver.
// Calls to Next should be serialized; concurrent calls are not safe since
// there is no way to reconcile the update ordering.
func (gw *gRPCWatcher) Next() ([]*naming.Update, error) {
func (gw *gRPCWatcher) Next() ([]*gnaming.Update, error) {
if gw.wch == nil {
// first Next() returns all addresses
return gw.firstNext()
Expand All @@ -87,17 +86,17 @@ func (gw *gRPCWatcher) Next() ([]*naming.Update, error) {
return nil, gw.err
}

updates := make([]*naming.Update, 0, len(wr.Events))
updates := make([]*gnaming.Update, 0, len(wr.Events))
for _, e := range wr.Events {
var jupdate naming.Update
var jupdate gnaming.Update
var err error
switch e.Type {
case etcd.EventTypePut:
err = json.Unmarshal(e.Kv.Value, &jupdate)
jupdate.Op = naming.Add
jupdate.Op = gnaming.Add
case etcd.EventTypeDelete:
err = json.Unmarshal(e.PrevKv.Value, &jupdate)
jupdate.Op = naming.Delete
jupdate.Op = gnaming.Delete
default:
continue
}
Expand All @@ -108,17 +107,17 @@ func (gw *gRPCWatcher) Next() ([]*naming.Update, error) {
return updates, nil
}

func (gw *gRPCWatcher) firstNext() ([]*naming.Update, error) {
func (gw *gRPCWatcher) firstNext() ([]*gnaming.Update, error) {
// Use serialized request so resolution still works if the target etcd
// server is partitioned away from the quorum.
resp, err := gw.c.Get(gw.ctx, gw.target, etcd.WithPrefix(), etcd.WithSerializable())
if gw.err = err; err != nil {
return nil, err
}

updates := make([]*naming.Update, 0, len(resp.Kvs))
updates := make([]*gnaming.Update, 0, len(resp.Kvs))
for _, kv := range resp.Kvs {
var jupdate naming.Update
var jupdate gnaming.Update
if err := json.Unmarshal(kv.Value, &jupdate); err != nil {
continue
}
Expand Down
73 changes: 73 additions & 0 deletions clientv3/naming/grpcnaming/naming.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/*
*
* Copyright 2014 gRPC authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/

// Package naming defines the naming API and related data structures for gRPC.
//
// This package is deprecated: please use package resolver instead.


// Notice: this file is a copy of naming/naming.go from grpc-go v1.29.1.
// The package of grpc naming is removed since grpc-go v1.30.0.
// This is a work around to make etcd work with grpc new version (>=v1.30.0) without too many code change.
package grpcnaming

// Operation defines the corresponding operations for a name resolution change.
//
// Deprecated: please use package resolver.
type Operation uint8

const (
// Add indicates a new address is added.
Add Operation = iota
// Delete indicates an existing address is deleted.
Delete
)

// Update defines a name resolution update. Notice that it is not valid having both
// empty string Addr and nil Metadata in an Update.
//
// Deprecated: please use package resolver.
type Update struct {
// Op indicates the operation of the update.
Op Operation
// Addr is the updated address. It is empty string if there is no address update.
Addr string
// Metadata is the updated metadata. It is nil if there is no metadata update.
// Metadata is not required for a custom naming implementation.
Metadata interface{}
}

// Resolver creates a Watcher for a target to track its resolution changes.
//
// Deprecated: please use package resolver.
type Resolver interface {
// Resolve creates a Watcher for target.
Resolve(target string) (Watcher, error)
}

// Watcher watches for the updates on the specified target.
//
// Deprecated: please use package resolver.
type Watcher interface {
// Next blocks until an update or error happens. It may return one or more
// updates. The first call should get the full set of the results. It should
// return an error if and only if Watcher cannot recover.
Next() ([]*Update, error)
// Close closes the Watcher.
Close()
}
16 changes: 8 additions & 8 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ require (
github.com/dustin/go-humanize v0.0.0-20171111073723-bb3d318650d4
github.com/fatih/color v1.7.0 // indirect
github.com/gogo/protobuf v1.3.1
github.com/golang/groupcache v0.0.0-20160516000752-02826c3e7903
github.com/golang/protobuf v1.3.5
github.com/golang/groupcache v0.0.0-20200121045136-8c9f03a8e57e
github.com/golang/protobuf v1.4.2
Copy link
Contributor

Choose a reason for hiding this comment

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

Updating above protobuf 1.3.x is creating runtime errors, please check:

I've seen:

panic: field raftpb.Message.type has invalid type: got raftpb.MessageType, want pointer

goroutine 57 [running]:
google.golang.org/protobuf/internal/impl.fieldInfoForScalar(0xd38e40, 0xc00034ca80, 0xbaca27, 0x4, 0x0, 0x0, 0xd38f60, 0xbcd160, 0xbaca2d, 0x45, ...)
	/home/ptab/private/golang/pkg/mod/google.golang.org/protobuf@v1.25.0/internal/impl/message_reflect_field.go:228 +0x7d7
google.golang.org/protobuf/internal/impl.(*MessageInfo).makeKnownFieldsFunc(0xc000166780, 0x180, 0xffffffffffffffff, 0x168, 0xffffffffffffffff, 0xc00035c240, 0xc00035c270, 0xc00035c2a0, 0xc00035c2d0)
	/home/ptab/private/golang/pkg/mod/google.golang.org/protobuf@v1.25.0/internal/impl/message_reflect.go:67 +0x97d
google.golang.org/protobuf/internal/impl.(*MessageInfo).makeReflectFuncs(0xc000166780, 0xd38f60, 0xc4aee0, 0x180, 0xffffffffffffffff, 0x168, 0xffffffffffffffff, 0xc00035c240, 0xc00035c270, 0xc00035c2a0, ...)
	/home/ptab/private/golang/pkg/mod/google.golang.org/protobuf@v1.25.0/internal/impl/message_reflect.go:36 +0x65
google.golang.org/protobuf/internal/impl.(*MessageInfo).initOnce(0xc000166780)
	/home/ptab/private/golang/pkg/mod/google.golang.org/protobuf@v1.25.0/internal/impl/message.go:90 +0x192
google.golang.org/protobuf/internal/impl.(*MessageInfo).init(...)
	/home/ptab/private/golang/pkg/mod/google.golang.org/protobuf@v1.25.0/internal/impl/message.go:72
google.golang.org/protobuf/internal/impl.(*messageReflectWrapper).Has(0xc000323300, 0xd38e40, 0xc00034ca80, 0xc00034ca80)
	/home/ptab/private/golang/pkg/mod/google.golang.org/protobuf@v1.25.0/internal/impl/message_reflect_gen.go:185 +0xf9
github.com/golang/protobuf/proto.(*textWriter).writeMessage(0xc000339f80, 0xd37080, 0xc000323300, 0x0, 0x0)
	/home/ptab/private/golang/pkg/mod/github.com/golang/protobuf@v1.4.2/proto/text_encode.go:278 +0x935
github.com/golang/protobuf/proto.(*TextMarshaler).marshal(0x10cb480, 0xd2cb40, 0xc000353ba0, 0xc000100210, 0x112a6c0, 0xc0001001e0, 0xc8deb0, 0xc0001dc118)
	/home/ptab/private/golang/pkg/mod/github.com/golang/protobuf@v1.4.2/proto/text_encode.go:86 +0x190
github.com/golang/protobuf/proto.(*TextMarshaler).Text(...)
	/home/ptab/private/golang/pkg/mod/github.com/golang/protobuf@v1.4.2/proto/text_encode.go:44
github.com/golang/protobuf/proto.CompactTextString(...)
	/home/ptab/private/golang/pkg/mod/github.com/golang/protobuf@v1.4.2/proto/text_encode.go:106
go.etcd.io/etcd/v3/raft/raftpb.(*Message).String(...)
	/home/ptab/corp/etcd/raft/raftpb/raft.pb.go:409
go.etcd.io/etcd/v3/raft.TestNodeProposeWaitDropped.func1(0xc0001f9a40, 0x2, 0x0, 0x1, 0x0, 0x0, 0x0, 0xc000358410, 0x1, 0x1, ...)
	/home/ptab/corp/etcd/raft/node_test.go:462 +0x31d
go.etcd.io/etcd/v3/raft.(*raft).Step(0xc0001f9a40, 0x2, 0x0, 0x1, 0x0, 0x0, 0x0, 0xc000358410, 0x1, 0x1, ...)
	/home/ptab/corp/etcd/raft/raft.go:990 +0xa58
go.etcd.io/etcd/v3/raft.(*node).run(0xc0003485a0)
	/home/ptab/corp/etcd/raft/node.go:346 +0xc1c
created by go.etcd.io/etcd/v3/raft.TestNodeProposeWaitDropped
	/home/ptab/corp/etcd/raft/node_test.go:474 +0x545
FAIL	go.etcd.io/etcd/v3/raft	0.124s

on test -short -timeout=3m -cpu=4 --race=false ./...

It would be good to stay on 1.3.5 till we will replace gogo with a generator that supports 1.4.x

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 checked the grpc code (go.mod file ) that grpc v1.32.0 only requires protobuf 1.3.3:

github.com/golang/protobuf v1.3.3

Why it changed to v1.4.2?

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 have added a commit to downgrade the github.com/golang/protobuf version back to 1.3.5, the lasted release before v1.4.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

by command go get github.com/golang/protobuf@v1.3.5

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 found that when I execute command "go mod tidy", the version of github.com/golang/protobuf will auto changed to version v1.4.2.

github.com/google/btree v1.0.0
github.com/google/uuid v1.0.0
github.com/gorilla/websocket v0.0.0-20170926233335-4201258b820c // indirect
Expand Down Expand Up @@ -40,12 +40,12 @@ require (
go.etcd.io/etcd/api/v3 v3.0.0-00010101000000-000000000000
go.etcd.io/etcd/pkg/v3 v3.0.0-00010101000000-000000000000
go.uber.org/zap v1.15.0
golang.org/x/crypto v0.0.0-20191002192127-34f69633bfdc
golang.org/x/net v0.0.0-20191002035440-2ec189313ef0
golang.org/x/text v0.3.3 // indirect
golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2
google.golang.org/genproto v0.0.0-20200513103714-09dca8ec2884
google.golang.org/grpc v1.29.1
golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9
golang.org/x/net v0.0.0-20200707034311-ab3426394381
golang.org/x/time v0.0.0-20191024005414-555d28b269f0
google.golang.org/genproto v0.0.0-20200806141610-86f49bd18e98
google.golang.org/grpc v1.32.0
google.golang.org/grpc/examples v0.0.0-20201014215113-7b167fd6eca1 // indirect
gopkg.in/cheggaaa/pb.v1 v1.0.25
sigs.k8s.io/yaml v1.1.0
)
Expand Down
Loading