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

config: remove the special limit of txn-total-size-limit when binlog enabled #16919

Merged
merged 3 commits into from
Apr 29, 2020

Conversation

jackysp
Copy link
Member

@jackysp jackysp commented Apr 28, 2020

Signed-off-by: Shuaipeng Yu jackysp@gmail.com

What problem does this PR solve?

Issue Number: close #xxx

Problem Summary:
We've tested the big transaction when binlog enabled, pump, and drainer can handle 10GB transactions. But with Kafka as an arbiter or some other usage of binlog, it could only handle 1GB transactions, because Kafka has its own max message limit.

What is changed and how it works?

Proposal: xxx

What's Changed:
We could not know if there is a Kafka cluster using in the binlog replication. So we remove the limitation and keep the warnings of txn-total-size-limit usage.

How it Works:

Related changes

  • PR to update pingcap/docs/pingcap/docs-cn:
  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test
  • Manual test (add detailed scripts or steps below)
    There is an internal doc for it.

Release note

Remove the special limit of txn-total-size-limit when binlog enabled.

…enabled

Signed-off-by: Shuaipeng Yu <jackysp@gmail.com>
@jackysp jackysp added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Apr 28, 2020
Copy link
Contributor

@lysu lysu left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@WangXiangUSTC WangXiangUSTC left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@lysu lysu left a comment

Choose a reason for hiding this comment

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

LGTM

@lysu
Copy link
Contributor

lysu commented Apr 29, 2020

/merge

@sre-bot sre-bot added the status/can-merge Indicates a PR has been approved by a committer. label Apr 29, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Apr 29, 2020

/run-all-tests

@lysu lysu added this to the 4.0.0-rc.2 milestone Apr 29, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Apr 29, 2020

@jackysp merge failed.

@codecov
Copy link

codecov bot commented Apr 29, 2020

Codecov Report

Merging #16919 into master will increase coverage by 0.0512%.
The diff coverage is n/a.

@@               Coverage Diff                @@
##             master     #16919        +/-   ##
================================================
+ Coverage   80.0910%   80.1423%   +0.0512%     
================================================
  Files           508        508                
  Lines        138149     138269       +120     
================================================
+ Hits         110645     110812       +167     
+ Misses        18565      18525        -40     
+ Partials       8939       8932         -7     

@lysu
Copy link
Contributor

lysu commented Apr 29, 2020

/run-integration-copr-test

@lysu
Copy link
Contributor

lysu commented Apr 29, 2020

/merge

@sre-bot
Copy link
Contributor

sre-bot commented Apr 29, 2020

cherry pick to release-4.0 in PR #16941

@jackysp jackysp deleted the big_binlog branch November 25, 2020 02:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/config release-note Denotes a PR that will be considered when it comes time to generate release notes. status/can-merge Indicates a PR has been approved by a committer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants