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

Go Client Package can't be used in applications importing golang.org/x/net/trace #7884

Closed
zgiber opened this issue Oct 12, 2018 · 17 comments
Closed
Assignees
Labels
type/question The issue belongs to a question.

Comments

@zgiber
Copy link

zgiber commented Oct 12, 2018

Bug Report

Please answer these questions before submitting your issue. Thanks!

  1. What did you do?
    Developing a GRPC application which also requires to use tikv client.

  2. What did you expect to see?
    No errors were expected on importing packages.

  3. What did you see instead?
    The GRPC package indirectly imports golang.org/x/net/trace which clashes with the vendored version used by the tikv client.

panic: /debug/requests is already registered. You may have two independent copies of golang.org/x/net/trace in your binary, trying to maintain separate state. This may involve a vendored copy of golang.org/x/net/trace.

goroutine 1 [running]:
golang.org/x/net/trace.init.0()
	/Users/zoltangiber/go/src/golang.org/x/net/trace/trace.go:116 +0x16c
exit status 2
  1. What version of TiDB are you using (tidb-server -V or run select tidb_version(); on TiDB)?
    TIKV client from tip d21f294393f5c6cb442cd6eb16e0a468f2d1483c

You can easily reproduce the issue by running the following code (it's non functional, but crashes before application logic is executed):

package main

import (
	"github.com/pingcap/tidb/config"
	"github.com/pingcap/tidb/store/tikv"
	"golang.org/x/net/trace"
)

func main() {
	_, _ = tikv.NewRawKVClient([]string{"127.0.0.1:2379"}, config.Security{})
	_ = trace.New("", "")
}
@jackysp jackysp added the type/question The issue belongs to a question. label Oct 12, 2018
@jackysp
Copy link
Member

jackysp commented Oct 12, 2018

Thanks for your report, @zgiber !
PTAL @tiancaiamao .

@zgiber
Copy link
Author

zgiber commented Oct 15, 2018

@jackysp Thanks for looking into it. We're at a point where we'd like to decide whether to go with TiKV or an alternative, this seems to be a showstopper. It would be a big help if golang.org/x/net/trace package was not vendored although I understand that you have reasons why it is.

@shenli
Copy link
Member

shenli commented Oct 15, 2018

@zgiber What's your go version?

@zgiber
Copy link
Author

zgiber commented Oct 15, 2018

@shenli go version go1.11.1 darwin/amd64 recently upgraded from 1.10.2 both versions can reproduce the issue.

@zhexuany
Copy link
Contributor

@zgiber Could you post the error message here? It will be really helpful.

@tiancaiamao
Copy link
Contributor

I'm asking the golang team how to handle this case. golang/go#28207

@zgiber

@zgiber
Copy link
Author

zgiber commented Oct 15, 2018

Sure, (the error message got slightly improved in 1.11):

panic: /debug/requests is already registered. You may have two independent copies of golang.org/x/net/trace in your binary, trying to maintain separate state. This may involve a vendored copy of golang.org/x/net/trace.

goroutine 1 [running]:
golang.org/x/net/trace.init.0()
	/Users/zoltangiber/go/src/golang.org/x/net/trace/trace.go:116 +0x16c
exit status 2

(updated initial post with error message)

@gregwebs
Copy link
Contributor

Using go modules or even Gopkg could fix this. Gopkg seemed to hang forever on dep init. With go modules I have a dependency error I didn't quite understand:

GO111MODULE=on go build -o main main.go
...
go: finding github.com/matttproud/golang_protobuf_extensions/pbutil latest
# github.com/pingcap/tidb/store/tikv
../../../../pkg/mod/github.com/pingcap/tidb@v2.0.7+incompatible/store/tikv/coprocessor.go:72:7: undefined: tipb.ExprType_And
...
../../../../pkg/mod/github.com/pingcap/tidb@v2.0.7+incompatible/store/tikv/coprocessor.go:76:39: too many errors

@zgiber
Copy link
Author

zgiber commented Oct 15, 2018

Yes go modules is a feasible workaround would be better though if it wasn't necessary.

@gregwebs
Copy link
Contributor

Is that an issue for tidb or golang? We can do the switch to gomodules for tidb ourselves if that fixes the issue. We are already starting the switch to gomodules in the pd repo, but it probably wouldn't be part of an official release until 2.1 at the earliest.

@shenli
Copy link
Member

shenli commented Oct 15, 2018

@gregwebs parser.go will be a blocking issue of switching to gomodules. It is generated from parser.y.

@gregwebs
Copy link
Contributor

I think we would just need to check in parser.go and vendor all tooling with retool. But that also shouldn't be a dependency for the TiKV client, so its also possible to look at making the TiKV client available separately.

@tiancaiamao
Copy link
Contributor

tiancaiamao commented Oct 16, 2018

The go team think current behavior is intended, so there is no better choice but some workarounds:

  1. Modify the source of golang.org/x/net/trace, either in GOPATH or vendor, register "/debug/requests" only once

This modification is quick and dirty, package golang.org/x/net/trace still been imported twice, and it's unclear which one is used.

  1. The "put all things in vendor" way

TiDB put all things in vendor, if a project imports TiKV client from TiDB, it also need a vendor and maybe tools like glide would solve the vendor package version conflict.

The vendor works like a virus, if one import a project with vendor, his project probably need one.

  1. Standard go module way

TiDB haven't employ the Go1.11 module yet, and there is no time plan either, although it should be.

@thinktainer
Copy link

I think being able to use tikv standalone would be great.

@tiancaiamao
Copy link
Contributor

tikv/tikv#3828 @zgiber
I tried on my local machine and now importing golang.org/x/net/trace doesn't panic.
My go.mod looks like this:

module hello

require (
	cloud.google.com/go v0.33.1 // indirect
	github.com/blacktear23/go-proxyprotocol v0.0.0-20180807104634-af7a81e8dd0d // indirect
	github.com/cznic/mathutil v0.0.0-20181122101859-297441e03548 // indirect
	github.com/cznic/sortutil v0.0.0-20181122101858-f5f958428db8 // indirect
	github.com/go-sql-driver/mysql v1.4.1 // indirect
	github.com/golang/lint v0.0.0-20181026193005-c67002cb31c3 // indirect
	github.com/juju/errors v0.0.0-20181118221551-089d3ea4e4d5 // indirect
	github.com/klauspost/cpuid v1.2.0 // indirect
	github.com/pingcap/kvproto v0.0.0-20181123124450-d48563486f61 // indirect
	github.com/pingcap/parser v0.0.0-20181125101201-e4d76a6014ba // indirect
	github.com/pingcap/tidb v0.0.0-20181123114736-1e0876fe810a // indirect
	github.com/pingcap/tidb-tools v2.1.0-rc.5+incompatible // indirect
	github.com/prometheus/client_golang v0.9.1 // indirect
	github.com/prometheus/common v0.0.0-20181120120127-aeab699e26f4 // indirect
	golang.org/x/crypto v0.0.0-20181112202954-3d3f9f413869 // indirect
	golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3 // indirect
	golang.org/x/net v0.0.0-20181114220301-adae6a3d119a // indirect
	golang.org/x/oauth2 v0.0.0-20181120190819-8f65e3013eba // indirect
	golang.org/x/sync v0.0.0-20181108010431-42b317875d0f // indirect
	golang.org/x/sys v0.0.0-20181122145206-62eef0e2fa9b // indirect
	golang.org/x/tools v0.0.0-20181122213734-04b5d21e00f1 // indirect
	google.golang.org/appengine v1.3.0 // indirect
	google.golang.org/genproto v0.0.0-20181109154231-b5d43981345b // indirect
	honnef.co/go/tools v0.0.0-20180920025451-e3ad64cb4ed3 // indirect
)

@tiancaiamao
Copy link
Contributor

FYI @disksing

@thinktainer
Copy link

Yep, all good, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/question The issue belongs to a question.
Projects
None yet
Development

No branches or pull requests

7 participants