You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brpc.apache.org by ja...@apache.org on 2020/01/07 09:16:27 UTC
[incubator-brpc] branch master updated: fix redis related issues
This is an automated email from the ASF dual-hosted git repository.
jamesge pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-brpc.git
The following commit(s) were added to refs/heads/master by this push:
new 38e5f8b fix redis related issues
38e5f8b is described below
commit 38e5f8bc57d98d828fa2ba36b618df8a5975cd84
Author: jamesge <jg...@gmail.com>
AuthorDate: Tue Jan 7 17:13:27 2020 +0800
fix redis related issues
---
example/redis_c++/Makefile | 14 ++++++++++++--
example/redis_c++/redis_server.cpp | 8 ++++----
src/brpc/policy/redis_protocol.cpp | 13 +++++--------
src/brpc/redis.cpp | 2 +-
src/brpc/redis.h | 9 +++++----
src/brpc/server.cpp | 3 +++
src/brpc/server.h | 8 +++++---
test/brpc_redis_unittest.cpp | 30 ++++++++++++++----------------
8 files changed, 49 insertions(+), 38 deletions(-)
diff --git a/example/redis_c++/Makefile b/example/redis_c++/Makefile
index 27b56cd..2f28bde 100644
--- a/example/redis_c++/Makefile
+++ b/example/redis_c++/Makefile
@@ -28,9 +28,11 @@ SOPATHS=$(addprefix -Wl$(COMMA)-rpath$(COMMA), $(LIBS))
DYNAMIC_LINKINGS += -lreadline -lncurses
PRESS_SOURCES = redis_press.cpp
CLI_SOURCES = redis_cli.cpp
+SERVER_SOURCES = redis_server.cpp
PRESS_OBJS = $(addsuffix .o, $(basename $(PRESS_SOURCES)))
CLI_OBJS = $(addsuffix .o, $(basename $(CLI_SOURCES)))
+SERVER_OBJS = $(addsuffix .o, $(basename $(SERVER_SOURCES)))
ifeq ($(SYSTEM),Darwin)
ifneq ("$(LINK_SO)", "")
@@ -48,12 +50,12 @@ else ifeq ($(SYSTEM),Linux)
endif
.PHONY:all
-all: redis_press redis_cli
+all: redis_press redis_cli redis_server
.PHONY:clean
clean:
@echo "Cleaning"
- @rm -rf redis_press redis_cli $(PRESS_OBJS) $(CLI_OBJS)
+ @rm -rf redis_press redis_cli $(PRESS_OBJS) $(CLI_OBJS) $(SERVER_OBJS)
redis_press:$(PRESS_OBJS)
@echo "Linking $@"
@@ -71,6 +73,14 @@ else
@$(CXX) $(LIBPATHS) $(LINK_OPTIONS) -o $@
endif
+redis_server:$(SERVER_OBJS)
+ @echo "Linking $@"
+ifneq ("$(LINK_SO)", "")
+ @$(CXX) $(LIBPATHS) $(SOPATHS) $(LINK_OPTIONS_SO) -o $@
+else
+ @$(CXX) $(LIBPATHS) $(LINK_OPTIONS) -o $@
+endif
+
%.o:%.cpp
@echo "Compiling $@"
@$(CXX) -c $(HDRPATHS) $(CXXFLAGS) $< -o $@
diff --git a/example/redis_c++/redis_server.cpp b/example/redis_c++/redis_server.cpp
index 3f717f6..ce39de9 100644
--- a/example/redis_c++/redis_server.cpp
+++ b/example/redis_c++/redis_server.cpp
@@ -59,12 +59,12 @@ private:
class GetCommandHandler : public brpc::RedisCommandHandler {
public:
- GetCommandHandler(RedisServiceImpl* rsimpl)
+ explicit GetCommandHandler(RedisServiceImpl* rsimpl)
: _rsimpl(rsimpl) {}
brpc::RedisCommandHandler::Result Run(const std::vector<const char*>& args,
brpc::RedisReply* output,
- bool flush_batched) override {
+ bool /*flush_batched*/) override {
if (args.size() <= 1) {
output->SetError("ERR wrong number of arguments for 'get' command");
return brpc::RedisCommandHandler::OK;
@@ -85,12 +85,12 @@ private:
class SetCommandHandler : public brpc::RedisCommandHandler {
public:
- SetCommandHandler(RedisServiceImpl* rsimpl)
+ explicit SetCommandHandler(RedisServiceImpl* rsimpl)
: _rsimpl(rsimpl) {}
brpc::RedisCommandHandler::Result Run(const std::vector<const char*>& args,
brpc::RedisReply* output,
- bool flush_batched) override {
+ bool /*flush_batched*/) override {
if (args.size() <= 2) {
output->SetError("ERR wrong number of arguments for 'set' command");
return brpc::RedisCommandHandler::OK;
diff --git a/src/brpc/policy/redis_protocol.cpp b/src/brpc/policy/redis_protocol.cpp
index 9ef305d..64bd28c 100644
--- a/src/brpc/policy/redis_protocol.cpp
+++ b/src/brpc/policy/redis_protocol.cpp
@@ -56,16 +56,15 @@ struct InputResponse : public InputMessageBase {
// This class is as parsing_context in socket.
class RedisConnContext : public Destroyable {
public:
- RedisConnContext()
- : redis_service(NULL)
+ explicit RedisConnContext(const RedisService* rs)
+ : redis_service(rs)
, batched_size(0) {}
~RedisConnContext();
// @Destroyable
void Destroy() override;
- SocketId socket_id;
- RedisService* redis_service;
+ const RedisService* redis_service;
// If user starts a transaction, transaction_handler indicates the
// handler pointer that runs the transaction command.
std::unique_ptr<RedisCommandHandler> transaction_handler;
@@ -151,15 +150,13 @@ ParseResult ParseRedisMessage(butil::IOBuf* source, Socket* socket,
}
const Server* server = static_cast<const Server*>(arg);
if (server) {
- RedisService* rs = server->options().redis_service;
+ const RedisService* const rs = server->options().redis_service;
if (!rs) {
return MakeParseError(PARSE_ERROR_TRY_OTHERS);
}
RedisConnContext* ctx = static_cast<RedisConnContext*>(socket->parsing_context());
if (ctx == NULL) {
- ctx = new RedisConnContext;
- ctx->socket_id = socket->id();
- ctx->redis_service = rs;
+ ctx = new RedisConnContext(rs);
socket->reset_parsing_context(ctx);
}
std::vector<const char*> current_commands;
diff --git a/src/brpc/redis.cpp b/src/brpc/redis.cpp
index fd5c309..267b461 100644
--- a/src/brpc/redis.cpp
+++ b/src/brpc/redis.cpp
@@ -447,7 +447,7 @@ bool RedisService::AddCommandHandler(const std::string& name, RedisCommandHandle
return true;
}
-RedisCommandHandler* RedisService::FindCommandHandler(const std::string& name) {
+RedisCommandHandler* RedisService::FindCommandHandler(const std::string& name) const {
std::string lcname = StringToLowerASCII(name);
auto it = _command_map.find(lcname);
if (it != _command_map.end()) {
diff --git a/src/brpc/redis.h b/src/brpc/redis.h
index e9013ad..9454c42 100644
--- a/src/brpc/redis.h
+++ b/src/brpc/redis.h
@@ -214,19 +214,20 @@ std::ostream& operator<<(std::ostream& os, const RedisResponse&);
class RedisCommandHandler;
-// Implement this class and assign an instance to ServerOption.redis_service
-// to enable redis support.
+// Container of CommandHandlers.
+// Assign an instance to ServerOption.redis_service to enable redis support.
class RedisService {
public:
- typedef std::unordered_map<std::string, RedisCommandHandler*> CommandMap;
virtual ~RedisService() {}
// Call this function to register `handler` that can handle command `name`.
bool AddCommandHandler(const std::string& name, RedisCommandHandler* handler);
// This function should not be touched by user and used by brpc deverloper only.
- RedisCommandHandler* FindCommandHandler(const std::string& name);
+ RedisCommandHandler* FindCommandHandler(const std::string& name) const;
+
private:
+ typedef std::unordered_map<std::string, RedisCommandHandler*> CommandMap;
CommandMap _command_map;
};
diff --git a/src/brpc/server.cpp b/src/brpc/server.cpp
index b021a55..225445e 100644
--- a/src/brpc/server.cpp
+++ b/src/brpc/server.cpp
@@ -433,6 +433,9 @@ Server::~Server() {
delete _options.auth;
_options.auth = NULL;
}
+
+ delete _options.redis_service;
+ _options.redis_service = NULL;
}
int Server::AddBuiltinServices() {
diff --git a/src/brpc/server.h b/src/brpc/server.h
index 6dbc7c6..e7687ad 100644
--- a/src/brpc/server.h
+++ b/src/brpc/server.h
@@ -68,12 +68,12 @@ struct ServerOptions {
std::string pid_file;
// Process requests in format of nshead_t + blob.
- // Owned by Server and deleted in server's destructor
+ // Owned by Server and deleted in server's destructor.
// Default: NULL
NsheadService* nshead_service;
// Process requests in format of thrift_binary_head_t + blob.
- // Owned by Server and deleted in server's destructor
+ // Owned by Server and deleted in server's destructor.
// Default: NULL
ThriftService* thrift_service;
@@ -215,7 +215,8 @@ struct ServerOptions {
// including accesses to builtin services and pb services.
// The service must have a method named "default_method" and the request
// and response must have no fields.
- // This service is owned by server and deleted in server's destructor
+ //
+ // Owned by Server and deleted in server's destructor
google::protobuf::Service* http_master_service;
// If this field is on, contents on /health page is generated by calling
@@ -236,6 +237,7 @@ struct ServerOptions {
H2Settings h2_settings;
// For processing Redis connections. Read src/brpc/redis.h for details.
+ // Owned by Server and deleted in server's destructor.
// Default: NULL (disabled)
RedisService* redis_service;
diff --git a/test/brpc_redis_unittest.cpp b/test/brpc_redis_unittest.cpp
index f569edc..0152f63 100644
--- a/test/brpc_redis_unittest.cpp
+++ b/test/brpc_redis_unittest.cpp
@@ -850,8 +850,8 @@ public:
class SetCommandHandler : public brpc::RedisCommandHandler {
public:
- SetCommandHandler(bool batch_process = false)
- : rs(NULL)
+ SetCommandHandler(RedisServiceImpl* rs, bool batch_process = false)
+ : _rs(rs)
, _batch_process(batch_process) {}
brpc::RedisCommandHandler::Result Run(const std::vector<const char*>& args,
@@ -862,7 +862,7 @@ public:
return brpc::RedisCommandHandler::OK;
}
if (_batch_process) {
- return rs->OnBatched(args, output, flush_batched);
+ return _rs->OnBatched(args, output, flush_batched);
} else {
DoSet(args[1], args[2], output);
return brpc::RedisCommandHandler::OK;
@@ -874,15 +874,15 @@ public:
output->SetStatus("OK");
}
- RedisServiceImpl* rs;
private:
+ RedisServiceImpl* _rs;
bool _batch_process;
};
class GetCommandHandler : public brpc::RedisCommandHandler {
public:
- GetCommandHandler(bool batch_process = false)
- : rs(NULL)
+ GetCommandHandler(RedisServiceImpl* rs, bool batch_process = false)
+ : _rs(rs)
, _batch_process(batch_process) {}
brpc::RedisCommandHandler::Result Run(const std::vector<const char*>& args,
@@ -893,7 +893,7 @@ public:
return brpc::RedisCommandHandler::OK;
}
if (_batch_process) {
- return rs->OnBatched(args, output, flush_batched);
+ return _rs->OnBatched(args, output, flush_batched);
} else {
DoGet(args[1], output);
return brpc::RedisCommandHandler::OK;
@@ -909,8 +909,8 @@ public:
}
}
- RedisServiceImpl* rs;
private:
+ RedisServiceImpl* _rs;
bool _batch_process;
};
@@ -939,8 +939,8 @@ TEST_F(RedisTest, server_sanity) {
brpc::Server server;
brpc::ServerOptions server_options;
RedisServiceImpl* rsimpl = new RedisServiceImpl;
- GetCommandHandler *gh = new GetCommandHandler;
- SetCommandHandler *sh = new SetCommandHandler;
+ GetCommandHandler *gh = new GetCommandHandler(rsimpl);
+ SetCommandHandler *sh = new SetCommandHandler(rsimpl);
IncrCommandHandler *ih = new IncrCommandHandler;
rsimpl->AddCommandHandler("get", gh);
rsimpl->AddCommandHandler("set", sh);
@@ -1084,8 +1084,8 @@ TEST_F(RedisTest, server_command_continue) {
brpc::Server server;
brpc::ServerOptions server_options;
RedisServiceImpl* rsimpl = new RedisServiceImpl;
- rsimpl->AddCommandHandler("get", new GetCommandHandler);
- rsimpl->AddCommandHandler("set", new SetCommandHandler);
+ rsimpl->AddCommandHandler("get", new GetCommandHandler(rsimpl));
+ rsimpl->AddCommandHandler("set", new SetCommandHandler(rsimpl));
rsimpl->AddCommandHandler("incr", new IncrCommandHandler);
rsimpl->AddCommandHandler("multi", new MultiCommandHandler);
server_options.redis_service = rsimpl;
@@ -1159,10 +1159,8 @@ TEST_F(RedisTest, server_handle_pipeline) {
brpc::Server server;
brpc::ServerOptions server_options;
RedisServiceImpl* rsimpl = new RedisServiceImpl;
- GetCommandHandler* getch = new GetCommandHandler(true);
- SetCommandHandler* setch = new SetCommandHandler(true);
- getch->rs = rsimpl;
- setch->rs = rsimpl;
+ GetCommandHandler* getch = new GetCommandHandler(rsimpl, true);
+ SetCommandHandler* setch = new SetCommandHandler(rsimpl, true);
rsimpl->AddCommandHandler("get", getch);
rsimpl->AddCommandHandler("set", setch);
rsimpl->AddCommandHandler("multi", new MultiCommandHandler);
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org