This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new 7f794e3 [rpc] update ServiceIf::service_name() signature
7f794e3 is described below
commit 7f794e3ac8c389af4c74faeac11274e4ef0a55c0
Author: Alexey Serbin <[email protected]>
AuthorDate: Sat Jan 2 23:48:17 2021 -0800
[rpc] update ServiceIf::service_name() signature
This patch changes the signature of the ServiceIf::service_name() method
to return 'const std::string&' instead of 'std::string' and updates a few
related call sites accordingly. In addition, this patch removes unused
ServiceIf::methods_by_name() accessor method and makes
ServiceIf::ParseParam() and ServiceIf::RespondBadMethod() methods
static.
The ServiceIf::service_name() method isn't in any hot path but it's used
in ServicePool::RejectTooBusy(). The latter is usually called under
high memory pressure when it's preferred to avoid memory allocations
at least because of the lock contention in the tcmalloc library.
I also did a few formatting changes in protoc-gen-krpc.cc to improve
the readability of the auto-generated code since I looked at that code
recently.
Change-Id: Ifa7fc425cf789fa31e99642061b6474b0effd489
Reviewed-on: http://gerrit.cloudera.org:8080/16916
Tested-by: Alexey Serbin <[email protected]>
Reviewed-by: Mahesh Reddy <[email protected]>
Reviewed-by: Andrew Wong <[email protected]>
---
src/kudu/rpc/protoc-gen-krpc.cc | 488 ++++++++++++++++++++--------------------
src/kudu/rpc/rpc-test-base.h | 14 +-
src/kudu/rpc/service_if.cc | 16 +-
src/kudu/rpc/service_if.h | 23 +-
src/kudu/rpc/service_pool.cc | 2 +-
src/kudu/rpc/service_pool.h | 2 +-
src/kudu/server/rpc_server.cc | 4 +-
7 files changed, 278 insertions(+), 271 deletions(-)
diff --git a/src/kudu/rpc/protoc-gen-krpc.cc b/src/kudu/rpc/protoc-gen-krpc.cc
index 8c64e2a..2226160 100644
--- a/src/kudu/rpc/protoc-gen-krpc.cc
+++ b/src/kudu/rpc/protoc-gen-krpc.cc
@@ -155,26 +155,25 @@ class FileSubstitutions : public Substituter {
class MethodSubstitutions : public Substituter {
public:
explicit MethodSubstitutions(const MethodDescriptor* method)
- : method_(method) {
+ : method_(method) {
}
void InitSubstitutionMap(map<string, string>* map) const override {
- (*map)["rpc_name"] = method_->name();
- (*map)["rpc_full_name"] = method_->full_name();
- (*map)["rpc_full_name_plainchars"] =
+ auto& m = *map;
+ m["rpc_name"] = method_->name();
+ m["rpc_full_name"] = method_->full_name();
+ m["rpc_full_name_plainchars"] =
StringReplace(method_->full_name(), ".", "_", true);
- (*map)["request"] =
- ReplaceNamespaceDelimiters(
- StripNamespaceIfPossible(method_->service()->full_name(),
- method_->input_type()->full_name()));
- (*map)["response"] =
- ReplaceNamespaceDelimiters(
- StripNamespaceIfPossible(method_->service()->full_name(),
- method_->output_type()->full_name()));
- (*map)["metric_enum_key"] = Substitute("kMetricIndex$0", method_->name());
+ m["request"] = ReplaceNamespaceDelimiters(
+ StripNamespaceIfPossible(method_->service()->full_name(),
+ method_->input_type()->full_name()));
+ m["response"] = ReplaceNamespaceDelimiters(
+ StripNamespaceIfPossible(method_->service()->full_name(),
+ method_->output_type()->full_name()));
+ m["metric_enum_key"] = Substitute("kMetricIndex$0", method_->name());
bool track_result =
static_cast<bool>(method_->options().GetExtension(track_rpc_result));
- (*map)["track_result"] = track_result ? " true" : "false";
- (*map)["authz_method"] =
GetAuthzMethod(*method_).get_value_or("AuthorizeAllowAll");
+ m["track_result"] = track_result ? " true" : "false";
+ m["authz_method"] =
GetAuthzMethod(*method_).get_value_or("AuthorizeAllowAll");
}
// Strips the package from method arguments if they are in the same package
as
@@ -209,15 +208,16 @@ class MethodSubstitutions : public Substituter {
class ServiceSubstitutions : public Substituter {
public:
explicit ServiceSubstitutions(const ServiceDescriptor* service)
- : service_(service)
- {}
+ : service_(service) {
+ }
void InitSubstitutionMap(map<string, string>* map) const override {
- (*map)["service_name"] = service_->name();
- (*map)["full_service_name"] = service_->full_name();
- (*map)["service_method_count"] = SimpleItoa(service_->method_count());
+ auto& m = *map;
+ m["service_name"] = service_->name();
+ m["full_service_name"] = service_->full_name();
+ m["service_method_count"] = SimpleItoa(service_->method_count());
- // TODO: upgrade to protobuf 2.5.x and attach service comments
+ // TODO(todd): upgrade to protobuf 2.5.x and attach service comments
// to the generated service classes using the SourceLocation API.
}
@@ -225,7 +225,6 @@ class ServiceSubstitutions : public Substituter {
const ServiceDescriptor* service_;
};
-
class SubstitutionContext {
public:
// Takes ownership of the substituter.
@@ -256,8 +255,6 @@ class SubstitutionContext {
vector<unique_ptr<const Substituter>> subs_;
};
-
-
class CodeGenerator : public ::google::protobuf::compiler::CodeGenerator {
public:
CodeGenerator() { }
@@ -316,31 +313,30 @@ class CodeGenerator : public
::google::protobuf::compiler::CodeGenerator {
SubstitutionContext* subs,
const FileDescriptor* file) {
Print(printer, *subs,
- "// THIS FILE IS AUTOGENERATED FROM $path$\n"
- "\n"
- "#pragma once\n"
- "\n"
- "#include <string>\n"
- "\n"
- "#include \"kudu/gutil/ref_counted.h\"\n"
- "#include \"kudu/rpc/service_if.h\"\n"
- "\n"
- "namespace google {\n"
- "namespace protobuf {\n"
- "class Message;\n"
- "} // namespace protobuf\n"
- "} // namespace google\n"
- "\n"
- "namespace kudu {\n"
- "class MetricEntity;\n"
- "namespace rpc {\n"
- "class ResultTracker;\n"
- "class RpcContext;\n"
- "} // namespace rpc\n"
- "} // namespace kudu\n"
- "\n"
- "$open_namespace$"
- "\n");
+ "// THIS FILE IS AUTOGENERATED FROM $path$\n"
+ "\n"
+ "#pragma once\n"
+ "\n"
+ "#include <string>\n"
+ "\n"
+ "#include \"kudu/gutil/ref_counted.h\"\n"
+ "#include \"kudu/rpc/service_if.h\"\n"
+ "\n"
+ "namespace google {\n"
+ "namespace protobuf {\n"
+ "class Message;\n"
+ "} // namespace protobuf\n"
+ "} // namespace google\n"
+ "\n"
+ "namespace kudu {\n"
+ "class MetricEntity;\n"
+ "namespace rpc {\n"
+ "class ResultTracker;\n"
+ "class RpcContext;\n"
+ "} // namespace rpc\n"
+ "} // namespace kudu\n"
+ "\n"
+ "$open_namespace$");
for (int service_idx = 0; service_idx < file->service_count();
++service_idx) {
@@ -348,26 +344,30 @@ class CodeGenerator : public
::google::protobuf::compiler::CodeGenerator {
subs->PushService(service);
Print(printer, *subs,
- "class $service_name$If : public ::kudu::rpc::GeneratedServiceIf {\n"
- " public:\n"
- " explicit $service_name$If(const
scoped_refptr<::kudu::MetricEntity>& entity,"
- " const scoped_refptr<::kudu::rpc::ResultTracker>&
result_tracker);\n"
- " virtual ~$service_name$If();\n"
- " std::string service_name() const override;\n"
- " static std::string static_service_name();\n"
- "\n"
- );
+ "\n"
+ "class $service_name$If : public ::kudu::rpc::GeneratedServiceIf {\n"
+ " public:\n"
+ " static const std::string& static_service_name();\n"
+ "\n"
+ " $service_name$If(\n"
+ " const scoped_refptr<::kudu::MetricEntity>& entity,\n"
+ " const scoped_refptr<::kudu::rpc::ResultTracker>&
result_tracker);\n"
+ " virtual ~$service_name$If();\n"
+ "\n"
+ " const std::string& service_name() const override;\n");
set<string> authz_methods;
for (int method_idx = 0; method_idx < service->method_count();
++method_idx) {
- const MethodDescriptor *method = service->method(method_idx);
+ const MethodDescriptor* method = service->method(method_idx);
subs->PushMethod(method);
Print(printer, *subs,
- " virtual void $rpc_name$(const class $request$ *req,\n"
- " class $response$ *resp, ::kudu::rpc::RpcContext *context) =
0;\n"
- );
+ "\n"
+ " virtual void $rpc_name$(\n"
+ " const class $request$* req,\n"
+ " class $response$* resp,\n"
+ " ::kudu::rpc::RpcContext* context) = 0;\n");
subs->Pop(); // method
if (auto m = GetAuthzMethod(*method)) {
authz_methods.insert(m.get());
@@ -376,50 +376,53 @@ class CodeGenerator : public
::google::protobuf::compiler::CodeGenerator {
if (!authz_methods.empty()) {
printer->Print(
- "\n\n"
- " // Authorization methods\n"
- " // ---------------------\n\n");
+ "\n"
+ " // Authorization methods\n"
+ " // ---------------------\n");
}
for (const string& m : authz_methods) {
printer->Print({ {"m", m} },
- " virtual bool $m$(const google::protobuf::Message* req,\n"
- " google::protobuf::Message* resp, ::kudu::rpc::RpcContext
*context) = 0;\n");
+ "\n"
+ " virtual bool $m$(\n"
+ " const google::protobuf::Message* req,\n"
+ " google::protobuf::Message* resp,\n"
+ " ::kudu::rpc::RpcContext* context) = 0;\n");
}
Print(printer, *subs,
- "\n"
- "};\n"
- );
+ "\n"
+ " private:\n"
+ " static const std::string kServiceName_;\n"
+ "};\n");
subs->Pop(); // Service
}
Print(printer, *subs,
- "\n"
- "$close_namespace$"
- "\n");
+ "\n"
+ "$close_namespace$");
}
static void GenerateServiceIf(Printer* printer,
SubstitutionContext* subs,
const FileDescriptor* file) {
Print(printer, *subs,
- "// THIS FILE IS AUTOGENERATED FROM $path$\n"
- "\n"
- "#include <functional>\n"
- "#include <memory>\n"
- "#include <unordered_map>\n"
- "#include <utility>\n"
- "\n"
- "#include <google/protobuf/message.h>\n"
- "\n"
- "#include \"$path_no_extension$.pb.h\"\n"
- "#include \"$path_no_extension$.service.h\"\n"
- "\n"
- "#include \"kudu/rpc/result_tracker.h\"\n"
- "#include \"kudu/rpc/service_if.h\"\n"
- "#include \"kudu/util/metrics.h\"\n"
- "\n");
+ "// THIS FILE IS AUTOGENERATED FROM $path$\n"
+ "\n"
+ "#include <functional>\n"
+ "#include <memory>\n"
+ "#include <unordered_map>\n"
+ "#include <utility>\n"
+ "\n"
+ "#include <google/protobuf/message.h>\n"
+ "\n"
+ "#include \"$path_no_extension$.pb.h\"\n"
+ "#include \"$path_no_extension$.service.h\"\n"
+ "\n"
+ "#include \"kudu/rpc/result_tracker.h\"\n"
+ "#include \"kudu/rpc/service_if.h\"\n"
+ "#include \"kudu/util/metrics.h\"\n"
+ "\n");
// Define metric prototypes for each method in the service.
for (int service_idx = 0; service_idx < file->service_count();
@@ -432,20 +435,22 @@ class CodeGenerator : public
::google::protobuf::compiler::CodeGenerator {
const MethodDescriptor* method = service->method(method_idx);
subs->PushMethod(method);
Print(printer, *subs,
- "METRIC_DEFINE_histogram(server,
handler_latency_$rpc_full_name_plainchars$,\n"
- " \"$rpc_full_name$ RPC Time\",\n"
- " kudu::MetricUnit::kMicroseconds,\n"
- " \"Microseconds spent handling $rpc_full_name$() RPC requests\",\n"
- " kudu::MetricLevel::kInfo,\n"
- " 60000000LU, 2);\n"
- "\n");
+ "METRIC_DEFINE_histogram(server,\n"
+ " handler_latency_$rpc_full_name_plainchars$,\n"
+ " \"$rpc_full_name$ RPC Time\",\n"
+ " kudu::MetricUnit::kMicroseconds,\n"
+ " \"Microseconds spent handling $rpc_full_name$ RPC
requests\",\n"
+ " kudu::MetricLevel::kInfo,\n"
+ " 60000000LU, 2);\n"
+ "\n");
Print(printer, *subs,
- "METRIC_DEFINE_counter(server,
queue_overflow_rejections_$rpc_full_name_plainchars$,\n"
- " \"$rpc_full_name$ RPC Rejections\",\n"
- " kudu::MetricUnit::kRequests,\n"
- " \"Number of rejected $rpc_full_name$() requests due to RPC queue
overflow\",\n"
- " kudu::MetricLevel::kInfo);\n"
- "\n");
+ "METRIC_DEFINE_counter(server,\n"
+ " queue_overflow_rejections_$rpc_full_name_plainchars$,\n"
+ " \"$rpc_full_name$ RPC Rejections\",\n"
+ " kudu::MetricUnit::kRequests,\n"
+ " \"Number of rejected $rpc_full_name$ requests due to RPC
queue overflow\",\n"
+ " kudu::MetricLevel::kInfo);\n"
+ "\n");
subs->Pop(); // method
}
@@ -453,15 +458,14 @@ class CodeGenerator : public
::google::protobuf::compiler::CodeGenerator {
}
Print(printer, *subs,
- "using google::protobuf::Message;\n"
- "using kudu::MetricEntity;\n"
- "using kudu::rpc::ResultTracker;\n"
- "using kudu::rpc::RpcContext;\n"
- "using kudu::rpc::RpcMethodInfo;\n"
- "using std::unique_ptr;\n"
- "\n"
- "$open_namespace$"
- "\n");
+ "using google::protobuf::Message;\n"
+ "using kudu::MetricEntity;\n"
+ "using kudu::rpc::ResultTracker;\n"
+ "using kudu::rpc::RpcContext;\n"
+ "using kudu::rpc::RpcMethodInfo;\n"
+ "using std::string;\n"
+ "\n"
+ "$open_namespace$");
for (int service_idx = 0; service_idx < file->service_count();
++service_idx) {
@@ -469,9 +473,17 @@ class CodeGenerator : public
::google::protobuf::compiler::CodeGenerator {
subs->PushService(service);
Print(printer, *subs,
- "$service_name$If::$service_name$If(const scoped_refptr<MetricEntity>&
entity,"
- " const scoped_refptr<ResultTracker>& result_tracker) {\n"
- "result_tracker_ = result_tracker;\n"
+ "\n"
+ "const string $service_name$If::kServiceName_ =
\"$full_service_name$\";\n"
+ "\n"
+ "const string& $service_name$If::static_service_name() {\n"
+ " return kServiceName_;\n"
+ "}\n"
+ "\n"
+ "$service_name$If::$service_name$If(\n"
+ " const scoped_refptr<MetricEntity>& entity,\n"
+ " const scoped_refptr<ResultTracker>& result_tracker)\n"
+ " : GeneratedServiceIf(result_tracker) {\n"
);
for (int method_idx = 0; method_idx < service->method_count();
++method_idx) {
@@ -479,82 +491,76 @@ class CodeGenerator : public
::google::protobuf::compiler::CodeGenerator {
subs->PushMethod(method);
Print(printer, *subs,
- " {\n"
- " scoped_refptr<RpcMethodInfo> mi(new RpcMethodInfo);\n"
- " mi->req_prototype.reset(new $request$);\n"
- " mi->resp_prototype.reset(new $response$);\n"
- " mi->authz_method = [this](const Message* req, Message*
resp,\n"
- " RpcContext* ctx) {\n"
- " return this->$authz_method$(static_cast<const
$request$*>(req),\n"
- " static_cast<$response$*>(resp),\n"
- " ctx);\n"
- " };\n"
- " mi->track_result = $track_result$;\n"
- " mi->handler_latency_histogram =\n"
- "
METRIC_handler_latency_$rpc_full_name_plainchars$.Instantiate(entity);\n"
- " mi->queue_overflow_rejections =\n"
- "
METRIC_queue_overflow_rejections_$rpc_full_name_plainchars$.Instantiate(\n"
- " entity);\n"
- " mi->func = [this](const Message* req, Message* resp,
RpcContext* ctx) {\n"
- " this->$rpc_name$(static_cast<const $request$*>(req),\n"
- " static_cast<$response$*>(resp),\n"
- " ctx);\n"
- " };\n"
- " methods_by_name_[\"$rpc_name$\"] = std::move(mi);\n"
- " }\n");
+ " {\n"
+ " scoped_refptr<RpcMethodInfo> mi(new RpcMethodInfo);\n"
+ " mi->req_prototype.reset(new $request$);\n"
+ " mi->resp_prototype.reset(new $response$);\n"
+ " mi->authz_method = [this](const Message* req, Message*
resp,\n"
+ " RpcContext* ctx) {\n"
+ " return this->$authz_method$(\n"
+ " static_cast<const $request$*>(req),\n"
+ " static_cast<$response$*>(resp),\n"
+ " ctx);\n"
+ " };\n"
+ " mi->track_result = $track_result$;\n"
+ " mi->handler_latency_histogram =\n"
+ "
METRIC_handler_latency_$rpc_full_name_plainchars$.Instantiate(entity);\n"
+ " mi->queue_overflow_rejections =\n"
+ "
METRIC_queue_overflow_rejections_$rpc_full_name_plainchars$.Instantiate("
+ "entity);\n"
+ " mi->func = [this](const Message* req, Message* resp,
RpcContext* ctx) {\n"
+ " this->$rpc_name$(\n"
+ " static_cast<const $request$*>(req),\n"
+ " static_cast<$response$*>(resp),\n"
+ " ctx);\n"
+ " };\n"
+ " methods_by_name_[\"$rpc_name$\"] = std::move(mi);\n"
+ " }\n");
subs->Pop(); // method
}
Print(printer, *subs,
- "}\n"
- "\n"
- "$service_name$If::~$service_name$If() {\n"
- "}\n"
- "\n"
- "std::string $service_name$If::service_name() const {\n"
- " return \"$full_service_name$\";\n"
- "}\n"
- "std::string $service_name$If::static_service_name() {\n"
- " return \"$full_service_name$\";\n"
- "}\n"
- "\n"
- );
+ "}\n"
+ "\n"
+ "$service_name$If::~$service_name$If() {\n"
+ "}\n"
+ "\n"
+ "const string& $service_name$If::service_name() const {\n"
+ " return kServiceName_;\n"
+ "}\n");
subs->Pop(); // service
}
Print(printer, *subs,
- "\n"
- "$close_namespace$"
- "\n"
- );
+ "\n"
+ "$close_namespace$");
}
static void GenerateProxyHeader(Printer* printer,
SubstitutionContext* subs,
const FileDescriptor* file) {
Print(printer, *subs,
- "// THIS FILE IS AUTOGENERATED FROM $path$\n"
- "\n"
- "#pragma once\n"
- "\n"
- "#include <memory>\n"
- "#include <string>\n"
- "\n"
- "#include \"kudu/rpc/proxy.h\"\n"
- "#include \"kudu/rpc/response_callback.h\"\n"
- "#include \"kudu/util/status.h\"\n"
- "\n"
- "namespace kudu { class Sockaddr; }\n"
- "namespace kudu {\n"
- "namespace rpc {\n"
- "class Messenger;\n"
- "class RpcController;\n"
- "} // namespace rpc\n"
- "} // namespace kudu\n"
- "\n"
- "$open_namespace$"
- "\n");
+ "// THIS FILE IS AUTOGENERATED FROM $path$\n"
+ "\n"
+ "#pragma once\n"
+ "\n"
+ "#include <memory>\n"
+ "#include <string>\n"
+ "\n"
+ "#include \"kudu/rpc/proxy.h\"\n"
+ "#include \"kudu/rpc/response_callback.h\"\n"
+ "#include \"kudu/util/status.h\"\n"
+ "\n"
+ "namespace kudu { class Sockaddr; }\n"
+ "namespace kudu {\n"
+ "namespace rpc {\n"
+ "class Messenger;\n"
+ "class RpcController;\n"
+ "} // namespace rpc\n"
+ "} // namespace kudu\n"
+ "\n"
+ "$open_namespace$");
for (int service_idx = 0; service_idx < file->service_count();
++service_idx) {
@@ -562,14 +568,14 @@ class CodeGenerator : public
::google::protobuf::compiler::CodeGenerator {
subs->PushService(service);
Print(printer, *subs,
- "class $service_name$Proxy : public ::kudu::rpc::Proxy {\n"
- " public:\n"
- " $service_name$Proxy(std::shared_ptr<::kudu::rpc::Messenger>\n"
- " messenger, const ::kudu::Sockaddr &sockaddr,"
- " std::string hostname);\n"
- " ~$service_name$Proxy();\n"
- "\n"
- );
+ "\n"
+ "class $service_name$Proxy : public ::kudu::rpc::Proxy {\n"
+ " public:\n"
+ " $service_name$Proxy(\n"
+ " std::shared_ptr<::kudu::rpc::Messenger> messenger,\n"
+ " const ::kudu::Sockaddr& sockaddr,\n"
+ " std::string hostname);\n"
+ " ~$service_name$Proxy();\n");
for (int method_idx = 0; method_idx < service->method_count();
++method_idx) {
@@ -577,92 +583,96 @@ class CodeGenerator : public
::google::protobuf::compiler::CodeGenerator {
subs->PushMethod(method);
Print(printer, *subs,
- "\n"
- " ::kudu::Status $rpc_name$(const class $request$ &req,\n"
- " class $response$ *resp,\n"
- " ::kudu::rpc::RpcController
*controller);\n"
- " void $rpc_name$Async(const class $request$ &req,\n"
- " class $response$ *response,\n"
- " ::kudu::rpc::RpcController *controller,\n"
- " const ::kudu::rpc::ResponseCallback
&callback);\n"
- );
+ "\n"
+ " ::kudu::Status $rpc_name$(\n"
+ " const class $request$& req,\n"
+ " class $response$* resp,\n"
+ " ::kudu::rpc::RpcController* controller);\n"
+ " void $rpc_name$Async(\n"
+ " const class $request$& req,\n"
+ " class $response$* response,\n"
+ " ::kudu::rpc::RpcController* controller,\n"
+ " const ::kudu::rpc::ResponseCallback& callback);\n");
subs->Pop(); // method
}
- Print(printer, *subs,
- "};\n");
+ Print(printer, *subs, "};\n");
subs->Pop(); // service
}
Print(printer, *subs,
- "\n"
- "$close_namespace$"
- "\n"
- );
+ "\n"
+ "$close_namespace$");
}
static void GenerateProxy(Printer* printer,
SubstitutionContext* subs,
const FileDescriptor* file) {
Print(printer, *subs,
- "// THIS FILE IS AUTOGENERATED FROM $path$\n"
- "\n"
- "#include <string>\n"
- "#include <utility>\n"
- "\n"
- "#include \"$path_no_extension$.pb.h\"\n"
- "#include \"$path_no_extension$.proxy.h\"\n"
- "\n"
- "namespace kudu {\n"
- "namespace rpc {\n"
- "class Messenger;\n"
- "class RpcController;\n"
- "} // namespace rpc\n"
- "} // namespace kudu\n"
- "\n"
- "$open_namespace$"
- "\n");
+ "// THIS FILE IS AUTOGENERATED FROM $path$\n"
+ "\n"
+ "#include <string>\n"
+ "#include <utility>\n"
+ "\n"
+ "#include \"$path_no_extension$.pb.h\"\n"
+ "#include \"$path_no_extension$.proxy.h\"\n"
+ "\n"
+ "namespace kudu {\n"
+ "namespace rpc {\n"
+ "class Messenger;\n"
+ "class RpcController;\n"
+ "} // namespace rpc\n"
+ "} // namespace kudu\n"
+ "\n"
+ "$open_namespace$");
for (int service_idx = 0; service_idx < file->service_count();
++service_idx) {
const ServiceDescriptor* service = file->service(service_idx);
subs->PushService(service);
Print(printer, *subs,
- "$service_name$Proxy::$service_name$Proxy(\n"
- " std::shared_ptr< ::kudu::rpc::Messenger> messenger,\n"
- " const ::kudu::Sockaddr &remote, std::string hostname)\n"
- " : Proxy(std::move(messenger), remote, std::move(hostname),
\"$full_service_name$\") {\n"
- "}\n"
- "\n"
- "$service_name$Proxy::~$service_name$Proxy() {\n"
- "}\n"
- "\n"
- "\n");
+ "\n"
+ "$service_name$Proxy::$service_name$Proxy(\n"
+ " std::shared_ptr<::kudu::rpc::Messenger> messenger,\n"
+ " const ::kudu::Sockaddr& remote,\n"
+ " std::string hostname)\n"
+ " : Proxy(std::move(messenger),\n"
+ " remote,\n"
+ " std::move(hostname),\n"
+ " \"$full_service_name$\") {\n"
+ "}\n"
+ "\n"
+ "$service_name$Proxy::~$service_name$Proxy() {\n"
+ "}\n");
for (int method_idx = 0; method_idx < service->method_count();
++method_idx) {
const MethodDescriptor* method = service->method(method_idx);
subs->PushMethod(method);
Print(printer, *subs,
- "::kudu::Status $service_name$Proxy::$rpc_name$(const $request$ &req,
$response$ *resp,\n"
- " ::kudu::rpc::RpcController
*controller) {\n"
- " return SyncRequest(\"$rpc_name$\", req, resp, controller);\n"
- "}\n"
- "\n"
- "void $service_name$Proxy::$rpc_name$Async(const $request$ &req,\n"
- " $response$ *resp, ::kudu::rpc::RpcController
*controller,\n"
- " const ::kudu::rpc::ResponseCallback &callback)
{\n"
- " AsyncRequest(\"$rpc_name$\", req, resp, controller, callback);\n"
- "}\n"
- "\n");
+ "\n"
+ "::kudu::Status $service_name$Proxy::$rpc_name$(\n"
+ " const $request$& req,\n"
+ " $response$* resp,\n"
+ " ::kudu::rpc::RpcController* controller) {\n"
+ " return SyncRequest(\"$rpc_name$\", req, resp, controller);\n"
+ "}\n"
+ "\n"
+ "void $service_name$Proxy::$rpc_name$Async(\n"
+ " const $request$& req,\n"
+ " $response$* resp,\n"
+ " ::kudu::rpc::RpcController* controller,\n"
+ " const ::kudu::rpc::ResponseCallback& callback) {\n"
+ " AsyncRequest(\"$rpc_name$\", req, resp, controller,
callback);\n"
+ "}\n");
subs->Pop(); // method
}
subs->Pop(); // service
}
Print(printer, *subs,
- "\n"
- "$close_namespace$"
- "\n");
+ "\n"
+ "$close_namespace$");
}
};
+
} // namespace rpc
} // namespace kudu
diff --git a/src/kudu/rpc/rpc-test-base.h b/src/kudu/rpc/rpc-test-base.h
index 3bfb391..484f3f6 100644
--- a/src/kudu/rpc/rpc-test-base.h
+++ b/src/kudu/rpc/rpc-test-base.h
@@ -56,9 +56,6 @@
DECLARE_bool(rpc_encrypt_loopback_connections);
-namespace kudu {
-namespace rpc {
-
using kudu::rpc_test::AddRequestPB;
using kudu::rpc_test::AddResponsePB;
using kudu::rpc_test::CalculatorError;
@@ -86,11 +83,14 @@ using kudu::rpc_test::WhoAmIResponsePB;
using kudu::rpc_test_diff_package::ReqDiffPackagePB;
using kudu::rpc_test_diff_package::RespDiffPackagePB;
+namespace kudu {
+namespace rpc {
+
// Implementation of CalculatorService which just implements the generic
// RPC handler (no generated code).
class GenericCalculatorService : public ServiceIf {
public:
- static const char *kFullServiceName;
+ static const std::string kFullServiceName;
static const char *kAddMethodName;
static const char *kSleepMethodName;
static const char *kSleepWithSidecarMethodName;
@@ -127,8 +127,8 @@ class GenericCalculatorService : public ServiceIf {
}
}
- std::string service_name() const override { return kFullServiceName; }
- static std::string static_service_name() { return kFullServiceName; }
+ const std::string& service_name() const override { return kFullServiceName; }
+ static const std::string& static_service_name() { return kFullServiceName; }
private:
void DoAdd(InboundCall *incoming) {
@@ -399,7 +399,7 @@ class CalculatorService : public CalculatorServiceIf {
};
-const char *GenericCalculatorService::kFullServiceName =
"kudu.rpc.GenericCalculatorService";
+const std::string GenericCalculatorService::kFullServiceName =
"kudu.rpc.GenericCalculatorService";
const char *GenericCalculatorService::kAddMethodName = "Add";
const char *GenericCalculatorService::kSleepMethodName = "Sleep";
const char *GenericCalculatorService::kSleepWithSidecarMethodName =
"SleepWithSidecar";
diff --git a/src/kudu/rpc/service_if.cc b/src/kudu/rpc/service_if.cc
index 792ac95..63c5aec 100644
--- a/src/kudu/rpc/service_if.cc
+++ b/src/kudu/rpc/service_if.cc
@@ -48,7 +48,6 @@ TAG_FLAG(enable_exactly_once, hidden);
using google::protobuf::Message;
using std::string;
-using std::unique_ptr;
using strings::Substitute;
namespace kudu {
@@ -64,7 +63,7 @@ bool ServiceIf::SupportsFeature(uint32_t feature) const {
return false;
}
-bool ServiceIf::ParseParam(InboundCall *call, google::protobuf::Message
*message) {
+bool ServiceIf::ParseParam(InboundCall* call, Message* message) {
Slice param(call->serialized_request());
if (PREDICT_FALSE(!message->ParseFromArray(param.data(), param.size()))) {
string err = Substitute("invalid parameter for call $0: missing fields:
$1",
@@ -78,10 +77,10 @@ bool ServiceIf::ParseParam(InboundCall *call,
google::protobuf::Message *message
return true;
}
-void ServiceIf::RespondBadMethod(InboundCall *call) {
- Sockaddr local_addr, remote_addr;
-
+void ServiceIf::RespondBadMethod(InboundCall* call) {
+ Sockaddr local_addr;
CHECK_OK(call->connection()->socket()->GetSocketAddress(&local_addr));
+ Sockaddr remote_addr;
CHECK_OK(call->connection()->socket()->GetPeerAddress(&remote_addr));
string err = Substitute("Call on service $0 received at $1 from $2 with an "
"invalid method name: $3",
@@ -97,8 +96,7 @@ void ServiceIf::RespondBadMethod(InboundCall *call) {
GeneratedServiceIf::~GeneratedServiceIf() {
}
-
-void GeneratedServiceIf::Handle(InboundCall *call) {
+void GeneratedServiceIf::Handle(InboundCall* call) {
const RpcMethodInfo* method_info = call->method_info();
if (!method_info) {
RespondBadMethod(call);
@@ -139,7 +137,6 @@ void GeneratedServiceIf::Handle(InboundCall *call) {
method_info->func(ctx->request_pb(), resp, ctx);
}
-
RpcMethodInfo* GeneratedServiceIf::LookupMethod(const RemoteMethod& method) {
DCHECK_EQ(method.service_name(), service_name());
const auto& it = methods_by_name_.find(method.method_name());
@@ -149,6 +146,9 @@ RpcMethodInfo* GeneratedServiceIf::LookupMethod(const
RemoteMethod& method) {
return it->second.get();
}
+GeneratedServiceIf::GeneratedServiceIf(const scoped_refptr<ResultTracker>&
tracker)
+ : result_tracker_(tracker) {
+}
} // namespace rpc
} // namespace kudu
diff --git a/src/kudu/rpc/service_if.h b/src/kudu/rpc/service_if.h
index 2fc5f09..71226db 100644
--- a/src/kudu/rpc/service_if.h
+++ b/src/kudu/rpc/service_if.h
@@ -69,9 +69,9 @@ struct RpcMethodInfo : public
RefCountedThreadSafe<RpcMethodInfo> {
class ServiceIf {
public:
virtual ~ServiceIf();
- virtual void Handle(InboundCall* incoming) = 0;
+ virtual void Handle(InboundCall* call) = 0;
virtual void Shutdown();
- virtual std::string service_name() const = 0;
+ virtual const std::string& service_name() const = 0;
// The service should return true if it supports the provided application
// specific feature flag.
@@ -96,8 +96,8 @@ class ServiceIf {
}
protected:
- bool ParseParam(InboundCall* call, google::protobuf::Message* message);
- void RespondBadMethod(InboundCall* call);
+ static bool ParseParam(InboundCall* call, google::protobuf::Message*
message);
+ static void RespondBadMethod(InboundCall* call);
};
@@ -110,23 +110,22 @@ class GeneratedServiceIf : public ServiceIf {
// it on the current thread.
//
// If no such method is found, responds with an error.
- void Handle(InboundCall* incoming) override;
+ void Handle(InboundCall* call) override;
RpcMethodInfo* LookupMethod(const RemoteMethod& method) override;
- // Returns the mapping from method names to method infos.
- typedef std::unordered_map<std::string, scoped_refptr<RpcMethodInfo>>
MethodInfoMap;
- const MethodInfoMap& methods_by_name() const { return methods_by_name_; }
-
protected:
+ explicit GeneratedServiceIf(const scoped_refptr<ResultTracker>& tracker);
+
+ // The result tracker for this service's methods.
+ const scoped_refptr<ResultTracker> result_tracker_;
+
// For each method, stores the relevant information about how to handle the
// call. Methods are inserted by the constructor of the generated subclass.
// After construction, this map is accessed by multiple threads and therefore
// must not be modified.
+ typedef std::unordered_map<std::string, scoped_refptr<RpcMethodInfo>>
MethodInfoMap;
MethodInfoMap methods_by_name_;
-
- // The result tracker for this service's methods.
- scoped_refptr<ResultTracker> result_tracker_;
};
} // namespace rpc
diff --git a/src/kudu/rpc/service_pool.cc b/src/kudu/rpc/service_pool.cc
index a510937..d1755aa 100644
--- a/src/kudu/rpc/service_pool.cc
+++ b/src/kudu/rpc/service_pool.cc
@@ -233,7 +233,7 @@ void ServicePool::RunThread() {
}
}
-const string ServicePool::service_name() const {
+const string& ServicePool::service_name() const {
return service_->service_name();
}
diff --git a/src/kudu/rpc/service_pool.h b/src/kudu/rpc/service_pool.h
index a6a3da4..986ffbf 100644
--- a/src/kudu/rpc/service_pool.h
+++ b/src/kudu/rpc/service_pool.h
@@ -86,7 +86,7 @@ class ServicePool : public RpcService {
return rpcs_queue_overflow_.get();
}
- const std::string service_name() const;
+ const std::string& service_name() const;
private:
void RunThread();
diff --git a/src/kudu/server/rpc_server.cc b/src/kudu/server/rpc_server.cc
index 337a185..7e476fe 100644
--- a/src/kudu/server/rpc_server.cc
+++ b/src/kudu/server/rpc_server.cc
@@ -159,7 +159,6 @@ Status RpcServer::Init(const shared_ptr<Messenger>&
messenger) {
Status RpcServer::RegisterService(unique_ptr<rpc::ServiceIf> service) {
CHECK(server_state_ == INITIALIZED ||
server_state_ == BOUND) << "bad state: " << server_state_;
- string service_name = service->service_name();
scoped_refptr<rpc::ServicePool> service_pool =
new rpc::ServicePool(std::move(service), messenger_->metric_entity(),
options_.service_queue_length);
@@ -170,8 +169,7 @@ Status
RpcServer::RegisterService(unique_ptr<rpc::ServiceIf> service) {
too_busy_hook_(service_pool_raw_ptr);
}
});
- RETURN_NOT_OK(messenger_->RegisterService(service_name, service_pool));
- return Status::OK();
+ return messenger_->RegisterService(service_pool->service_name(),
service_pool);
}
Status RpcServer::AddBindAddress(const Sockaddr& addr) {
