Bundle grpc/protobuf in tiflash #3896

zanmato1984 · 2022-01-19T11:54:48Z

What problem does this PR solve?

Problem Summary: Currently developing tiflash requires pre-installing grpc/protobuf (and in a weird way), this is a lot pain for most developers. We'd better bundle grpc/protobuf within tiflash, like other third party libraries.

What is changed and how it works?

Add grpc/protobuf and dependent submodules, and enhance cmake finding. Most cmake changes are copied from upstream ClickHouse with some minor tweaks.

Another problem is how to co-work with kvproto/tipb/client-c(which depends on kvproto as well) who depend on grpc/protobuf as well. Thanks to @JaySon-Huang 's great work in pingcap/kvproto#775, pingcap/tipb#230, this has been made relatively easy. I made separated changes pingcap/kvproto#853, pingcap/tipb#257, tikv/client-c#84 in these submodules.

Check List

Side effects

Basically no side effects if you build tiflash using bundled grpc/protobuf (the default option), except for the first time you'll have to pull more submodules and build them, but that's mostly a one-time deal.

If you go with the old-fashioned way, i.e., build tiflash using your own grpc/protobuf, you can use cmake option:

cmake -DUSE_INTERNAL_PROTOBUF_LIBRARY=FALSE -DUSE_INTERNAL_GRPC_LIBRARY=FALSE ...

If your gprc/protobuf is not located in system default path, you can use extra option to specify it:

cmake -DUSE_INTERNAL_PROTOBUF_LIBRARY=FALSE -DUSE_INTERNAL_GRPC_LIBRARY=FALSE -DPREBUILT_LIBS_ROOT="<PATH1>;<PATH2>..." ...

This has been tested on:

My local Mac, with pre-installed dynamic grpc/protobuf lib (brew installed) - working;
My local Mac, with pre-installed static grpc/protobuf lib (built from source, twice) - working;
Linux, with pre-installed dynamic grpc/protobuf lib (apt/yum) - untested;
Linux, with pre-installed static grpc/protobuf lib (built from source, twice) - failed by linking error, not sure why and I'm not planning dig into it for now;

Documentation

Release note

None

ti-chi-bot · 2022-01-19T11:54:49Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

JaySon-Huang

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.

zanmato1984 · 2022-01-19T12:42:48Z

/run-all-tests

zanmato1984 · 2022-01-19T13:31:21Z

/run-all-tests

solotzg · 2022-01-19T13:35:06Z

How about openssl？

sre-bot · 2022-01-19T14:26:49Z

Coverage detail: https://ci-internal.pingcap.net/job/tics_ghpr_unit_test/1069/cobertura/
(Coverage detail url is limited office network access)

lines: 44.7% (51498 out of 115169)
branches: 7.0% (89810 out of 1275204)

.gitmodules

cmake/find_grpc.cmake

dbms/src/Flash/Coprocessor/DAGQueryBlockInterpreter.h

zanmato1984 · 2022-01-19T16:03:37Z

How about openssl？

This PR doesn't change the way how openssl is used. We may come back to it in another thread.

Co-authored-by: JaySon <tshent@qq.com>

zanmato1984 · 2022-01-21T09:56:03Z

/merge

ti-chi-bot · 2022-01-21T09:56:04Z

@zanmato1984: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

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.

ti-chi-bot · 2022-01-21T09:56:10Z

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

Commit hash: 571a628

JaySon-Huang · 2022-01-21T10:09:43Z

/cc @SchrodingerZhu , This PR would affect the build docker image of the toolchain upgrade.

…o bundle-grpc

zanmato1984 · 2022-01-21T10:15:43Z

/merge

ti-chi-bot · 2022-01-21T10:15:44Z

@zanmato1984: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

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.

ti-chi-bot · 2022-01-21T10:15:49Z

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

Commit hash: a5853d4

sre-bot · 2022-01-21T11:07:06Z

Coverage detail: https://ci-internal.pingcap.net/job/tics_ghpr_unit_test/1109/cobertura/
(Coverage detail url is limited office network access)

lines: 44.7% (51532 out of 115235)
branches: 7.0% (89834 out of 1274914)

Fully rename project to tiflash (#3901) Conflicts: libs/libdaemon/cmake/find_unwind.cmake Remove useless curl dependency (#3974) Bundle all dependencies: zlib, openssl and curl (#3987) Conflicts: contrib/poco update contrib/poco update ci/cd process Signed-off-by: Zhigao Tong <tongzhigao@pingcap.com> fix zlib-ng Signed-off-by: Zhigao Tong <tongzhigao@pingcap.com>

Fully rename project to tiflash (pingcap#3901) Conflicts: libs/libdaemon/cmake/find_unwind.cmake Remove useless curl dependency (pingcap#3974) Bundle all dependencies: zlib, openssl and curl (pingcap#3987) Conflicts: contrib/poco update contrib/poco update ci/cd process Signed-off-by: Zhigao Tong <tongzhigao@pingcap.com> fix zlib-ng Signed-off-by: Zhigao Tong <tongzhigao@pingcap.com> extra fix formatted

Fully rename project to tiflash (pingcap#3901) Conflicts: libs/libdaemon/cmake/find_unwind.cmake Remove useless curl dependency (pingcap#3974) Bundle all dependencies: zlib, openssl and curl (pingcap#3987) Conflicts: contrib/poco update contrib/poco update ci/cd process Signed-off-by: Zhigao Tong <tongzhigao@pingcap.com> fix zlib-ng Signed-off-by: Zhigao Tong <tongzhigao@pingcap.com> extra fix formatted fix: remove dttool, unify definition fix: add functions fix: replace ClickHouse to TiFlash

Fully rename project to tiflash (pingcap#3901) Conflicts: libs/libdaemon/cmake/find_unwind.cmake Remove useless curl dependency (pingcap#3974) Bundle all dependencies: zlib, openssl and curl (pingcap#3987) Conflicts: contrib/poco update contrib/poco update ci/cd process Signed-off-by: Zhigao Tong <tongzhigao@pingcap.com> fix zlib-ng Signed-off-by: Zhigao Tong <tongzhigao@pingcap.com> extra fix formatted fix: remove dttool, unify definition fix: add functions fix: replace ClickHouse to TiFlash fix: update dtpb cmake fix: compile issue fmt fix: gtests fix: gtests scripts

) (#3987) (#4591) close #4196

) (#3987) (#4599) close #4196

) (#3987) (#4689) close #4196

) (#3987) (#4369) close #4196

) (#3987) (#4600) close #4196

zanmato1984 added 7 commits January 19, 2022 11:46

Update pingcap submodules

9d354a8

Add changes that definitely will be useful

ec2a242

Add other things

017511f

Move abseil-cpp

2e4dcae

Refine modules

20a1b87

Refine modules

403f280

Add abseil-cpp

5a6d71f

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 19, 2022

zanmato1984 added 4 commits January 19, 2022 19:55

Merge branch 'master' into bundle-grpc

fb66b38

format

1415707

Format

89ae01d

Remove comments

cef3a09

zanmato1984 added 2 commits January 19, 2022 20:44

Add finding local grpc

316d9d7

Remove copy libnsl

c161dba

JaySon-Huang reviewed Jan 19, 2022

View reviewed changes

.gitmodules Outdated Show resolved Hide resolved

JaySon-Huang reviewed Jan 19, 2022

View reviewed changes

cmake/find_grpc.cmake Show resolved Hide resolved

JaySon-Huang reviewed Jan 19, 2022

View reviewed changes

dbms/src/Flash/Coprocessor/DAGQueryBlockInterpreter.h Show resolved Hide resolved

zanmato1984 and others added 5 commits January 20, 2022 00:04

Update .gitmodules

69cde05

Co-authored-by: JaySon <tshent@qq.com>

Fix cctz missing core foundation on mac

37cf0c0

Merge branch 'bundle-grpc' into bundle-grpc-next

665166f

Merge branch 'bundle-grpc-next' into bundle-grpc

614067a

Add lib grp in finding local grpc

82d7743

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jan 21, 2022

Merge branch 'bundle-grpc' of https://github.com/zanmato1984/tics int…

a5853d4

…o bundle-grpc

ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Jan 21, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jan 21, 2022

ti-chi-bot merged commit 522d4ac into pingcap:master Jan 21, 2022

zanmato1984 deleted the bundle-grpc branch January 21, 2022 11:18

solotzg mentioned this pull request Mar 10, 2022

[DEPRECATED] Bundle grpc/protobuf curl zlib ssl in release-5.4 (#3896) (#3901) (#3974) (#3987) #4211

Closed

12 tasks

LittleFall mentioned this pull request Mar 21, 2022

Bundle grpc/protobuf curl zlib ssl in release-5.3 (#3896) (#3901) (#3974) (#3987) #4369

Merged

12 tasks

LittleFall mentioned this pull request Apr 6, 2022

Bundle grpc/protobuf curl zlib ssl in release-5.2 (#3896) (#3901) (#3974) (#3987) #4591

Merged

12 tasks

LittleFall mentioned this pull request Apr 6, 2022

Bundle grpc/protobuf curl zlib ssl in release-5.1 (#3896) (#3901) (#3974) (#3987) #4599

Merged

12 tasks

LittleFall mentioned this pull request Apr 6, 2022

Bundle grpc/protobuf curl zlib ssl in release-5.0 (#3896) (#3901) (#3974) (#3987) #4600

Merged

12 tasks

ti-chi-bot pushed a commit that referenced this pull request Apr 11, 2022

Bundle grpc/protobuf curl zlib ssl in release-5.2 (#3896) (#3901) (#3974

15b2664

) (#3987) (#4591) close #4196

solotzg mentioned this pull request Apr 18, 2022

Bundle grpc/protobuf curl zlib ssl in release-5.4 (#3896) (#3901) (#3974) (#3987) #4689

Merged

12 tasks

ti-chi-bot pushed a commit that referenced this pull request Apr 22, 2022

Bundle grpc/protobuf curl zlib ssl in release-5.1 (#3896) (#3901) (#3974

d655595

) (#3987) (#4599) close #4196

ti-chi-bot pushed a commit that referenced this pull request Apr 22, 2022

Bundle grpc/protobuf curl zlib ssl in release-5.4 (#3896) (#3901) (#3974

5d8e89f

) (#3987) (#4689) close #4196

ti-chi-bot pushed a commit that referenced this pull request Apr 22, 2022

Bundle grpc/protobuf curl zlib ssl in release-5.3 (#3896) (#3901) (#3974

ba1f109

) (#3987) (#4369) close #4196

ti-chi-bot pushed a commit that referenced this pull request May 11, 2022

Bundle grpc/protobuf curl zlib ssl in release-5.0 (#3896) (#3901) (#3974

94609a6

) (#3987) (#4600) close #4196

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bundle grpc/protobuf in tiflash #3896

Bundle grpc/protobuf in tiflash #3896

zanmato1984 commented Jan 19, 2022 •

edited

Loading

ti-chi-bot commented Jan 19, 2022 •

edited

Loading

zanmato1984 commented Jan 19, 2022

zanmato1984 commented Jan 19, 2022

solotzg commented Jan 19, 2022

sre-bot commented Jan 19, 2022

zanmato1984 commented Jan 19, 2022

zanmato1984 commented Jan 21, 2022

ti-chi-bot commented Jan 21, 2022

ti-chi-bot commented Jan 21, 2022

JaySon-Huang commented Jan 21, 2022

zanmato1984 commented Jan 21, 2022

ti-chi-bot commented Jan 21, 2022

ti-chi-bot commented Jan 21, 2022

sre-bot commented Jan 21, 2022

Bundle grpc/protobuf in tiflash #3896

Bundle grpc/protobuf in tiflash #3896

Conversation

zanmato1984 commented Jan 19, 2022 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Jan 19, 2022 • edited Loading

zanmato1984 commented Jan 19, 2022

zanmato1984 commented Jan 19, 2022

solotzg commented Jan 19, 2022

sre-bot commented Jan 19, 2022

zanmato1984 commented Jan 19, 2022

zanmato1984 commented Jan 21, 2022

ti-chi-bot commented Jan 21, 2022

ti-chi-bot commented Jan 21, 2022

JaySon-Huang commented Jan 21, 2022

zanmato1984 commented Jan 21, 2022

ti-chi-bot commented Jan 21, 2022

ti-chi-bot commented Jan 21, 2022

sre-bot commented Jan 21, 2022

zanmato1984 commented Jan 19, 2022 •

edited

Loading

ti-chi-bot commented Jan 19, 2022 •

edited

Loading