Prepar to upgrade grpc version: grpc naming packge is moved in v1.30.0 #12609

skyao · 2021-01-10T10:24:03Z

Target: we want to upgrade grpc version from current v1.29.1 to new version (like v1.32.0)

Background: in grpc version v1.30.0, the grpc naming package is removed, which is used by ectd naming package and grpcproxy package

More information, please refrence to:

#12398

…v1.30.0, which will effect ectd naming package and grpcproxy package

scDisorder · 2021-01-10T12:42:51Z

yay, so I think this one is better to merge instead of #12608 , bc naming interfaces moved to separate package
but there is an issue with integration test on race too

skyao · 2021-01-11T02:16:55Z

but there is an issue with integration test on race too

I tried to run command sudo HOST_TMP_DIR=/tmp TEST_OPTS="GOARCH=386 PASSES='build e2e'" make docker-test locally and found some error like this:

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

goroutine 83 [running]:
google.golang.org/protobuf/internal/impl.fieldInfoForScalar(0x18b8e00, 0xc0003c4000, 0x17555a1, 0x4, 0x0, 0x0, 0x18b8f20, 0x176d280, 0x17555a7, 0x45, ...)
	/Users/sky/work/soft/gopath/pkg/mod/google.golang.org/protobuf@v1.25.0/internal/impl/message_reflect_field.go:228 +0xa85
google.golang.org/protobuf/internal/impl.(*MessageInfo).makeKnownFieldsFunc(0xc00015a780, 0xffffffffffffffff, 0xffffffffffffffff, 0x150, 0xffffffffffffffff, 0xc000380b10, 0xc000380b40, 0xc000380b70, 0xc000380ba0)
	/Users/sky/work/soft/gopath/pkg/mod/google.golang.org/protobuf@v1.25.0/internal/impl/message_reflect.go:67 +0xcc5
google.golang.org/protobuf/internal/impl.(*MessageInfo).makeReflectFuncs(0xc00015a780, 0x18b8f20, 0x17c3540, 0xffffffffffffffff, 0xffffffffffffffff, 0x150, 0xffffffffffffffff, 0xc000380b10, 0xc000380b40, 0xc000380b70, ...)
	/Users/sky/work/soft/gopath/pkg/mod/google.golang.org/protobuf@v1.25.0/internal/impl/message_reflect.go:36 +0xca
google.golang.org/protobuf/internal/impl.(*MessageInfo).initOnce(0xc00015a780)
	/Users/sky/work/soft/gopath/pkg/mod/google.golang.org/protobuf@v1.25.0/internal/impl/message.go:90 +0x235
google.golang.org/protobuf/internal/impl.(*MessageInfo).init(0xc00015a780)
	/Users/sky/work/soft/gopath/pkg/mod/google.golang.org/protobuf@v1.25.0/internal/impl/message.go:72 +0x65
google.golang.org/protobuf/internal/impl.(*messageReflectWrapper).Has(0xc0003921f0, 0x18b8e00, 0xc0003c4000, 0xc0003c4000)
	/Users/sky/work/soft/gopath/pkg/mod/google.golang.org/protobuf@v1.25.0/internal/impl/message_reflect_gen.go:185 +0x5a
github.com/golang/protobuf/proto.(*textWriter).writeMessage(0xc000380870, 0x18b7340, 0xc0003921f0, 0x0, 0x0)
	/Users/sky/work/soft/gopath/pkg/mod/github.com/golang/protobuf@v1.4.2/proto/text_encode.go:278 +0xb62
github.com/golang/protobuf/proto.(*TextMarshaler).marshal(0x1b7b79c, 0x18af200, 0xc00039e480, 0x0, 0x0, 0x2c, 0x2c, 0x26)
	/Users/sky/work/soft/gopath/pkg/mod/github.com/golang/protobuf@v1.4.2/proto/text_encode.go:86 +0x2d6
github.com/golang/protobuf/proto.(*TextMarshaler).Text(...)
	/Users/sky/work/soft/gopath/pkg/mod/github.com/golang/protobuf@v1.4.2/proto/text_encode.go:44
github.com/golang/protobuf/proto.CompactTextString(...)
	/Users/sky/work/soft/gopath/pkg/mod/github.com/golang/protobuf@v1.4.2/proto/text_encode.go:106
go.etcd.io/etcd/v3/raft/raftpb.(*Message).String(...)
	/Users/sky/work/code/skyao/etcd/raft/raftpb/raft.pb.go:312
go.etcd.io/etcd/v3/raft.TestNodeProposeWaitDropped.func1(0xc000233180, 0x2, 0x0, 0x1, 0x0, 0x0, 0x0, 0xc0002f0f50, 0x1, 0x1, ...)
	/Users/sky/work/code/skyao/etcd/raft/node_test.go:462 +0x43c
go.etcd.io/etcd/v3/raft.(*raft).Step(0xc000233180, 0x2, 0x0, 0x1, 0x0, 0x0, 0x0, 0xc0002f0f50, 0x1, 0x1, ...)
	/Users/sky/work/code/skyao/etcd/raft/raft.go:990 +0xfd8
go.etcd.io/etcd/v3/raft.(*node).run(0xc00011f980)
	/Users/sky/work/code/skyao/etcd/raft/node.go:346 +0x10d8
created by go.etcd.io/etcd/v3/raft.TestNodeProposeWaitDropped
	/Users/sky/work/code/skyao/etcd/raft/node_test.go:474 +0x6de
FAIL	go.etcd.io/etcd/v3/raft	0.350s

And when I run sudo HOST_TMP_DIR=/tmp TEST_OPTS="GOARCH=amd64 PASSES='build e2e'" make docker-test locally, no such error. So it seems this error only happens in 386 arch?

skyao · 2021-01-11T02:49:48Z

For package naming in etcd, we need to make decision:

Proposal1. keep and mark as deprecated: as show in this PR
Proposal2. move to etcd grpcproxy as private package

I copied the comment from xiang90 from #12398:

@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.

skyao · 2021-01-11T02:51:13Z

For package naming in etcd, we need to make decision:

Proposal1. keep and mark as deprecated: as show in this PR
Proposal2. move to etcd grpcproxy as private package

I copied the comment from xiang90 from #12398:

@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.

Anyway, since grpc removed naming package and etcd naming package directly uses grpc naming package as method parameters, so there are must be some broken API changes when we update to new grpc version.

func (gr *GRPCResolver) Update(ctx context.Context, target string, nm naming.Update, opts ...etcd.OpOption) (err error) {}
func (gr *GRPCResolver) Resolve(target string) (naming.Watcher, error) {}
func (gw *gRPCWatcher) Next() ([]*naming.Update, error) {}

So the public API and document provided by etcd have to be updated ( with broken api changes): https://etcd.io/docs/v3.4.0/dev-guide/grpc_naming/

@ptabor @scDisorder How about proposal 2? If Ok, I will update this PR.

scDisorder · 2021-01-11T06:58:28Z

for me prop2 are nice, but we should wait for @ptabor or someone from owners

ptabor · 2021-01-11T20:32:57Z

A.d. #12609 (comment), I see there protobuf@v1.4.2 mentioned.
I don't see any changes to go.mod so I'm surprised by this dependency. It shouldn't happen as long as we stay on grpc-1.29.1.

A.d.. #12609 (comment) (Prop1, Prop2), let me do some experiment. I will go back by Tomorrow EOD, but intuitively to fullfill the usecase from (https://etcd.io/docs/v3.4.0/dev-guide/grpc_naming/) we need some code in client. Yes - it will be breaking change and we need to change the doc... but it's still supported by code in client (so I'm leaning toward some variant of prop1).

skyao · 2021-01-12T06:24:23Z

A.d. #12609 (comment), I see there protobuf@v1.4.2 mentioned.
I don't see any changes to go.mod so I'm surprised by this dependency. It shouldn't happen as long as we stay on grpc-1.29.1.

Yes, I'm also surprised for this error. And in #12608 , scDisorder also found the same error.

ptabor · 2021-01-12T07:38:10Z

SemaphoreCI: https://semaphoreci.com/etcd-io/etcd/branches/pull-request-12609/builds/1 failed due to TestIssue6361 flake that I'm fixing in #12611. I don't see there proto-1.4 related problem.

Also tried locally - but got success:

git remote add skyao git@github.com:skyao/etcd.git
git pull skyao
git checkout prepare-to-remove-grpc-naming-package
% HOST_TMP_DIR=/tmp TEST_OPTS="GOARCH=386 PASSES='build e2e'" make docker-test
GO_VERSION: 1.14.3
ETCD_VERSION: 87eb29949
TEST_OPTS: GOARCH=386 PASSES='build e2e'
...

go: downloading github.com/json-iterator/go v1.1.10
go: downloading github.com/modern-go/reflect2 v1.0.1
go: downloading go.uber.org/zap v1.16.0
go: downloading google.golang.org/grpc v1.29.1
go: downloading github.com/coreos/go-semver v0.3.0
go: downloading github.com/soheilhy/cmux v0.1.4
go: downloading github.com/prometheus/client_golang v1.5.1
go: downloading google.golang.org/genproto v0.0.0-20200513103714-09dca8ec2884
go: downloading golang.org/x/sys v0.0.0-20201009025420-dfb3f7c4e634
go: downloading github.com/dgrijalva/jwt-go v3.2.0+incompatible
go: downloading golang.org/x/net v0.0.0-20201006153459-a7d1128ccaa0
go: downloading github.com/golang/protobuf v1.3.5
go: downloading github.com/gogo/protobuf v1.3.1
go: downloading go.uber.org/multierr v1.5.0
go: downloading github.com/cespare/xxhash v1.1.0
go: downloading github.com/prometheus/common v0.10.0
go: downloading github.com/jonboulle/clockwork v0.2.2
go: downloading golang.org/x/time v0.0.0-20200630173020-3af7569d3a1e
go: downloading github.com/google/uuid v1.1.2
go: downloading github.com/beorn7/perks v1.0.1
go: downloading github.com/cespare/xxhash/v2 v2.1.1
go: downloading sigs.k8s.io/yaml v1.2.0
go: downloading github.com/tmc/grpc-websocket-proxy v0.0.0-20200427203606-3cfed13b9966
go: downloading go.uber.org/atomic v1.6.0
go: downloading github.com/grpc-ecosystem/go-grpc-middleware v1.2.2
go: downloading github.com/grpc-ecosystem/grpc-gateway v1.14.6
go: downloading golang.org/x/crypto v0.0.0-20201002170205-7f63de1d35b0
go: downloading github.com/gorilla/websocket v1.4.2
go: downloading github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2
go: downloading gopkg.in/yaml.v2 v2.3.0
go: downloading github.com/matttproud/golang_protobuf_extensions v1.0.1
go: downloading go.etcd.io/bbolt v1.3.5
go: downloading github.com/sirupsen/logrus v1.7.0
go: downloading github.com/coreos/go-systemd v0.0.0-20190321100706-95778dfbb74e
go: downloading golang.org/x/text v0.3.3
go: downloading github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0
go: downloading github.com/coreos/go-systemd/v22 v22.1.0
go: downloading github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd
go: downloading github.com/prometheus/procfs v0.2.0
go: downloading github.com/google/btree v1.0.0
go: downloading github.com/prometheus/client_model v0.2.0
go: downloading github.com/spf13/pflag v1.0.5
go: downloading github.com/dustin/go-humanize v1.0.0
Running with -cpu=4 --race=false

'build' started at Tue Jan 12 07:29:53 UTC 2021
Building etcd
% (cd api && 'go' 'build' './...')
stderr: go: downloading golang.org/x/net v0.0.0-20191002035440-2ec189313ef0
stderr: go: downloading golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a
stderr: go: downloading golang.org/x/text v0.3.0
% (cd pkg && 'go' 'build' './...')
stderr: go: downloading github.com/creack/pty v1.1.11
% (cd raft && 'go' 'build' './...')
stderr: go: downloading github.com/cockroachdb/datadriven v0.0.0-20200714090401-bf6692d28da5
stderr: go: downloading github.com/pmezard/go-difflib v1.0.0
stderr: go: downloading github.com/cockroachdb/errors v1.2.4
stderr: go: downloading github.com/cockroachdb/logtags v0.0.0-20190617123548-eb05cc24525f
stderr: go: downloading github.com/getsentry/raven-go v0.2.0
stderr: go: downloading github.com/pkg/errors v0.9.1
stderr: go: downloading github.com/certifi/gocertifi v0.0.0-20191021191039-0944d244cd40
% (cd client/v2 && 'go' 'build' './...')
stderr: go: downloading github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421
% (cd client/v3 && 'go' 'build' './...')
stderr: go: downloading gopkg.in/yaml.v2 v2.2.8
% (cd server && 'go' 'build' './...')
stderr: go: downloading github.com/golang/groupcache v0.0.0-20200121045136-8c9f03a8e57e
stderr: go: downloading github.com/spf13/cobra v1.1.1
% (cd etcdctl && 'go' 'build' './...')
stderr: go: downloading github.com/olekukonko/tablewriter v0.0.4
stderr: go: downloading gopkg.in/cheggaaa/pb.v1 v1.0.28
stderr: go: downloading github.com/bgentry/speakeasy v0.1.0
stderr: go: downloading github.com/urfave/cli v1.22.4
stderr: go: downloading github.com/mattn/go-runewidth v0.0.7
stderr: go: downloading github.com/cpuguy83/go-md2man/v2 v2.0.0
stderr: go: downloading github.com/russross/blackfriday/v2 v2.0.1
stderr: go: downloading github.com/shurcooL/sanitized_anchor_name v1.0.0
% (cd tests && 'go' 'build' './...')
stderr: go: downloading github.com/etcd-io/gofail v0.0.0-20190801230047-ad7f989257ca
% 'go' 'build' './...'
stderr: go: downloading github.com/mattn/go-runewidth v0.0.9
...

(no protobuf-1.4.2 on the list above)

'build' completed at Tue Jan 12 07:31:02 UTC 2021
'e2e' started at Tue Jan 12 07:31:02 UTC 2021
% (cd tests && 'env' 'go' 'test' '-timeout=30m' 'go.etcd.io/etcd/tests/v3/e2e')
ok  	go.etcd.io/etcd/tests/v3/e2e	360.781s

'e2e' completed at Tue Jan 12 07:37:08 UTC 2021
SUCCESS
! egrep "(--- FAIL:|DATA RACE|panic: test timed out|appears to have leaked)" -B50 -A10 test-MTYxMDQzNjU1OAo.log

ptabor · 2021-01-12T23:51:09Z

A.d.. #12609 (comment) (Prop1, Prop2), let me do some experiment. I will go back by Tomorrow EOD, but intuitively to fullfill the usecase from (https://etcd.io/docs/v3.4.0/dev-guide/grpc_naming/) we need some code in client. Yes - it will be breaking change and we need to change the doc... but it's still supported by code in client (so I'm leaning toward some variant of prop1).

As promised: As the target state for 3.5 naming API I would imagine following layout:
#12614

TLDR: Please take a look at the proposed modified 3.5 naming user guide:
https://github.com/etcd-io/etcd/pull/12614/files?short_path=5329e8f#diff-5329e8f4ea030d3052a34c8831bfc948bc5100e935c4cff4175fe75cbd9ee877

ptabor · 2021-01-18T13:17:55Z

I'm sorry: I pushed my clone of this branch that had not updated origin and (to my surprise) changed your exported branch.

I recovered the original (Your) commit: 87eb299

skyao · 2021-01-30T22:58:16Z

@ptabor So what's the status now? Should I close this PR?

ptabor · 2021-01-31T10:12:14Z

Yes. Let's close it. I'm sorry for chiming in and taking up your time.

#12652 tracks the current plan and help needed.

prepare for grpc new version: grpc naming packge is moved since grpc …

87eb299

…v1.30.0, which will effect ectd naming package and grpcproxy package

skyao changed the title ~~upgrade grpc version: prepare for grpc new version in which naming packge is moved.~~ Prepar to upgrade grpc version: grpc naming packge is moved in v1.30.0 Jan 10, 2021

skyao mentioned this pull request Jan 10, 2021

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

Closed

scDisorder mentioned this pull request Jan 11, 2021

do not use grpc/naming package #12608

Closed

ptabor mentioned this pull request Jan 15, 2021

Prepar to upgrade grpc client version to v1.35.0 #12626

Closed

dhermes mentioned this pull request Jan 20, 2021

Upgrade to google.golang.org/grpc@1.35.0 #12636

Closed

ptabor closed this Jan 31, 2021

skyao deleted the prepare-to-remove-grpc-naming-package branch February 3, 2021 03:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prepar to upgrade grpc version: grpc naming packge is moved in v1.30.0 #12609

Prepar to upgrade grpc version: grpc naming packge is moved in v1.30.0 #12609

skyao commented Jan 10, 2021

scDisorder commented Jan 10, 2021 •

edited

Loading

skyao commented Jan 11, 2021 •

edited

Loading

skyao commented Jan 11, 2021

skyao commented Jan 11, 2021 •

edited

Loading

scDisorder commented Jan 11, 2021

ptabor commented Jan 11, 2021

skyao commented Jan 12, 2021

ptabor commented Jan 12, 2021

ptabor commented Jan 12, 2021

ptabor commented Jan 18, 2021

skyao commented Jan 30, 2021

ptabor commented Jan 31, 2021

Prepar to upgrade grpc version: grpc naming packge is moved in v1.30.0 #12609

Prepar to upgrade grpc version: grpc naming packge is moved in v1.30.0 #12609

Conversation

skyao commented Jan 10, 2021

scDisorder commented Jan 10, 2021 • edited Loading

skyao commented Jan 11, 2021 • edited Loading

skyao commented Jan 11, 2021

skyao commented Jan 11, 2021 • edited Loading

scDisorder commented Jan 11, 2021

ptabor commented Jan 11, 2021

skyao commented Jan 12, 2021

ptabor commented Jan 12, 2021

ptabor commented Jan 12, 2021

ptabor commented Jan 18, 2021

skyao commented Jan 30, 2021

ptabor commented Jan 31, 2021

scDisorder commented Jan 10, 2021 •

edited

Loading

skyao commented Jan 11, 2021 •

edited

Loading

skyao commented Jan 11, 2021 •

edited

Loading