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

Initial version of craft binary mq message format #1621

Merged
merged 52 commits into from
Jun 22, 2021
Merged

Initial version of craft binary mq message format #1621

merged 52 commits into from
Jun 22, 2021

Conversation

sunxiaoguang
Copy link
Contributor

@sunxiaoguang sunxiaoguang commented Apr 8, 2021 •
edited
Loading

Signed-off-by: Xiaoguang Sun [email protected]

What problem does this PR solve?

#1613

What is changed and how it works?

Implement a binary MQ format that's under discussion in the issue #1613

Check List

Tests

  • Unit test

Code changes

  • Has persistent data change

Side effects

  • Increased code complexity

Related changes

  • Need to update the documentation

Release note

  • sink: Add a new binary MQ format that is specially designed for TiDB and more compact than JSON base Open Protocol. In addition to footprint and performance benefits. There is Flink format plugin support with this new format which makes it possible to use TiDB as a unified batch / streaming source connector in Flink.

@ti-chi-bot ti-chi-bot requested review from 5kbpers and liuzix April 8, 2021 13:56
@ti-chi-bot ti-chi-bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Apr 8, 2021
@sunxiaoguang
Add license
96ee865 
Signed-off-by: Xiaoguang Sun <[email protected]>
@sunxiaoguang
Fix some lint
7347d7c 
Signed-off-by: Xiaoguang Sun <[email protected]>
@sunxiaoguang
Add more test to supress not used function lint
86f4612 
Signed-off-by: Xiaoguang Sun <[email protected]>
@sunxiaoguang
Remove not useful decodeRowChangedEvent function
1cb0a2d 
Signed-off-by: Xiaoguang Sun <[email protected]>
@sunxiaoguang
Use the correct size table to decode column groups
b570de2 
Signed-off-by: Xiaoguang Sun <[email protected]>
@sunxiaoguang
Fix more lints
d1908d5 
Signed-off-by: Xiaoguang Sun <[email protected]>
@sunxiaoguang
Fix lint
d2bdea4 
Signed-off-by: Xiaoguang Sun <[email protected]>
@sunxiaoguang
Fix lint
0e21331 
Signed-off-by: Xiaoguang Sun <[email protected]>
@liuzix liuzix requested review from zier-one, overvenus and IANTHEREAL and removed request for 5kbpers April 8, 2021 16:10
@sunxiaoguang
Copy link
Contributor Author

Looks like the lint error comes from errors defined in generated protobuf code.

image

image

sunxiaoguang and others added 12 commits April 12, 2021 21:37
@sunxiaoguang
Change to little endian like what protobuf does.
97b9e1c 
Signed-off-by: Xiaoguang Sun <[email protected]>
@sunxiaoguang
Remove comments and fix column group values encoding
dc77557 
Signed-off-by: Xiaoguang Sun <[email protected]>
@sunxiaoguang
Set schema and table to nil when they are empty
b3a5ff9 
Signed-off-by: Xiaoguang Sun <[email protected]>
@sunxiaoguang
Merge branch 'master' into craft
e9d080f
@sunxiaoguang
Add more test cases and fix bugs about the fail tests
f1c602f 
Signed-off-by: Xiaoguang Sun <[email protected]>
@sunxiaoguang
Add protobuf vs craft benchmark related code
51d7980 
Signed-off-by: Xiaoguang Sun <[email protected]>
@sunxiaoguang
Fix column group & year type seralization bug
5cce40c 
Signed-off-by: Xiaoguang Sun <[email protected]>
@sunxiaoguang
Force encoding year type to varint
3f21064 
Signed-off-by: Xiaoguang Sun <[email protected]>
@sunxiaoguang
Fix body size table
495ffb2 
Signed-off-by: Xiaoguang Sun <[email protected]>
@sunxiaoguang
Merge branch 'master' into craft
051a9a7
@sunxiaoguang
Merge branch 'master' into craft
aca3558
@sunxiaoguang
Disable lint for not yet used utility functions
aec1ee4 
Signed-off-by: Xiaoguang Sun <[email protected]>
@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jun 21, 2021
@amyangfei
Copy link
Contributor

/merge cancel

@ti-chi-bot ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Jun 21, 2021
@sunxiaoguang
Remove deprecated RowID from header
686b55c 
Signed-off-by: Xiaoguang Sun <[email protected]>
@sunxiaoguang
Optimize codec size
618b773 
1. Remove deprecated RowID field from header
2. Use term dictionary to compress terms

Signed-off-by: Xiaoguang Sun <[email protected]>
@sunxiaoguang
Merge remote-tracking branch 'myrepo/craft' into craft
282af92
@sunxiaoguang
Fix lints
34a222f 
Signed-off-by: Xiaoguang Sun <[email protected]>
@ben1009
Copy link
Contributor

ben1009 commented Jun 21, 2021

/lgtm

@ti-chi-bot
Copy link
Member

@ben1009: Please use GitHub review feature instead of /lgtm [cancel] when you want to submit review to the pull request.
For how to use GitHub review feature, see also this document provided by GitHub.

For the reason we drop support to the commands, see also this page.
This reply is being used as a temporary reply during the migration of review process and will be removed on July 1st.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

@sunxiaoguang
Copy link
Contributor Author

/run-all-tests

1 similar comment
@sunxiaoguang
Copy link
Contributor Author

/run-all-tests

@amyangfei
Copy link
Contributor

/merge

@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: b96c3f5

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jun 22, 2021
@amyangfei
Copy link
Contributor

/run-all-tests

@amyangfei
Copy link
Contributor

/run-leak-tests

@amyangfei
Copy link
Contributor

/run-integration-tests
/run-kafka-tests

@ti-chi-bot ti-chi-bot merged commit ec8bf7c into pingcap:master Jun 22, 2021
@ti-chi-bot ti-chi-bot mentioned this pull request Jun 22, 2021
Closed
@ti-chi-bot
Copy link
Member

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

@Mini256 Mini256 mentioned this pull request Jul 9, 2021
Closed
1 task
@sunxiaoguang sunxiaoguang mentioned this pull request Sep 13, 2021
Open
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ben1009 ben1009 ben1009 approved these changes

@overvenus overvenus overvenus approved these changes

@amyangfei amyangfei amyangfei approved these changes

@liuzix liuzix Awaiting requested review from liuzix

@zier-one zier-one Awaiting requested review from zier-one

@IANTHEREAL IANTHEREAL Awaiting requested review from IANTHEREAL

Assignees
No one assigned
Labels
needs-cherry-pick-release-5.1 Should cherry pick this PR to release-5.1 branch. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Milestone
v5.1.1
Development

Successfully merging this pull request may close these issues.
6 participants
@sunxiaoguang @codecov-commenter @ti-chi-bot @amyangfei @ben1009 @overvenus