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.

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

panic: invalid Go type int for field go_etcd_io.etcd.etcdserver.etcdserverpb.loggablePutRequest.value_size #11992

Closed
dims opened this issue Jun 9, 2020 · 4 comments · Fixed by #12000 or tikv/pd#5343

Comments

@dims
Copy link
Contributor

dims commented Jun 9, 2020

Originally found by @bart0sh in kubernetes logs: kubernetes/test-infra#17743 (comment)

panic: invalid Go type int for field go_etcd_io.etcd.etcdserver.etcdserverpb.loggablePutRequest.value_size

goroutine 1 [running]:
google.golang.org/protobuf/internal/impl.newSingularConverter(0x177ec20, 0x15b9580, 0x177eb00, 0xc0002a55e0, 0x1777201, 0xc000128fa0)
        /Users/dims/go/pkg/mod/google.golang.org/[email protected]/internal/impl/convert.go:143 +0xff5
google.golang.org/protobuf/internal/impl.NewConverter(0x177ec20, 0x15b9580, 0x177eb00, 0xc0002a55e0, 0x203000, 0xc0001e1470)
        /Users/dims/go/pkg/mod/google.golang.org/[email protected]/internal/impl/convert.go:60 +0xd9
google.golang.org/protobuf/internal/impl.fieldInfoForScalar(0x177eb00, 0xc0002a55e0, 0x15b1656, 0x9, 0x0, 0x0, 0x177ec20, 0x15b9580, 0x15b1661, 0x2e, ...)
        /Users/dims/go/pkg/mod/google.golang.org/[email protected]/internal/impl/message_reflect_field.go:234 +0x110
google.golang.org/protobuf/internal/impl.(*MessageInfo).makeKnownFieldsFunc(0xc0002cc640, 0xffffffffffffffff, 0xffffffffffffffff, 0xffffffffffffffff, 0xffffffffffffffff, 0xc0001e1380, 0xc0001e13b0, 0xc0001e13e0, 0xc0001e1410)
        /Users/dims/go/pkg/mod/google.golang.org/[email protected]/internal/impl/message_reflect.go:67 +0x938
google.golang.org/protobuf/internal/impl.(*MessageInfo).makeReflectFuncs(0xc0002cc640, 0x177ec20, 0x165aaa0, 0xffffffffffffffff, 0xffffffffffffffff, 0xffffffffffffffff, 0xffffffffffffffff, 0xc0001e1380, 0xc0001e13b0, 0xc0001e13e0, ...)
        /Users/dims/go/pkg/mod/google.golang.org/[email protected]/internal/impl/message_reflect.go:36 +0x63
google.golang.org/protobuf/internal/impl.(*MessageInfo).initOnce(0xc0002cc640)
        /Users/dims/go/pkg/mod/google.golang.org/[email protected]/internal/impl/message.go:90 +0x182
google.golang.org/protobuf/internal/impl.(*MessageInfo).init(0xc0002cc640)
        /Users/dims/go/pkg/mod/google.golang.org/[email protected]/internal/impl/message.go:72 +0x3c
google.golang.org/protobuf/internal/impl.(*messageReflectWrapper).Has(0xc0002ca560, 0x177eb00, 0xc0002a5500, 0xc0002a5500)
        /Users/dims/go/pkg/mod/google.golang.org/[email protected]/internal/impl/message_reflect_gen.go:185 +0x33
github.com/golang/protobuf/proto.(*textWriter).writeMessage(0xc0001e1350, 0x177b660, 0xc0002ca560, 0x0, 0x0)
        /Users/dims/go/pkg/mod/github.com/golang/[email protected]/proto/text_encode.go:278 +0x925
github.com/golang/protobuf/proto.(*TextMarshaler).marshal(0x1b4c43c, 0x1772aa0, 0xc0001e12f0, 0x169d650, 0x9, 0x1b6c800, 0xf58, 0xf58)
        /Users/dims/go/pkg/mod/github.com/golang/[email protected]/proto/text_encode.go:86 +0x18a
github.com/golang/protobuf/proto.(*TextMarshaler).Text(...)
        /Users/dims/go/pkg/mod/github.com/golang/[email protected]/proto/text_encode.go:44
github.com/golang/protobuf/proto.CompactTextString(...)
        /Users/dims/go/pkg/mod/github.com/golang/[email protected]/proto/text_encode.go:106
go.etcd.io/etcd/etcdserver/etcdserverpb.(*loggablePutRequest).String(...)
        /Users/dims/go/pkg/mod/go.etcd.io/[email protected]/etcdserver/etcdserverpb/raft_internal_stringer.go:182
main.main()
        /Users/dims/go/src/k8s.io/kubernetes/main.go:9 +0xe0

Process finished with exit code 2

Easy to reproduce with the following snippet

package main

import (
	pb "go.etcd.io/etcd/etcdserver/etcdserverpb"
)

func main() {
	var req pb.PutRequest
	pb.NewLoggablePutRequest(&req).String()
}
@dims
Copy link
Contributor Author

dims commented Jun 9, 2020

possibly from github.com/golang/protobuf v1.4.2

full go.mod to recreate is:

module example.com/m

go 1.13

require (
	github.com/coreos/etcd v3.3.22+incompatible // indirect
	github.com/gogo/protobuf v1.3.1 // indirect
	github.com/golang/protobuf v1.4.2
	go.etcd.io/etcd v3.3.22+incompatible // indirect
	google.golang.org/grpc v1.26.0
)

No issues if we use github.com/golang/protobuf v1.3.5

@dims
Copy link
Contributor Author

dims commented Jun 9, 2020

a simple solution seems to be to ensure value_string is int64:

diff --git a/etcdserver/etcdserverpb/raft_internal_stringer.go b/etcdserver/etcdserverpb/raft_internal_stringer.go
index 3d3536a32..c5fb813ad 100644
--- a/etcdserver/etcdserverpb/raft_internal_stringer.go
+++ b/etcdserver/etcdserverpb/raft_internal_stringer.go
@@ -160,7 +160,7 @@ func (*loggableValueCompare) ProtoMessage()    {}
 // To preserve proto encoding of the key bytes, a faked out proto type is used here.
 type loggablePutRequest struct {
        Key         []byte `protobuf:"bytes,1,opt,name=key,proto3"`
-       ValueSize   int    `protobuf:"varint,2,opt,name=value_size,proto3"`
+       ValueSize   int64  `protobuf:"varint,2,opt,name=value_size,proto3"`
        Lease       int64  `protobuf:"varint,3,opt,name=lease,proto3"`
        PrevKv      bool   `protobuf:"varint,4,opt,name=prev_kv,proto3"`
        IgnoreValue bool   `protobuf:"varint,5,opt,name=ignore_value,proto3"`
@@ -170,7 +170,7 @@ type loggablePutRequest struct {
 func NewLoggablePutRequest(request *PutRequest) *loggablePutRequest {
        return &loggablePutRequest{
                request.Key,
-               len(request.Value),
+               int64(len(request.Value)),
                request.Lease,
                request.PrevKv,
                request.IgnoreValue,

@jpbetz
Copy link
Contributor

jpbetz commented Jun 10, 2020

@YoyinZyc Would you be willing to have a quick look? The solution by @dims looks reasonable. Maybe just make sure we apply that everywhere it is needed?

@YoyinZyc
Copy link
Contributor

I think it looks good to me. Let me prepare a PR to apply the change to everywhere that it is needed.

gerrit-photon pushed a commit to vmware/photon that referenced this issue Sep 8, 2020
etcd service panics at start. This is
etcd-io/etcd#11992

Patch is added to fix this issue.

Change-Id: I6bc7da1ee930fc99592c48341f2ae87281892d3f
Reviewed-on: http://photon-jenkins.eng.vmware.com:8082/11067
Tested-by: gerrit-photon <[email protected]>
Reviewed-by: Ankit Jain <[email protected]>
gerrit-photon pushed a commit to vmware/photon that referenced this issue Sep 8, 2020
etcd service panics at start. This is
etcd-io/etcd#11992

Patch is added to fix this issue.

Change-Id: If136d084b90ea9565a63f78260907c94ec66996b
Reviewed-on: http://photon-jenkins.eng.vmware.com:8082/11066
Tested-by: gerrit-photon <[email protected]>
Reviewed-by: Ankit Jain <[email protected]>
ti-chi-bot pushed a commit to tikv/pd that referenced this issue Jul 21, 2022
close #5342, fix etcd-io/etcd#11992

*: upgrade etcd to v3.4.19

Signed-off-by: nolouch <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants