Skip to content

Commit 9ac94af

Browse files
committed
[#11205] Trivial RPC methods
Summary: Currently, most of our RPC endpoints has a trivial implementation, which takes request, synchronously process, and sends the response. But our generated service methods allow asynchronous processing, which complicates the writing of every method, even it doesn't require asynchronous processing. This diff adds the ability to specify that method is trivial. So his service virtual method will have signature: ```Result<ResponsePB> Method(const RequestPB& req, CoarseTimePoint deadline);``` instead of ```void Method(const RequestPB req, ResponsePB* resp, RpcContext context);``` Test Plan: ybd --gtest_filter RpcStubTest.Trivial Reviewers: dmitry Reviewed By: dmitry Subscribers: ybase, bogdan Differential Revision: https://phabricator.dev.yugabyte.com/D14990
1 parent 3b54d16 commit 9ac94af

11 files changed

+179
-16
lines changed

build-support/common-build-env.sh

+1
Original file line numberDiff line numberDiff line change
@@ -2231,6 +2231,7 @@ activate_virtualenv() {
22312231
pip_no_cache="--no-cache-dir"
22322232
fi
22332233

2234+
which pip3
22342235
local pip_executable=pip3
22352236
if ! "$yb_readonly_virtualenv"; then
22362237
local requirements_file_path="$YB_SRC_ROOT/requirements_frozen.txt"

src/yb/gen_yrpc/metric_descriptor.cc

+21-9
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515

1616
#include <google/protobuf/descriptor.h>
1717

18+
#include "yb/gen_yrpc/model.h"
19+
1820
namespace yb {
1921
namespace gen_yrpc {
2022

@@ -66,35 +68,45 @@ void GenerateMetricDefines(
6668
}
6769
}
6870

69-
void GenerateHandlerAssignment(YBPrinter printer) {
71+
void GenerateHandlerAssignment(
72+
YBPrinter printer, const google::protobuf::MethodDescriptor* method) {
7073
printer(".handler = [this](::yb::rpc::InboundCallPtr call) {\n");
7174
ScopedIndent handler_indent(printer);
7275
printer(
7376
"call->SetRpcMethodMetrics(methods_["
7477
"static_cast<size_t>($service_method_enum$::$metric_enum_key$)].metrics);\n"
7578
"::yb::rpc::HandleCall<::yb::rpc::$params$Impl<$request$, $response$>>(\n"
7679
" std::move(call), [this](const $request$* req, $response$* resp, "
77-
"::yb::rpc::RpcContext rpc_context) {\n"
78-
" $rpc_name$(req, resp, std::move(rpc_context));\n"
80+
"::yb::rpc::RpcContext rpc_context) {\n");
81+
if (IsTrivialMethod(method)) {
82+
printer(
83+
" auto result = $rpc_name$(*req, rpc_context.GetClientDeadline());\n"
84+
" rpc_context.RespondTrivial(&result, resp);\n"
85+
);
86+
} else {
87+
printer(" $rpc_name$(req, resp, std::move(rpc_context));\n");
88+
}
89+
printer(
7990
"});\n"
8091
);
8192
handler_indent.Reset("},\n");
8293
}
8394

8495
void GenerateMethodAssignments(
8596
YBPrinter printer, const google::protobuf::ServiceDescriptor* service,
86-
const std::string &mutable_metric_fmt, bool method,
97+
const std::string &mutable_metric_fmt, bool service_side,
8798
const std::vector<MetricDescriptor>& metric_descriptors) {
8899
ScopedIndent indent(printer);
89100

90101
for (int method_idx = 0; method_idx < service->method_count(); ++method_idx) {
91-
ScopedSubstituter method_subs(printer, service->method(method_idx), rpc::RpcSides::SERVICE);
102+
auto* method = service->method(method_idx);
103+
ScopedSubstituter method_subs(printer, method, rpc::RpcSides::SERVICE);
92104

93105
printer(mutable_metric_fmt + " = {\n");
94-
if (method) {
106+
if (service_side) {
95107
ScopedIndent method_indent(printer);
96108
printer(".method = ::yb::rpc::RemoteMethod(\"$full_service_name$\", \"$rpc_name$\"),\n");
97-
GenerateHandlerAssignment(printer);
109+
GenerateHandlerAssignment(printer, method);
98110
printer(".metrics = ::yb::rpc::RpcMethodMetrics(\n");
99111
}
100112
bool first = true;
@@ -105,13 +117,13 @@ void GenerateMethodAssignments(
105117
printer(",\n");
106118
}
107119
ScopedSubstituter metric_subs(printer, desc.CreateSubstitutions());
108-
if (method) {
120+
if (service_side) {
109121
printer(" ");
110122
}
111123
printer(
112124
" METRIC_$metric_prefix$$metric_name$_$rpc_full_name_plainchars$.Instantiate(entity)");
113125
}
114-
printer((method ? ")" : std::string()) + "\n};\n\n");
126+
printer((service_side ? ")" : std::string()) + "\n};\n\n");
115127
}
116128
}
117129

src/yb/gen_yrpc/metric_descriptor.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ void GenerateMetricDefines(
4141

4242
void GenerateMethodAssignments(
4343
YBPrinter printer, const google::protobuf::ServiceDescriptor* service,
44-
const std::string &mutable_metric_fmt, bool method,
44+
const std::string &mutable_metric_fmt, bool service_side,
4545
const std::vector<MetricDescriptor>& metric_descriptors);
4646

4747
} // namespace gen_yrpc

src/yb/gen_yrpc/model.cc

+6
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
#include "yb/gutil/strings/join.h"
2323
#include "yb/gutil/strings/split.h"
2424

25+
#include "yb/rpc/service.pb.h"
26+
2527
using google::protobuf::internal::WireFormatLite;
2628

2729
namespace yb {
@@ -87,6 +89,10 @@ bool IsLightweightMethod(const google::protobuf::MethodDescriptor* method, rpc::
8789
return options.sides() == side || options.sides() == rpc::RpcSides::BOTH;
8890
}
8991

92+
bool IsTrivialMethod(const google::protobuf::MethodDescriptor* method) {
93+
return method->options().GetExtension(rpc::trivial);
94+
}
95+
9096
bool HasLightweightMethod(const google::protobuf::ServiceDescriptor* service, rpc::RpcSides side) {
9197
for (int i = 0; i != service->method_count(); ++i) {
9298
if (IsLightweightMethod(service->method(i), side)) {

src/yb/gen_yrpc/model.h

+1
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ google::protobuf::internal::WireFormatLite::WireType WireType(
3434
size_t FixedSize(const google::protobuf::FieldDescriptor* field);
3535
std::string MakeLightweightName(const std::string& input);
3636
bool IsLightweightMethod(const google::protobuf::MethodDescriptor* method, rpc::RpcSides side);
37+
bool IsTrivialMethod(const google::protobuf::MethodDescriptor* method);
3738
bool HasLightweightMethod(const google::protobuf::ServiceDescriptor* service, rpc::RpcSides side);
3839
bool HasLightweightMethod(const google::protobuf::FileDescriptor* file, rpc::RpcSides side);
3940
std::string ReplaceNamespaceDelimiters(const std::string& arg_full_name);

src/yb/gen_yrpc/service_generator.cc

+49-6
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@ void ServiceGenerator::Header(YBPrinter printer, const google::protobuf::FileDes
7474
"#include \"yb/rpc/rpc_header.pb.h\"\n"
7575
"#include \"yb/rpc/service_if.h\"\n"
7676
"\n"
77+
"#include \"yb/util/monotime.h\"\n"
78+
"\n"
7779
"namespace yb {\n"
7880
"class MetricEntity;\n"
7981
"} // namespace yb\n"
@@ -106,12 +108,19 @@ void ServiceGenerator::Header(YBPrinter printer, const google::protobuf::FileDes
106108
const auto* method = service->method(method_idx);
107109
ScopedSubstituter method_subs(printer, method, rpc::RpcSides::SERVICE);
108110

109-
printer(
110-
" virtual void $rpc_name$(\n"
111-
" const $request$ *req,\n"
112-
" $response$ *resp,\n"
113-
" ::yb::rpc::RpcContext context) = 0;\n"
114-
);
111+
if (IsTrivialMethod(method)) {
112+
printer(
113+
" virtual ::yb::Result<$response$> $rpc_name$(\n"
114+
" const $request$& req, ::yb::CoarseTimePoint deadline) = 0;\n"
115+
);
116+
} else {
117+
printer(
118+
" virtual void $rpc_name$(\n"
119+
" const $request$* req,\n"
120+
" $response$* resp,\n"
121+
" ::yb::rpc::RpcContext context) = 0;\n"
122+
);
123+
}
115124
}
116125

117126
printer(
@@ -160,6 +169,40 @@ void ServiceGenerator::Source(YBPrinter printer, const google::protobuf::FileDes
160169

161170
printer("$open_namespace$\n");
162171

172+
std::set<std::string> error_types;
173+
for (int service_idx = 0; service_idx < file->service_count(); ++service_idx) {
174+
const auto* service = file->service(service_idx);
175+
for (int method_idx = 0; method_idx < service->method_count(); ++method_idx) {
176+
auto* method = service->method(method_idx);
177+
if (IsTrivialMethod(method)) {
178+
Lightweight lightweight(IsLightweightMethod(method, rpc::RpcSides::SERVICE));
179+
auto resp = method->output_type();
180+
std::string type;
181+
for (int field_idx = 0; field_idx < resp->field_count(); ++field_idx) {
182+
auto* field = resp->field(field_idx);
183+
if (field->name() == "error") {
184+
type = MapFieldType(field, lightweight);
185+
break;
186+
} else if (field->name() == "status") {
187+
type = MapFieldType(field, lightweight);
188+
// We don't break here, since error has greater priority than status.
189+
}
190+
}
191+
if (!type.empty()) {
192+
error_types.insert(type);
193+
}
194+
}
195+
}
196+
}
197+
198+
if (!error_types.empty()) {
199+
for (const auto& type : error_types) {
200+
printer("void SetupError(" + type + "* error, const Status& status);\n");
201+
}
202+
printer("\n");
203+
}
204+
205+
163206
for (int service_idx = 0; service_idx < file->service_count(); ++service_idx) {
164207
const auto* service = file->service(service_idx);
165208
ScopedSubstituter service_subs(printer, service);

src/yb/rpc/rpc-test-base.cc

+19
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,16 @@ class CalculatorService: public CalculatorServiceIf {
380380
context.RespondSuccess();
381381
}
382382

383+
Result<rpc_test::TrivialResponsePB> Trivial(
384+
const rpc_test::TrivialRequestPB& req, CoarseTimePoint deadline) override {
385+
if (req.value() < 0) {
386+
return STATUS_FORMAT(InvalidArgument, "Negative value: $0", req.value());
387+
}
388+
rpc_test::TrivialResponsePB resp;
389+
resp.set_value(req.value());
390+
return resp;
391+
}
392+
383393
private:
384394
void DoSleep(const SleepRequestPB* req, RpcContext context) {
385395
SleepFor(MonoDelta::FromMicroseconds(req->sleep_micros()));
@@ -608,4 +618,13 @@ MessengerBuilder RpcTestBase::CreateMessengerBuilder(const string &name,
608618
}
609619

610620
} // namespace rpc
621+
622+
namespace rpc_test {
623+
624+
void SetupError(TrivialErrorPB* error, const Status& status) {
625+
error->set_code(status.code());
626+
}
627+
628+
}
629+
611630
} // namespace yb

src/yb/rpc/rpc_context.h

+40
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@
3434

3535
#include <string>
3636

37+
#include <boost/type_traits/is_detected.hpp>
38+
3739
#include "yb/rpc/rpc_header.pb.h"
3840
#include "yb/rpc/serialization.h"
3941
#include "yb/rpc/service_if.h"
@@ -144,6 +146,33 @@ class RpcCallLWParamsImpl : public RpcCallLWParams {
144146
Resp resp_;
145147
};
146148

149+
template <class T>
150+
using MutableErrorDetector = decltype(boost::declval<T&>().mutable_error());
151+
152+
template <bool>
153+
struct ResponseErrorHelper;
154+
155+
template <>
156+
struct ResponseErrorHelper<true> {
157+
template <class T>
158+
static auto Apply(T* t) {
159+
return t->mutable_error();
160+
}
161+
};
162+
163+
template <>
164+
struct ResponseErrorHelper<false> {
165+
template <class T>
166+
static auto Apply(T* t) {
167+
return t->mutable_status();
168+
}
169+
};
170+
171+
template <class T>
172+
auto ResponseError(T* t) {
173+
return ResponseErrorHelper<boost::is_detected_v<MutableErrorDetector, T>>::Apply(t);
174+
}
175+
147176
// The context provided to a generated ServiceIf. This provides
148177
// methods to respond to the RPC. In the future, this will also
149178
// include methods to access information about the caller: e.g
@@ -276,6 +305,17 @@ class RpcContext {
276305
return *params_;
277306
}
278307

308+
template <class Response>
309+
void RespondTrivial(Result<Response>* result, Response* response) {
310+
if (result->ok()) {
311+
response->Swap(result->get_ptr());
312+
} else {
313+
SetupError(ResponseError(response), result->status());
314+
}
315+
RespondSuccess();
316+
}
317+
318+
279319
// Panic the server. This logs a fatal error with the given message, and
280320
// also includes the current RPC request, requestor, trace information, etc,
281321
// to make it easier to debug.

src/yb/rpc/rpc_stub-test.cc

+20
Original file line numberDiff line numberDiff line change
@@ -1037,5 +1037,25 @@ TEST_F(RpcStubTest, CustomServiceName) {
10371037
ASSERT_EQ(resp.result(), "yugabyte");
10381038
}
10391039

1040+
TEST_F(RpcStubTest, Trivial) {
1041+
CalculatorServiceProxy proxy(proxy_cache_.get(), server_hostport_);
1042+
1043+
RpcController controller;
1044+
controller.set_timeout(30s);
1045+
1046+
rpc_test::TrivialRequestPB req;
1047+
req.set_value(42);
1048+
rpc_test::TrivialResponsePB resp;
1049+
ASSERT_OK(proxy.Trivial(req, &resp, &controller));
1050+
ASSERT_EQ(resp.value(), req.value());
1051+
1052+
req.set_value(-1);
1053+
controller.Reset();
1054+
controller.set_timeout(30s);
1055+
ASSERT_OK(proxy.Trivial(req, &resp, &controller));
1056+
ASSERT_TRUE(resp.has_error());
1057+
ASSERT_EQ(resp.error().code(), Status::Code::kInvalidArgument);
1058+
}
1059+
10401060
} // namespace rpc
10411061
} // namespace yb

src/yb/rpc/rtest.proto

+17
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,10 @@ service CalculatorService {
147147
rpc Lightweight(LightweightRequestPB) returns (LightweightResponsePB) {
148148
option (yb.rpc.lightweight_method).sides = SERVICE;
149149
};
150+
151+
rpc Trivial(TrivialRequestPB) returns (TrivialResponsePB) {
152+
option (yb.rpc.trivial) = true;
153+
};
150154
}
151155

152156
message ConcatRequestPB {
@@ -257,3 +261,16 @@ message LightweightResponsePB {
257261

258262
optional string short_debug_string = 100;
259263
}
264+
265+
message TrivialRequestPB {
266+
optional int32 value = 1;
267+
}
268+
269+
message TrivialErrorPB {
270+
optional int32 code = 1;
271+
}
272+
273+
message TrivialResponsePB {
274+
optional TrivialErrorPB error = 1;
275+
optional int32 value = 2;
276+
}

src/yb/rpc/service.proto

+4
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,7 @@ package yb.rpc;
2020
extend google.protobuf.ServiceOptions {
2121
string custom_service_name = 50011;
2222
}
23+
24+
extend google.protobuf.MethodOptions {
25+
bool trivial = 50001;
26+
}

0 commit comments

Comments
 (0)