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

binlog: use pumps client to write binlog (#7659) (#8078) #8822

Merged
merged 1 commit into from Dec 26, 2018

Conversation

WangXiangUSTC
Copy link
Contributor

@WangXiangUSTC WangXiangUSTC commented Dec 25, 2018

What problem does this PR solve?

support new binlog in v2.0.10

What is changed and how it works?

git cherry-pick
pr:
#7659
#8078

Check List

Tests

  • Integration test

This change is Reviewable

@WangXiangUSTC
Copy link
Contributor Author

/run-all-tests tidb-test=release-2.0 tikv=release-2.0 pd=release-2.0

@WangXiangUSTC
Copy link
Contributor Author

/run-integration-compatibility-test

@shenli
Copy link
Member

shenli commented Dec 25, 2018

/run-integration-compatibility-test tidb-test=release-2.0 tikv=release-2.0 pd=release-2.0

@shenli
Copy link
Member

shenli commented Dec 25, 2018

/run-integration-ddl-test tidb-test=release-2.0 tikv=release-2.0 pd=release-2.0

@zhouqiang-cl
Copy link
Contributor

/run-integration-compatibility-test tidb-test=release-2.0 tikv=release-2.0 pd=release-2.0 tidb=release-2.0

"google.golang.org/grpc"
)

func init() {
grpc.EnableTracing = false
// don't need output pumps client's log
pumpcli.Logger.Out = ioutil.Discard
Copy link
Contributor

Choose a reason for hiding this comment

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

why not to output?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

at the first, zhou sir don't want pump client print any log, it may disturb tidb's log. so I add this code.
we can refine pump client's log later, and remove this line if any other reviewer agree.

Copy link
Contributor

Choose a reason for hiding this comment

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

hard to debug, create an issue for it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK,I will create a issue

@IANTHEREAL
Copy link
Contributor

LGTM

@IANTHEREAL IANTHEREAL added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 25, 2018
@winkyao
Copy link
Contributor

winkyao commented Dec 26, 2018

@WangXiangUSTC ddl-test failed by the changes of https://github.com/pingcap/tidb-test/pull/708, it doesn't matter, just merge it, I'll move the ddl_test to tidb repo.

@WangXiangUSTC
Copy link
Contributor Author

@winkyao ok, thanks.

@tiancaiamao
Copy link
Contributor

Rebase releaes-2.0 master ?
The release-2.0 master should not fail like this @WangXiangUSTC

@WangXiangUSTC
Copy link
Contributor Author

@tiancaiamao this version need guarantee the stability, rebase release-2.0 seems not a good thing

@tiancaiamao
Copy link
Contributor

Oh, I see. @WangXiangUSTC

Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

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

LGTM

@WangXiangUSTC WangXiangUSTC merged commit 41c34fb into pingcap:v2.0.10-binlog Dec 26, 2018
@jackysp
Copy link
Member

jackysp commented Dec 26, 2018

Please add the origin PR link in the PR description.

@WangXiangUSTC
Copy link
Contributor Author

@jackysp link added

@WangXiangUSTC WangXiangUSTC added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Dec 26, 2018
WangXiangUSTC added a commit to WangXiangUSTC/tidb that referenced this pull request Jan 3, 2019
@WangXiangUSTC WangXiangUSTC deleted the wx/pump-client branch January 24, 2019 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/binlog status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants