diff --git a/build-support/common-build-env.sh b/build-support/common-build-env.sh index 9a14143b10fe..03fcc5b31fa3 100644 --- a/build-support/common-build-env.sh +++ b/build-support/common-build-env.sh @@ -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" diff --git a/src/yb/gen_yrpc/metric_descriptor.cc b/src/yb/gen_yrpc/metric_descriptor.cc index 327cc71ac0ce..dd9ea053edcb 100644 --- a/src/yb/gen_yrpc/metric_descriptor.cc +++ b/src/yb/gen_yrpc/metric_descriptor.cc @@ -15,6 +15,8 @@ #include +#include "yb/gen_yrpc/model.h" + namespace yb { namespace gen_yrpc { @@ -66,7 +68,8 @@ 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( @@ -74,8 +77,16 @@ void GenerateHandlerAssignment(YBPrinter printer) { "static_cast($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"); @@ -83,18 +94,19 @@ void GenerateHandlerAssignment(YBPrinter printer) { 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& 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; @@ -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"); } } diff --git a/src/yb/gen_yrpc/metric_descriptor.h b/src/yb/gen_yrpc/metric_descriptor.h index 450bf8d306dc..0b1358a7b3c1 100644 --- a/src/yb/gen_yrpc/metric_descriptor.h +++ b/src/yb/gen_yrpc/metric_descriptor.h @@ -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& metric_descriptors); } // namespace gen_yrpc diff --git a/src/yb/gen_yrpc/model.cc b/src/yb/gen_yrpc/model.cc index bc636cb1b90b..a824bf7fc696 100644 --- a/src/yb/gen_yrpc/model.cc +++ b/src/yb/gen_yrpc/model.cc @@ -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 { @@ -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)) { diff --git a/src/yb/gen_yrpc/model.h b/src/yb/gen_yrpc/model.h index 2c4d49ffeba7..148a669334f6 100644 --- a/src/yb/gen_yrpc/model.h +++ b/src/yb/gen_yrpc/model.h @@ -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); diff --git a/src/yb/gen_yrpc/service_generator.cc b/src/yb/gen_yrpc/service_generator.cc index 6be74211e489..ccf465297bef 100644 --- a/src/yb/gen_yrpc/service_generator.cc +++ b/src/yb/gen_yrpc/service_generator.cc @@ -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" @@ -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( @@ -160,6 +169,40 @@ void ServiceGenerator::Source(YBPrinter printer, const google::protobuf::FileDes printer("$open_namespace$\n"); + std::set 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); diff --git a/src/yb/rpc/rpc-test-base.cc b/src/yb/rpc/rpc-test-base.cc index 9013aa800226..c78d7b5e7733 100644 --- a/src/yb/rpc/rpc-test-base.cc +++ b/src/yb/rpc/rpc-test-base.cc @@ -380,6 +380,16 @@ class CalculatorService: public CalculatorServiceIf { context.RespondSuccess(); } + Result 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())); @@ -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 diff --git a/src/yb/rpc/rpc_context.h b/src/yb/rpc/rpc_context.h index 7b81fdbfc823..d7b2b8fca4a8 100644 --- a/src/yb/rpc/rpc_context.h +++ b/src/yb/rpc/rpc_context.h @@ -34,6 +34,8 @@ #include +#include + #include "yb/rpc/rpc_header.pb.h" #include "yb/rpc/serialization.h" #include "yb/rpc/service_if.h" @@ -144,6 +146,33 @@ class RpcCallLWParamsImpl : public RpcCallLWParams { Resp resp_; }; +template +using MutableErrorDetector = decltype(boost::declval().mutable_error()); + +template +struct ResponseErrorHelper; + +template <> +struct ResponseErrorHelper { + template + static auto Apply(T* t) { + return t->mutable_error(); + } +}; + +template <> +struct ResponseErrorHelper { + template + static auto Apply(T* t) { + return t->mutable_status(); + } +}; + +template +auto ResponseError(T* t) { + return ResponseErrorHelper>::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 @@ -276,6 +305,17 @@ class RpcContext { return *params_; } + template + void RespondTrivial(Result* 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. diff --git a/src/yb/rpc/rpc_stub-test.cc b/src/yb/rpc/rpc_stub-test.cc index eecea5fe1daa..8294d143f9ff 100644 --- a/src/yb/rpc/rpc_stub-test.cc +++ b/src/yb/rpc/rpc_stub-test.cc @@ -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 diff --git a/src/yb/rpc/rtest.proto b/src/yb/rpc/rtest.proto index ad25f6664da1..e9f77da53ebc 100644 --- a/src/yb/rpc/rtest.proto +++ b/src/yb/rpc/rtest.proto @@ -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 { @@ -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; +} diff --git a/src/yb/rpc/service.proto b/src/yb/rpc/service.proto index 1c28b17b1080..ff636785cafb 100644 --- a/src/yb/rpc/service.proto +++ b/src/yb/rpc/service.proto @@ -20,3 +20,7 @@ package yb.rpc; extend google.protobuf.ServiceOptions { string custom_service_name = 50011; } + +extend google.protobuf.MethodOptions { + bool trivial = 50001; +}