Skip to content

Commit

Permalink
[#11205] Trivial RPC methods
Browse files Browse the repository at this point in the history
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
  • Loading branch information
spolitov committed Jan 26, 2022
1 parent 3b54d16 commit 9ac94af
Show file tree
Hide file tree
Showing 11 changed files with 179 additions and 16 deletions.
1 change: 1 addition & 0 deletions build-support/common-build-env.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2231,6 +2231,7 @@ activate_virtualenv() {
pip_no_cache="--no-cache-dir"
fi

which pip3
local pip_executable=pip3
if ! "$yb_readonly_virtualenv"; then
local requirements_file_path="$YB_SRC_ROOT/requirements_frozen.txt"
Expand Down
30 changes: 21 additions & 9 deletions src/yb/gen_yrpc/metric_descriptor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@

#include <google/protobuf/descriptor.h>

#include "yb/gen_yrpc/model.h"

namespace yb {
namespace gen_yrpc {

Expand Down Expand Up @@ -66,35 +68,45 @@ void GenerateMetricDefines(
}
}

void GenerateHandlerAssignment(YBPrinter printer) {
void GenerateHandlerAssignment(
YBPrinter printer, const google::protobuf::MethodDescriptor* method) {
printer(".handler = [this](::yb::rpc::InboundCallPtr call) {\n");
ScopedIndent handler_indent(printer);
printer(
"call->SetRpcMethodMetrics(methods_["
"static_cast<size_t>($service_method_enum$::$metric_enum_key$)].metrics);\n"
"::yb::rpc::HandleCall<::yb::rpc::$params$Impl<$request$, $response$>>(\n"
" std::move(call), [this](const $request$* req, $response$* resp, "
"::yb::rpc::RpcContext rpc_context) {\n"
" $rpc_name$(req, resp, std::move(rpc_context));\n"
"::yb::rpc::RpcContext rpc_context) {\n");
if (IsTrivialMethod(method)) {
printer(
" auto result = $rpc_name$(*req, rpc_context.GetClientDeadline());\n"
" rpc_context.RespondTrivial(&result, resp);\n"
);
} else {
printer(" $rpc_name$(req, resp, std::move(rpc_context));\n");
}
printer(
"});\n"
);
handler_indent.Reset("},\n");
}

void GenerateMethodAssignments(
YBPrinter printer, const google::protobuf::ServiceDescriptor* service,
const std::string &mutable_metric_fmt, bool method,
const std::string &mutable_metric_fmt, bool service_side,
const std::vector<MetricDescriptor>& metric_descriptors) {
ScopedIndent indent(printer);

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

printer(mutable_metric_fmt + " = {\n");
if (method) {
if (service_side) {
ScopedIndent method_indent(printer);
printer(".method = ::yb::rpc::RemoteMethod(\"$full_service_name$\", \"$rpc_name$\"),\n");
GenerateHandlerAssignment(printer);
GenerateHandlerAssignment(printer, method);
printer(".metrics = ::yb::rpc::RpcMethodMetrics(\n");
}
bool first = true;
Expand All @@ -105,13 +117,13 @@ void GenerateMethodAssignments(
printer(",\n");
}
ScopedSubstituter metric_subs(printer, desc.CreateSubstitutions());
if (method) {
if (service_side) {
printer(" ");
}
printer(
" METRIC_$metric_prefix$$metric_name$_$rpc_full_name_plainchars$.Instantiate(entity)");
}
printer((method ? ")" : std::string()) + "\n};\n\n");
printer((service_side ? ")" : std::string()) + "\n};\n\n");
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/yb/gen_yrpc/metric_descriptor.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ void GenerateMetricDefines(

void GenerateMethodAssignments(
YBPrinter printer, const google::protobuf::ServiceDescriptor* service,
const std::string &mutable_metric_fmt, bool method,
const std::string &mutable_metric_fmt, bool service_side,
const std::vector<MetricDescriptor>& metric_descriptors);

} // namespace gen_yrpc
Expand Down
6 changes: 6 additions & 0 deletions src/yb/gen_yrpc/model.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
#include "yb/gutil/strings/join.h"
#include "yb/gutil/strings/split.h"

#include "yb/rpc/service.pb.h"

using google::protobuf::internal::WireFormatLite;

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

bool IsTrivialMethod(const google::protobuf::MethodDescriptor* method) {
return method->options().GetExtension(rpc::trivial);
}

bool HasLightweightMethod(const google::protobuf::ServiceDescriptor* service, rpc::RpcSides side) {
for (int i = 0; i != service->method_count(); ++i) {
if (IsLightweightMethod(service->method(i), side)) {
Expand Down
1 change: 1 addition & 0 deletions src/yb/gen_yrpc/model.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ google::protobuf::internal::WireFormatLite::WireType WireType(
size_t FixedSize(const google::protobuf::FieldDescriptor* field);
std::string MakeLightweightName(const std::string& input);
bool IsLightweightMethod(const google::protobuf::MethodDescriptor* method, rpc::RpcSides side);
bool IsTrivialMethod(const google::protobuf::MethodDescriptor* method);
bool HasLightweightMethod(const google::protobuf::ServiceDescriptor* service, rpc::RpcSides side);
bool HasLightweightMethod(const google::protobuf::FileDescriptor* file, rpc::RpcSides side);
std::string ReplaceNamespaceDelimiters(const std::string& arg_full_name);
Expand Down
55 changes: 49 additions & 6 deletions src/yb/gen_yrpc/service_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ void ServiceGenerator::Header(YBPrinter printer, const google::protobuf::FileDes
"#include \"yb/rpc/rpc_header.pb.h\"\n"
"#include \"yb/rpc/service_if.h\"\n"
"\n"
"#include \"yb/util/monotime.h\"\n"
"\n"
"namespace yb {\n"
"class MetricEntity;\n"
"} // namespace yb\n"
Expand Down Expand Up @@ -106,12 +108,19 @@ void ServiceGenerator::Header(YBPrinter printer, const google::protobuf::FileDes
const auto* method = service->method(method_idx);
ScopedSubstituter method_subs(printer, method, rpc::RpcSides::SERVICE);

printer(
" virtual void $rpc_name$(\n"
" const $request$ *req,\n"
" $response$ *resp,\n"
" ::yb::rpc::RpcContext context) = 0;\n"
);
if (IsTrivialMethod(method)) {
printer(
" virtual ::yb::Result<$response$> $rpc_name$(\n"
" const $request$& req, ::yb::CoarseTimePoint deadline) = 0;\n"
);
} else {
printer(
" virtual void $rpc_name$(\n"
" const $request$* req,\n"
" $response$* resp,\n"
" ::yb::rpc::RpcContext context) = 0;\n"
);
}
}

printer(
Expand Down Expand Up @@ -160,6 +169,40 @@ void ServiceGenerator::Source(YBPrinter printer, const google::protobuf::FileDes

printer("$open_namespace$\n");

std::set<std::string> error_types;
for (int service_idx = 0; service_idx < file->service_count(); ++service_idx) {
const auto* service = file->service(service_idx);
for (int method_idx = 0; method_idx < service->method_count(); ++method_idx) {
auto* method = service->method(method_idx);
if (IsTrivialMethod(method)) {
Lightweight lightweight(IsLightweightMethod(method, rpc::RpcSides::SERVICE));
auto resp = method->output_type();
std::string type;
for (int field_idx = 0; field_idx < resp->field_count(); ++field_idx) {
auto* field = resp->field(field_idx);
if (field->name() == "error") {
type = MapFieldType(field, lightweight);
break;
} else if (field->name() == "status") {
type = MapFieldType(field, lightweight);
// We don't break here, since error has greater priority than status.
}
}
if (!type.empty()) {
error_types.insert(type);
}
}
}
}

if (!error_types.empty()) {
for (const auto& type : error_types) {
printer("void SetupError(" + type + "* error, const Status& status);\n");
}
printer("\n");
}


for (int service_idx = 0; service_idx < file->service_count(); ++service_idx) {
const auto* service = file->service(service_idx);
ScopedSubstituter service_subs(printer, service);
Expand Down
19 changes: 19 additions & 0 deletions src/yb/rpc/rpc-test-base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,16 @@ class CalculatorService: public CalculatorServiceIf {
context.RespondSuccess();
}

Result<rpc_test::TrivialResponsePB> Trivial(
const rpc_test::TrivialRequestPB& req, CoarseTimePoint deadline) override {
if (req.value() < 0) {
return STATUS_FORMAT(InvalidArgument, "Negative value: $0", req.value());
}
rpc_test::TrivialResponsePB resp;
resp.set_value(req.value());
return resp;
}

private:
void DoSleep(const SleepRequestPB* req, RpcContext context) {
SleepFor(MonoDelta::FromMicroseconds(req->sleep_micros()));
Expand Down Expand Up @@ -608,4 +618,13 @@ MessengerBuilder RpcTestBase::CreateMessengerBuilder(const string &name,
}

} // namespace rpc

namespace rpc_test {

void SetupError(TrivialErrorPB* error, const Status& status) {
error->set_code(status.code());
}

}

} // namespace yb
40 changes: 40 additions & 0 deletions src/yb/rpc/rpc_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@

#include <string>

#include <boost/type_traits/is_detected.hpp>

#include "yb/rpc/rpc_header.pb.h"
#include "yb/rpc/serialization.h"
#include "yb/rpc/service_if.h"
Expand Down Expand Up @@ -144,6 +146,33 @@ class RpcCallLWParamsImpl : public RpcCallLWParams {
Resp resp_;
};

template <class T>
using MutableErrorDetector = decltype(boost::declval<T&>().mutable_error());

template <bool>
struct ResponseErrorHelper;

template <>
struct ResponseErrorHelper<true> {
template <class T>
static auto Apply(T* t) {
return t->mutable_error();
}
};

template <>
struct ResponseErrorHelper<false> {
template <class T>
static auto Apply(T* t) {
return t->mutable_status();
}
};

template <class T>
auto ResponseError(T* t) {
return ResponseErrorHelper<boost::is_detected_v<MutableErrorDetector, T>>::Apply(t);
}

// The context provided to a generated ServiceIf. This provides
// methods to respond to the RPC. In the future, this will also
// include methods to access information about the caller: e.g
Expand Down Expand Up @@ -276,6 +305,17 @@ class RpcContext {
return *params_;
}

template <class Response>
void RespondTrivial(Result<Response>* result, Response* response) {
if (result->ok()) {
response->Swap(result->get_ptr());
} else {
SetupError(ResponseError(response), result->status());
}
RespondSuccess();
}


// Panic the server. This logs a fatal error with the given message, and
// also includes the current RPC request, requestor, trace information, etc,
// to make it easier to debug.
Expand Down
20 changes: 20 additions & 0 deletions src/yb/rpc/rpc_stub-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1037,5 +1037,25 @@ TEST_F(RpcStubTest, CustomServiceName) {
ASSERT_EQ(resp.result(), "yugabyte");
}

TEST_F(RpcStubTest, Trivial) {
CalculatorServiceProxy proxy(proxy_cache_.get(), server_hostport_);

RpcController controller;
controller.set_timeout(30s);

rpc_test::TrivialRequestPB req;
req.set_value(42);
rpc_test::TrivialResponsePB resp;
ASSERT_OK(proxy.Trivial(req, &resp, &controller));
ASSERT_EQ(resp.value(), req.value());

req.set_value(-1);
controller.Reset();
controller.set_timeout(30s);
ASSERT_OK(proxy.Trivial(req, &resp, &controller));
ASSERT_TRUE(resp.has_error());
ASSERT_EQ(resp.error().code(), Status::Code::kInvalidArgument);
}

} // namespace rpc
} // namespace yb
17 changes: 17 additions & 0 deletions src/yb/rpc/rtest.proto
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,10 @@ service CalculatorService {
rpc Lightweight(LightweightRequestPB) returns (LightweightResponsePB) {
option (yb.rpc.lightweight_method).sides = SERVICE;
};

rpc Trivial(TrivialRequestPB) returns (TrivialResponsePB) {
option (yb.rpc.trivial) = true;
};
}

message ConcatRequestPB {
Expand Down Expand Up @@ -257,3 +261,16 @@ message LightweightResponsePB {

optional string short_debug_string = 100;
}

message TrivialRequestPB {
optional int32 value = 1;
}

message TrivialErrorPB {
optional int32 code = 1;
}

message TrivialResponsePB {
optional TrivialErrorPB error = 1;
optional int32 value = 2;
}
4 changes: 4 additions & 0 deletions src/yb/rpc/service.proto
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,7 @@ package yb.rpc;
extend google.protobuf.ServiceOptions {
string custom_service_name = 50011;
}

extend google.protobuf.MethodOptions {
bool trivial = 50001;
}

0 comments on commit 9ac94af

Please sign in to comment.