ISSUE-3605: Replace arrow flight with gRPC #3606

ariesdevil · 2021-12-22T14:29:32Z

I hereby agree to the terms of the CLA available at: https://databend.rs/dev/policies/cla/

Summary

Replace arrow flight with gRPC

Changelog

Improvement

Related Issues

Fixes #3605

Test Plan

Unit Tests

Stateless Tests

databend-bot · 2021-12-22T14:29:35Z

Thanks for the contribution!
I have applied any labels matching special text in your PR Changelog.

Please review the labels and make any necessary changes.

vercel · 2021-12-22T14:29:36Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/databend/databend/EkBEd7STmjW79WZYdpyTic3Ld177
✅ Preview: Canceled

[Deployment for d4e6cec canceled]

common/flight-rpc/src/dns_resolver.rs

drmingdrmer

not bad~

common/flight-rpc/src/dns_resolver.rs

common/meta/raft-store/src/grpc/grpc_action.rs

mergify · 2021-12-24T09:01:04Z

This pull request has merge conflicts that must be resolved before it can be merged. @ariesdevil please rebase it 🙏

codecov-commenter · 2021-12-29T19:45:51Z

Codecov Report

Merging #3606 (d4e6cec) into main (a2529db) will increase coverage by 0%.
The diff coverage is 66%.

@@          Coverage Diff           @@
##            main   #3606    +/-   ##
======================================
  Coverage     60%     60%            
======================================
  Files        706     707     +1     
  Lines      37901   38003   +102     
======================================
+ Hits       22806   22891    +85     
- Misses     15095   15112    +17

Impacted Files	Coverage Δ
common/meta/raft-store/src/grpc/grpc_client.rs	`0% <0%> (ø)`
...ommon/meta/raft-store/src/grpc/grpc_client_conf.rs	`0% <0%> (ø)`
common/meta/raft-store/src/state_machine/sm.rs	`97% <ø> (ø)`
common/meta/types/src/applied_state.rs	`50% <ø> (ø)`
common/meta/types/src/message.rs	`95% <ø> (ø)`
metasrv/src/executor/meta_handlers.rs	`81% <ø> (ø)`
metasrv/src/meta_service/meta_leader.rs	`82% <ø> (ø)`
metasrv/src/meta_service/meta_node_kv_api_impl.rs	`86% <ø> (ø)`
metasrv/src/meta_service/meta_service_impl.rs	`92% <ø> (ø)`
metasrv/src/meta_service/raftmeta.rs	`47% <ø> (+<1%)`	⬆️
... and 32 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a2529db...d4e6cec. Read the comment docs.

drmingdrmer

Looks gorgeous to me

metasrv/tests/it/flight/metasrv_grpc_kv_api_restart_cluster.rs

common/meta/types/src/message.rs

drmingdrmer · 2021-12-30T07:33:10Z

common/meta/types/proto/meta.proto

+
+  // message
+  rpc WriteMsg(RaftRequest) returns (RaftReply);
+  rpc ReadMsg(GetReq) returns (GetReply);


Rename GetReq to GetRequest in next PR? 🤔

drmingdrmer · 2021-12-30T07:53:25Z

metasrv/src/api/grpc/grpc_service.rs

+        let body = self.action_handler.execute_read(action).await?;
+        let r = GetReply {
+            ok: true,
+            key: "".to_string(),
+            value: body,
+        };
+        Ok(Response::new(r))


If key is not used at all, we could refine GetReply in next PR. :DDD

common/meta/raft-store/tests/it/grpc/grpc_server.rs

common/meta/raft-store/tests/it/grpc/grpc_client.rs

drmingdrmer

Nice fix up

databend-bot · 2021-12-30T11:04:03Z

CI Passed
Reviewers Approved
Let's Merge
Thank you for the PR @ariesdevil

databend-bot added the pr-improvement label Dec 22, 2021

databend-bot added the need-review label Dec 22, 2021

vercel bot temporarily deployed to Preview December 22, 2021 14:29 Inactive

Xuanwo reviewed Dec 23, 2021

View reviewed changes

common/flight-rpc/src/dns_resolver.rs Outdated Show resolved Hide resolved

ariesdevil force-pushed the r-replace-arrow-flight branch from f3dd6f6 to 69365ca Compare December 23, 2021 10:03

vercel bot temporarily deployed to Preview December 23, 2021 10:03 Inactive

vercel bot temporarily deployed to Preview December 23, 2021 10:11 Inactive

drmingdrmer reviewed Dec 24, 2021

View reviewed changes

common/flight-rpc/src/dns_resolver.rs Outdated Show resolved Hide resolved

common/meta/raft-store/src/grpc/grpc_action.rs Outdated Show resolved Hide resolved

common/meta/raft-store/src/grpc/grpc_action.rs Outdated Show resolved Hide resolved

Xuanwo mentioned this pull request Dec 24, 2021

ci: Only ping author after PR has been approved #3637

Merged

vercel bot temporarily deployed to Preview December 26, 2021 10:07 Inactive

vercel bot temporarily deployed to Preview December 27, 2021 14:18 Inactive

vercel bot temporarily deployed to Preview December 28, 2021 08:36 Inactive

vercel bot temporarily deployed to Preview December 29, 2021 15:15 Inactive

vercel bot temporarily deployed to Preview December 29, 2021 18:15 Inactive

ariesdevil added 12 commits December 30, 2021 02:15

init commit

a11626f

build success

bbef1cd

cleanup meta proto file

cc19e60

make lint happy

1afe2ab

add license header

c8ab046

try fix one test

e12081c

add another grpc service to serve query actions

3b9e617

grpc server start and stop

3b5f81d

make lint happy again

e0fbe17

fix build

25e0d79

try fix tests

cab9d20

try fix tests

85776b9

ariesdevil force-pushed the r-replace-arrow-flight branch from 65a82e6 to 85776b9 Compare December 29, 2021 18:21

vercel bot temporarily deployed to Preview December 29, 2021 18:22 Inactive

rename suggested changes

af1a5d7

vercel bot temporarily deployed to Preview December 29, 2021 18:29 Inactive

refactor some codes

855e075

vercel bot temporarily deployed to Preview December 29, 2021 18:58 Inactive

add grpc client tests

d8fa67f

vercel bot temporarily deployed to Preview December 29, 2021 20:01 Inactive

ariesdevil marked this pull request as ready for review December 30, 2021 05:45

ariesdevil requested a review from BohuTANG as a code owner December 30, 2021 05:45

ariesdevil requested a review from drmingdrmer December 30, 2021 05:45

fix stateless cluster tests

d1e9866

vercel bot temporarily deployed to Preview December 30, 2021 06:31 Inactive

rename flight-rpc env name

68466ae

vercel bot temporarily deployed to Preview December 30, 2021 06:48 Inactive

drmingdrmer approved these changes Dec 30, 2021

View reviewed changes

refine codes

da78e65

vercel bot temporarily deployed to Preview December 30, 2021 09:14 Inactive

drmingdrmer approved these changes Dec 30, 2021

View reviewed changes

fix timeout tests q.q

d4e6cec

vercel bot temporarily deployed to Preview December 30, 2021 10:12 Inactive

databend-bot removed the need-review label Dec 30, 2021

databend-bot merged commit f6f21c0 into datafuselabs:main Dec 30, 2021

ariesdevil deleted the r-replace-arrow-flight branch December 30, 2021 11:19

ariesdevil mentioned this pull request Dec 31, 2021

[metasrv] refine grpc #3717

Merged

ariesdevil mentioned this pull request Jan 26, 2022

Consider replacing metasrv flight API with bare grpc? #1927

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ISSUE-3605: Replace arrow flight with gRPC #3606

ISSUE-3605: Replace arrow flight with gRPC #3606

ariesdevil commented Dec 22, 2021 •

edited

databend-bot commented Dec 22, 2021

vercel bot commented Dec 22, 2021 •

edited

drmingdrmer left a comment

mergify bot commented Dec 24, 2021

codecov-commenter commented Dec 29, 2021 •

edited

drmingdrmer left a comment

drmingdrmer Dec 30, 2021

drmingdrmer Dec 30, 2021

drmingdrmer left a comment

databend-bot commented Dec 30, 2021

ISSUE-3605: Replace arrow flight with gRPC #3606

ISSUE-3605: Replace arrow flight with gRPC #3606

Conversation

ariesdevil commented Dec 22, 2021 • edited

Summary

Changelog

Related Issues

Test Plan

databend-bot commented Dec 22, 2021

vercel bot commented Dec 22, 2021 • edited

drmingdrmer left a comment

Choose a reason for hiding this comment

mergify bot commented Dec 24, 2021

codecov-commenter commented Dec 29, 2021 • edited

Codecov Report

drmingdrmer left a comment

Choose a reason for hiding this comment

drmingdrmer Dec 30, 2021

Choose a reason for hiding this comment

drmingdrmer Dec 30, 2021

Choose a reason for hiding this comment

drmingdrmer left a comment

Choose a reason for hiding this comment

databend-bot commented Dec 30, 2021

ariesdevil commented Dec 22, 2021 •

edited

vercel bot commented Dec 22, 2021 •

edited

codecov-commenter commented Dec 29, 2021 •

edited