Add gRPC support to FDB #11782

vishesh · 2024-11-14T17:19:24Z

This PR gRPC server along with Flow with goal of supporting new/specific features but not entirely replacing FlowRPC itself.

Code-Reviewer Section

The general pull request guidelines can be found here.

Please check each of the following things and check all boxes before accepting a PR.

The PR has a description, explaining both the problem and the solution.
The description mentions which forms of testing were done and the testing seems reasonable.
Every function/class/actor that was touched is reasonably well documented.

For Release-Branches

If this PR is made against a release-branch, please also check the following:

This change/bugfix is a cherry-pick from the next younger branch (younger release-branch or main if this is the youngest branch)
There is a good reason why this PR needs to go into a release branch and this reason is documented (either in the description above or in a linked GitHub issue)

This patch adds `collect_unit_tests()` to CMake which searches over the codebase and finds all the unit-tests written using Flow's TEST_CASE macro and register as ctest. The test then can be then run using ctest command or directly via Test Explorer in VSCode.

foundationdb-ci · 2024-11-14T17:24:06Z

Result of foundationdb-pr on Linux CentOS 7

Commit ID: 8036f3b
Duration 0:04:28
Result: ❌ FAILED
Error: Error while executing command: if [[ $(git diff --shortstat 2> /dev/null | tail -n1) == "" ]]; then echo "CODE FORMAT CLEAN"; else echo "CODE FORMAT NOT CLEAN"; echo; echo "THE FOLLOWING FILES NEED TO BE FORMATTED"; echo; git ls-files -m; echo; exit 1; fi. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2024-11-14T17:24:06Z

Result of foundationdb-pr-clang-arm on Linux CentOS 7

Commit ID: 8036f3b
Duration 0:04:29
Result: ❌ FAILED
Error: Error while executing command: if [[ $(git diff --shortstat 2> /dev/null | tail -n1) == "" ]]; then echo "CODE FORMAT CLEAN"; else echo "CODE FORMAT NOT CLEAN"; echo; echo "THE FOLLOWING FILES NEED TO BE FORMATTED"; echo; git ls-files -m; echo; exit 1; fi. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2024-11-14T17:24:10Z

Result of foundationdb-pr-clang on Linux CentOS 7

Commit ID: 8036f3b
Duration 0:04:33
Result: ❌ FAILED
Error: Error while executing command: if [[ $(git diff --shortstat 2> /dev/null | tail -n1) == "" ]]; then echo "CODE FORMAT CLEAN"; else echo "CODE FORMAT NOT CLEAN"; echo; echo "THE FOLLOWING FILES NEED TO BE FORMATTED"; echo; git ls-files -m; echo; exit 1; fi. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2024-11-14T17:24:11Z

Result of foundationdb-pr-clang-ide on Linux CentOS 7

Commit ID: 8036f3b
Duration 0:04:33
Result: ❌ FAILED
Error: Error while executing command: if [[ $(git diff --shortstat 2> /dev/null | tail -n1) == "" ]]; then echo "CODE FORMAT CLEAN"; else echo "CODE FORMAT NOT CLEAN"; echo; echo "THE FOLLOWING FILES NEED TO BE FORMATTED"; echo; git ls-files -m; echo; exit 1; fi. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2024-11-14T17:24:36Z

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Commit ID: 8036f3b
Duration 0:04:59
Result: ❌ FAILED
Error: Error while executing command: if [[ $(git diff --shortstat 2> /dev/null | tail -n1) == "" ]]; then echo "CODE FORMAT CLEAN"; else echo "CODE FORMAT NOT CLEAN"; echo; echo "THE FOLLOWING FILES NEED TO BE FORMATTED"; echo; git ls-files -m; echo; exit 1; fi. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)
Cluster Test Logs zip file of the test logs (available for 30 days)

foundationdb-ci · 2024-11-14T17:26:50Z

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

Commit ID: 8036f3b
Duration 0:07:16
Result: ❌ FAILED
Error: Error while executing command: if [[ $(git diff --shortstat 2> /dev/null | tail -n1) == "" ]]; then echo "CODE FORMAT CLEAN"; else echo "CODE FORMAT NOT CLEAN"; echo; echo "THE FOLLOWING FILES NEED TO BE FORMATTED"; echo; git ls-files -m; echo; exit 1; fi. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2024-11-14T17:26:59Z

Result of foundationdb-pr-macos on macOS Ventura 13.x

Commit ID: 8036f3b
Duration 0:07:21
Result: ❌ FAILED
Error: Error while executing command: if [[ $(git diff --shortstat 2> /dev/null | tail -n1) == "" ]]; then echo "CODE FORMAT CLEAN"; else echo "CODE FORMAT NOT CLEAN"; echo; echo "THE FOLLOWING FILES NEED TO BE FORMATTED"; echo; git ls-files -m; echo; exit 1; fi. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

saintstack · 2024-11-14T18:58:14Z

fdbrpc/FlowGrpc.actor.cpp

+
+void GrpcServer::registerService(std::shared_ptr<grpc::Service> service) {
+	registered_services_.push_back(service);
+}


No ACTOR in here?

Not sure why it would be actor?

saintstack · 2024-11-14T18:59:00Z

fdbrpc/FlowGrpcTests.actor.cpp

+namespace asio = boost::asio;
+
+TEST_CASE("/fdbrpc/grpc/basic_sync_client") {
+	state NetworkAddress addr(NetworkAddress::parse("127.0.0.1:50001"));


Test will fail if this port is already occupied?

Yes. I have make some sort of getFreePort() function. Currently, I'm just making sure each test has different port.

saintstack · 2024-11-14T19:00:55Z

fdbrpc/FlowGrpcTests.actor.cpp

+	server.registerService(make_shared<TestEchoServiceImpl>());
+	state Future<Void> _ = server.run();
+	return Void();
+}


What is the expectation here? Can we assert for it?

If lose last server reference, it cleans up, shutsdown and doesn't cause any random segfaults etc.

saintstack · 2024-11-14T19:02:42Z

fdbrpc/FlowGrpcTests.cpp

+    return Void();
+}
+
+} // namespace fdbrpc_test


Is there any flow in here?

Nothing that uses flow compiler. Just Flow/c++ coroutine integration stuff.

saintstack · 2024-11-14T19:04:31Z

fdbrpc/file_transfer.proto

+message DownloadChunk {
+    int32 offset = 2;
+    bytes data = 3;
+}


Is there a proto for file transfer already? Perhaps up in the google cloud repos? There are probably some proto smarts we could 'borrow' -- stuff folks have worked out already around file transfer problem.

vishesh and others added 30 commits August 24, 2024 10:48

Implement gRPC support

e7629c7

Move some CMake stuff around.

8cacc14

Fix typo

d322bb2

Add some test

efc66d1

Add async client

e09d497

Add test for checking destroy

0ad35ff

Merge branch 'discover_unit_tests' into grpc

f7fcbdd

Fix some tests

f5bfe17

Use NetworkAddress

780f53d

Add another variant of call method

f4a6da3

Add a failed call test

171d269

Refactor

010a473

Cleanup shutdown

86f9573

Start working on streaming

4272db5

Implement server streaming

c0a9baf

Cleanup some unnecessary templating

f6052bb

Cleanup some tests

3d81782

WIP Client Streaming

fc27c9b

WIP

02429ed

File Transfer WIP

b69646d

Remove UnitTest.h

997d15b

Merge remote-tracking branch 'apple/main' into grpc

bf5ca81

Take grpc addresses from command line

1b822c6

startup grpc in fdbserver

17eb913

Cancel if future ref is 0

b24bedb

noop

ee8ccc7

Merge remote-tracking branch 'apple/main' into grpc

dc3c12a

Update some Cmake files

8814ab4

Fix some build/run issues

8036f3b

saintstack reviewed Nov 14, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add gRPC support to FDB #11782

Add gRPC support to FDB #11782

vishesh commented Nov 14, 2024

foundationdb-ci commented Nov 14, 2024

foundationdb-ci commented Nov 14, 2024

foundationdb-ci commented Nov 14, 2024

foundationdb-ci commented Nov 14, 2024

foundationdb-ci commented Nov 14, 2024

foundationdb-ci commented Nov 14, 2024

foundationdb-ci commented Nov 14, 2024

saintstack Nov 14, 2024

vishesh Nov 14, 2024

saintstack Nov 14, 2024

vishesh Nov 14, 2024

saintstack Nov 14, 2024

vishesh Nov 14, 2024

saintstack Nov 14, 2024

vishesh Nov 14, 2024

saintstack Nov 14, 2024

Add gRPC support to FDB #11782

Are you sure you want to change the base?

Add gRPC support to FDB #11782

Conversation

vishesh commented Nov 14, 2024

Code-Reviewer Section

For Release-Branches

foundationdb-ci commented Nov 14, 2024

Result of foundationdb-pr on Linux CentOS 7

foundationdb-ci commented Nov 14, 2024

Result of foundationdb-pr-clang-arm on Linux CentOS 7

foundationdb-ci commented Nov 14, 2024

Result of foundationdb-pr-clang on Linux CentOS 7

foundationdb-ci commented Nov 14, 2024

Result of foundationdb-pr-clang-ide on Linux CentOS 7

foundationdb-ci commented Nov 14, 2024

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

foundationdb-ci commented Nov 14, 2024

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

foundationdb-ci commented Nov 14, 2024

Result of foundationdb-pr-macos on macOS Ventura 13.x

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment