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

upgrade grpc to 1.23 #756

Merged
merged 4 commits into from
Oct 8, 2019
Merged

upgrade grpc to 1.23 #756

merged 4 commits into from
Oct 8, 2019

Conversation

july2993
Copy link
Contributor

@july2993 july2993 commented Sep 24, 2019

What problem does this PR solve?

After pingcap/tidb#12055 tidb upgrade grpc to 1.23, the client can't no longer connect to pump server.

What is changed and how it works?

upgrade to gRPC to 1.23 from 1.17.
change cmux how to match for gRPC, see:
soheilhy/cmux#64
https://github.com/soheilhy/cmux#limitations

this still compatible with the older version tidb(with gRPC client 1.17).

Check List

Tests

  • Unit test
  • Integration test
    run with 1.23 and 1.17 gRPC client version tidb

Code changes

Side effects

Related changes

  • Need to be included in the release note

@july2993
Copy link
Contributor Author

/run-all-tests

@july2993
Copy link
Contributor Author

/run-integration-tests tidb=release-3.0 tikv=release-3.0 pd=release-3.0

@july2993
Copy link
Contributor Author

/run-integration-tests tidb=release-3.0 tikv=release-3.0 pd=release-3.0

@july2993
Copy link
Contributor Author

/run-all-tests tidb=release-3.0 tikv=release-3.0 pd=release-3.0

Copy link
Contributor

@zier-one zier-one left a comment

Choose a reason for hiding this comment

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

LGTM

@IANTHEREAL
Copy link
Collaborator

Very Nice!LGTM

@IANTHEREAL
Copy link
Collaborator

/run-all-tests tidb=release-3.0 tikv=release-3.0 pd=release-3.0

Copy link
Collaborator

@IANTHEREAL IANTHEREAL left a comment

Choose a reason for hiding this comment

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

LGTM

@IANTHEREAL IANTHEREAL merged commit 11b9d1d into pingcap:master Oct 8, 2019
july2993 added a commit to july2993/tidb-binlog that referenced this pull request Oct 21, 2019
IANTHEREAL pushed a commit that referenced this pull request Oct 21, 2019
july2993 added a commit to july2993/tidb-binlog that referenced this pull request Nov 12, 2019
* go.mod: upgrade grpc to 1.23

* Fix  client blocks until it receives a SETTINGS frame from the server

* grpc.WithDialer is deprecated: use WithContextDialer instead.
IANTHEREAL pushed a commit that referenced this pull request Nov 12, 2019
july2993 added a commit to july2993/tidb-binlog that referenced this pull request Feb 14, 2020
* go.mod: upgrade grpc to 1.23

* Fix  client blocks until it receives a SETTINGS frame from the server

* grpc.WithDialer is deprecated: use WithContextDialer instead.
july2993 added a commit to july2993/tidb-binlog that referenced this pull request Feb 14, 2020
* go.mod: upgrade grpc to 1.23

* Fix  client blocks until it receives a SETTINGS frame from the server

* grpc.WithDialer is deprecated: use WithContextDialer instead.
july2993 added a commit to july2993/tidb-binlog that referenced this pull request Feb 14, 2020
* go.mod: upgrade grpc to 1.23

* Fix  client blocks until it receives a SETTINGS frame from the server

* grpc.WithDialer is deprecated: use WithContextDialer instead.
july2993 added a commit that referenced this pull request Feb 14, 2020
* go.mod: upgrade grpc to 1.23

* Fix  client blocks until it receives a SETTINGS frame from the server

* grpc.WithDialer is deprecated: use WithContextDialer instead.
@@ -376,7 +376,12 @@ func (s *Server) Start() error {
// sets a timeout for the read of matchers
m.SetReadTimeout(time.Second * 10)

grpcL := m.Match(cmux.HTTP2HeaderField("content-type", "application/grpc"))
// grpcL := m.Match(cmux.HTTP2HeaderField("content-type", "application/grpc"))
Copy link
Member

Choose a reason for hiding this comment

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

Why do we keep this line?

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 think we can and better just delete it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants