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.

Sign up for GitHub

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

Jump to bottom

drainer: fix kafka message limit problem #1039

Merged
merged 14 commits into from
Oct 11, 2021
Merged

drainer: fix kafka message limit problem #1039

merged 14 commits into from
Oct 11, 2021

Conversation

lichunzhu
Copy link
Contributor

@lichunzhu lichunzhu commented Apr 16, 2021 •
edited
Loading

What problem does this PR solve?

close #1156
When we use kafka as downstream, the transaction size exceed 1G can't to transport to downstream.

What is changed and how it works?

  1. Set grpc message limit to IntMax instead of max.Int32.
  2. Add configuration max-message-size in drainer's config.

Check List

Tests

  • Unit test
  • Integration test

Code changes

  • Has exported variable/fields change

Related changes

  • Need to cherry-pick to the release branch
  • Need to be included in the release note

Release note

  • Set grpc message limit to IntMax instead of max.Int32. Add configuration max-message-size in drainer's config.

@sre-bot
Copy link

sre-bot commented Apr 16, 2021

No release note, Please follow https://github.com/pingcap/community/blob/master/contributors/release-note-checker.md

@lichunzhu
Copy link
Contributor Author

/rebuild

@lichunzhu
Copy link
Contributor Author

/run-all-tests

1 similar comment
@lichunzhu
Copy link
Contributor Author

/run-all-tests

@lichunzhu
Copy link
Contributor Author

/run-all-tests

@lichunzhu
Copy link
Contributor Author

/run-all-tests

glorv
glorv reviewed Sep 11, 2021
View reviewed changes
drainer/config.go Outdated
maxMsgSize = maxKafkaMsgSize

if cfg.SyncerCfg.To.KafkaMaxMessageSize > 0 {
maxMsgSize = cfg.SyncerCfg.To.KafkaMaxMessageSize
Copy link
Contributor

Choose a reason for hiding this comment

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

What if KafkaMaxMessageSize > MaxInt32 。 I think we need to do this check here

@sre-bot
Copy link

sre-bot commented Sep 14, 2021

No release note, Please follow https://github.com/pingcap/community/blob/master/contributors/release-note-checker.md

@lichunzhu
Copy link
Contributor Author

/rebuild

@lichunzhu
Copy link
Contributor Author

/run-all-tests

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Sep 15, 2021 •
edited
Loading

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • amyangfei
  • glorv

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@lichunzhu
Copy link
Contributor Author

/rebuild

@glorv
Copy link
Contributor

glorv commented Oct 11, 2021

/run-check

@glorv
Copy link
Contributor

glorv commented Oct 11, 2021

/run-all-tests

@ti-chi-bot ti-chi-bot merged commit 11f74f6 into pingcap:master Oct 11, 2021
@ti-chi-bot ti-chi-bot mentioned this pull request Oct 11, 2021
Merged
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created: #1077.

@ti-chi-bot ti-chi-bot mentioned this pull request Oct 11, 2021
Merged
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created: #1078.

@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created: #1079.

ti-chi-bot pushed a commit to ti-chi-bot/tidb-binlog that referenced this pull request Oct 12, 2021
@lichunzhu @ti-chi-bot
drainer: fix kafka message limit problem (pingcap#1039)
7ff19f1
ti-chi-bot pushed a commit to ti-chi-bot/tidb-binlog that referenced this pull request Oct 12, 2021
@lichunzhu @ti-chi-bot
drainer: fix kafka message limit problem (pingcap#1039)
f5e54c9
This was referenced Oct 12, 2021
Merged
Merged
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created: #1080.

ti-chi-bot added a commit that referenced this pull request Oct 12, 2021
@ti-chi-bot
drainer: fix kafka message limit problem (#1039) (#1078)
daf5de8
ti-chi-bot added a commit that referenced this pull request Oct 12, 2021
@ti-chi-bot
drainer: fix kafka message limit problem (#1039) (#1080)
75683e3
ti-chi-bot added a commit that referenced this pull request Oct 13, 2021
@ti-chi-bot
drainer: fix kafka message limit problem (#1039) (#1079)
311edce
ti-chi-bot added a commit that referenced this pull request Oct 13, 2021
@ti-chi-bot
drainer: fix kafka message limit problem (#1039) (#1077)
bf3617e
@lichunzhu lichunzhu deleted the fixKafkaMessageLimit branch November 3, 2021 03:18
@lichunzhu lichunzhu mentioned this pull request Nov 9, 2021
Merged
14 tasks
This was referenced Dec 6, 2021
Merged
Merged
Merged
Merged
@lichunzhu lichunzhu mentioned this pull request Dec 7, 2021
Closed
@niubell
Copy link

niubell commented Apr 14, 2022

close pingcap/tidb#28659

@lichunzhu lichunzhu mentioned this pull request Aug 16, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@glorv glorv glorv approved these changes

@amyangfei amyangfei amyangfei approved these changes

Assignees
No one assigned
Labels
needs-cherry-pick-release-4.0 needs-cherry-pick-release-5.0 needs-cherry-pick-release-5.1 needs-cherry-pick-release-5.2 Type: Need cherry pick to release-5.2 size/M status/can-merge status/LGT2 status/PTAL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

pump/drainer: binlog can't replicate data any more if filtered binlog exceeds 2G
6 participants
@lichunzhu @sre-bot @ti-chi-bot @glorv @niubell @amyangfei