redis_server_protocol: refine code

src/brpc/policy/redis_protocol.cpp

@@ -67,71 +67,72 @@ public:
 
     SocketId socket_id;
     RedisService* redis_service;
-    // If user starts a transaction, handler_continue indicates the
+    // If user starts a transaction, transaction_handler indicates the
     // handler pointer that runs the transaction command.
-    std::unique_ptr<RedisCommandHandler> handler_continue;
+    std::unique_ptr<RedisCommandHandler> transaction_handler;
     // >0 if command handler is run in batched mode.
     int batched_size;
 
     RedisCommandParser parser;
 };
 
-static std::string ToLowercase(const std::string& command) {
-    std::string res;
-    res.resize(command.size());
-    std::transform(command.begin(), command.end(), res.begin(),
-        [](unsigned char c){ return std::tolower(c); });
-    return res;
-}
-
 int ConsumeCommand(RedisConnContext* ctx,
                    const std::unique_ptr<const char*[]>& commands,
-                   int len, butil::Arena* arena,
+                   int command_len, butil::Arena* arena,
                    bool is_last,
                    butil::IOBuf* sendbuf) {
     RedisReply output(arena);
     RedisCommandHandler::Result result = RedisCommandHandler::OK;
-    if (ctx->handler_continue) {
-        result = ctx->handler_continue->Run(len, commands.get(), &output, is_last);
+    if (ctx->transaction_handler) {
+        result = ctx->transaction_handler->Run(
+                command_len, commands.get(), &output, is_last);
         if (result == RedisCommandHandler::OK) {
-            ctx->handler_continue.reset(NULL);
+            ctx->transaction_handler.reset(NULL);
         } else if (result == RedisCommandHandler::BATCHED) {
-            LOG(ERROR) << "BATCHED should not be returned in redis transaction process.";
+            LOG(ERROR) << "BATCHED should not be returned by a transaction handler.";
             return -1;
         }
     } else {
-        std::string lcname = ToLowercase(commands[0]);
-        RedisCommandHandler* ch = ctx->redis_service->FindCommandHandler(lcname);
+        RedisCommandHandler* ch = ctx->redis_service->FindCommandHandler(commands[0]);
         if (!ch) {
             char buf[64];
-            snprintf(buf, sizeof(buf), "ERR unknown command `%s`", lcname.c_str());
+            snprintf(buf, sizeof(buf), "ERR unknown command `%s`", commands[0]);
             output.SetError(buf);
         } else {
-            result = ch->Run(len, commands.get(), &output, is_last);
+            result = ch->Run(command_len, commands.get(), &output, is_last);
             if (result == RedisCommandHandler::CONTINUE) {
                 if (ctx->batched_size) {
                     LOG(ERROR) << "CONTINUE should not be returned in redis batched process.";
                     return -1;
                 }
-                ctx->handler_continue.reset(ch->NewTransactionHandler());
+                ctx->transaction_handler.reset(ch->NewTransactionHandler());
             } else if (result == RedisCommandHandler::BATCHED) {
                 ctx->batched_size++;
             }
         }
     }
-    if (result == RedisCommandHandler::OK && ctx->batched_size) {
-        if ((int)output.size() != (ctx->batched_size + 1)) {
-            LOG(ERROR) << "reply array size can't be matched with batched size, "
-                << " expected=" << ctx->batched_size + 1 << " actual=" << output.size();
-            return -1;
-        }
-        for (int i = 0; i < (int)output.size(); ++i) {
-            output[i].SerializeTo(sendbuf);
+    if (result == RedisCommandHandler::OK) {
+        if (ctx->batched_size) {
+            if ((int)output.size() != (ctx->batched_size + 1)) {
+                LOG(ERROR) << "reply array size can't be matched with batched size, "
+                    << " expected=" << ctx->batched_size + 1 << " actual=" << output.size();
+                return -1;
+            }
+            for (int i = 0; i < (int)output.size(); ++i) {
+                output[i].SerializeTo(sendbuf);
+            }
+            ctx->batched_size = 0;
+        } else {
+            output.SerializeTo(sendbuf);
         }
-        ctx->batched_size = 0;
-    } else if (result != RedisCommandHandler::BATCHED) {
+    } else if (result == RedisCommandHandler::CONTINUE) {
         output.SerializeTo(sendbuf);
-    } // else result == RedisCommandHandler::BATCHED, do not serialize to buf
+    } else if (result == RedisCommandHandler::BATCHED) {
+        // just do nothing and wait handler to return OK.
+    } else {
+        LOG(ERROR) << "unknown status=" << result;
+        return -1;
+    }
     return 0;
 }
 
@@ -180,16 +181,15 @@ ParseResult ParseRedisMessage(butil::IOBuf* source, Socket* socket,
             if (err != PARSE_OK) {
                 break;
             }
-            // safe to read first element.
-            // current_commands and next_commands both have at least one element(NULL).
-            bool is_last = (strcasecmp(current_commands[0], next_commands[0]) != 0);
-            if (ConsumeCommand(ctx, current_commands, current_len, &arena, is_last, &sendbuf) != 0) {
+            if (ConsumeCommand(ctx, current_commands, current_len, &arena,
+                               false, &sendbuf) != 0) {
                 return MakeParseError(PARSE_ERROR_ABSOLUTELY_WRONG);
             }
             current_commands.swap(next_commands);
             current_len = next_len;
         }
-        if (ConsumeCommand(ctx, current_commands, current_len, &arena, true, &sendbuf) != 0) {
+        if (ConsumeCommand(ctx, current_commands, current_len, &arena,
+                           true /* must be last message */, &sendbuf) != 0) {
             return MakeParseError(PARSE_ERROR_ABSOLUTELY_WRONG);
         }
         CHECK(!sendbuf.empty());

src/brpc/redis.cpp

@@ -17,9 +17,10 @@
 
 // Authors: Ge,Jun (gejun@baidu.com)
 
-#include <google/protobuf/reflection_ops.h>                 // ReflectionOps::Merge
+#include <google/protobuf/reflection_ops.h>     // ReflectionOps::Merge
 #include <gflags/gflags.h>
 #include "butil/status.h"
+#include "butil/strings/string_util.h"          // StringToLowerASCII
 #include "brpc/redis.h"
 #include "brpc/redis_command.h"
 
@@ -437,11 +438,9 @@ std::ostream& operator<<(std::ostream& os, const RedisResponse& response) {
 }
 
 bool RedisService::AddCommandHandler(const std::string& name, RedisCommandHandler* handler) {
-    std::string lcname;
-    lcname.resize(name.size());
-    std::transform(name.begin(), name.end(), lcname.begin(),
-            [](unsigned char c){ return std::tolower(c); });
-    if (_command_map.count(lcname)) {
+    std::string lcname = StringToLowerASCII(name);
+    auto it = _command_map.find(lcname);
+    if (it != _command_map.end()) {
         LOG(ERROR) << "redis command name=" << name << " exist";
         return false;
     }
@@ -450,7 +449,8 @@ bool RedisService::AddCommandHandler(const std::string& name, RedisCommandHandle
 }
  
 RedisCommandHandler* RedisService::FindCommandHandler(const std::string& name) {
-    auto it = _command_map.find(name);
+    std::string lcname = StringToLowerASCII(name);
+    auto it = _command_map.find(lcname);
     if (it != _command_map.end()) {
         return it->second;
     }

src/brpc/redis_command.h

@@ -20,6 +20,7 @@
 #ifndef BRPC_REDIS_COMMAND_H
 #define BRPC_REDIS_COMMAND_H
 
+#include <memory>           // std::unique_ptr
 #include <vector>
 #include "butil/iobuf.h"
 #include "butil/status.h"

src/brpc/redis_reply.cpp

@@ -413,34 +413,39 @@ void RedisReply::CopyFromDifferentArena(const RedisReply& other,
     }
 }
 
-bool RedisReply::SetArray(int size) {
-    if (!_arena || _type != REDIS_REPLY_NIL) {
-        return false;
+void RedisReply::SetArray(int size) {
+    if (!_arena) {
+        return;
+    }
+    if (_type != REDIS_REPLY_NIL) {
+        Reset();
     }
     _type = REDIS_REPLY_ARRAY;
     if (size < 0) {
         LOG(ERROR) << "negative size=" << size << " when calling SetArray";
-        return false;
+        return;
     } else if (size == 0) {
         _length = 0;
-        return true;
+        return;
     }
     RedisReply* subs = (RedisReply*)_arena->allocate(sizeof(RedisReply) * size);
     if (!subs) {
         LOG(FATAL) << "Fail to allocate RedisReply[" << size << "]";
-        return false;
+        return;
     }
     for (int i = 0; i < size; ++i) {
         new (&subs[i]) RedisReply(_arena);
     }
     _length = size;
     _data.array.replies = subs;
-    return true;
 }
 
-bool RedisReply::SetBasicString(const std::string& str, RedisReplyType type) {
-    if (!_arena || _type != REDIS_REPLY_NIL) {
-        return false;
+void RedisReply::SetStringImpl(const std::string& str, RedisReplyType type) {
+    if (!_arena) {
+        return;
+    }
+    if (_type != REDIS_REPLY_NIL) {
+        Reset();
     }
     const size_t size = str.size();
     if (size < sizeof(_data.short_str)) {
@@ -450,7 +455,7 @@ bool RedisReply::SetBasicString(const std::string& str, RedisReplyType type) {
         char* d = (char*)_arena->allocate((size/8 + 1) * 8);
         if (!d) {
             LOG(FATAL) << "Fail to allocate string[" << size << "]";
-            return false;
+            return;
         }
         memcpy(d, str.c_str(), size);
         d[size] = '\0';
@@ -458,7 +463,6 @@ bool RedisReply::SetBasicString(const std::string& str, RedisReplyType type) {
     }
     _type = type;
     _length = size;
-    return true;
 }
 
 } // namespace brpc

src/brpc/redis_reply.h

@@ -59,35 +59,28 @@ public:
     bool is_string() const;  // True if the reply is a string.
     bool is_array() const;   // True if the reply is an array.
 
-    // Set the reply to the null string. Return True if it is set
-    // successfully. If the reply has already been set, return false.
-    bool SetNullString();
+    // Set the reply to the null string.
+    void SetNullString();
 
-    // Set the reply to the null array. Return True if it is set
-    // successfully. If the reply has already been set, return false.
-    bool SetNullArray();
+    // Set the reply to the null array.
+    void SetNullArray();
 
     // Set the reply to the array with `size' elements. After calling
     // SetArray, use operator[] to visit sub replies and set their
-    // value. Return True if it is set successfully. If the reply has
-    // already been set, return false.
-    bool SetArray(int size);
+    // value.
+    void SetArray(int size);
 
-    // Set the reply to status message `str'. Return True if it is set
-    // successfully. If the reply has already been set, return false.
-    bool SetStatus(const std::string& str);
+    // Set the reply to status message `str'.
+    void SetStatus(const std::string& str);
 
-    // Set the reply to error message `str'. Return True if it is set
-    // successfully. If the reply has already been set, return false.
-    bool SetError(const std::string& str);
+    // Set the reply to error message `str'.
+    void SetError(const std::string& str);
 
-    // Set the reply to integer `value'. Return True if it is set
-    // successfully. If the reply has already been set, return false.
-    bool SetInteger(int64_t value);
+    // Set the reply to integer `value'.
+    void SetInteger(int64_t value);
 
-    // Set the reply to string `str'. Return True if it is set
-    // successfully. If the reply has already been set, return false.
-    bool SetString(const std::string& str);
+    // Set the reply to string `str'.
+    void SetString(const std::string& str);
 
     // Convert the reply into a signed 64-bit integer(according to
     // http://redis.io/topics/protocol). If the reply is not an integer,
@@ -156,7 +149,8 @@ private:
     // by calling CopyFrom[Different|Same]Arena.
     DISALLOW_COPY_AND_ASSIGN(RedisReply);
 
-    bool SetBasicString(const std::string& str, RedisReplyType type);
+    void SetStringImpl(const std::string& str, RedisReplyType type);
+    void Reset();
     
     RedisReplyType _type;
     uint32_t _length;  // length of short_str/long_str, count of replies
@@ -180,17 +174,21 @@ inline std::ostream& operator<<(std::ostream& os, const RedisReply& r) {
     return os;
 }
 
-inline RedisReply::RedisReply(butil::Arena* arena)
-    : RedisReply() {
+inline void RedisReply::Reset() {
+    _type = REDIS_REPLY_NIL;
+    _length = 0;
+    _data.array.last_index = -1;
+    _data.array.replies = NULL;
+}
+
+inline RedisReply::RedisReply(butil::Arena* arena) {
+    Reset();
     _arena = arena;
 }
 
-inline RedisReply::RedisReply()
-    : _type(REDIS_REPLY_NIL)
-    , _length(0)
-    , _arena(NULL) {
-    _data.array.last_index = -1;
-    _data.array.replies = NULL;
+inline RedisReply::RedisReply() {
+    Reset();
+    _arena = NULL;
 }
 
 inline bool RedisReply::is_nil() const {
@@ -213,44 +211,41 @@ inline int64_t RedisReply::integer() const {
     return 0;
 }
 
-inline bool RedisReply::SetNullArray() {
+inline void RedisReply::SetNullArray() {
     if (_type != REDIS_REPLY_NIL) {
-        return false;
+        Reset();
     }
     _type = REDIS_REPLY_ARRAY;
     _length = npos;
-    return true;
 }
 
-inline bool RedisReply::SetNullString() {
+inline void RedisReply::SetNullString() {
     if (_type != REDIS_REPLY_NIL) {
-        return false;
+        Reset();
     }
     _type = REDIS_REPLY_STRING;
     _length = npos;
-    return true;
 }
 
-inline bool RedisReply::SetStatus(const std::string& str) {
-    return SetBasicString(str, REDIS_REPLY_STATUS);
+inline void RedisReply::SetStatus(const std::string& str) {
+    return SetStringImpl(str, REDIS_REPLY_STATUS);
 }
 
-inline bool RedisReply::SetError(const std::string& str) {
-    return SetBasicString(str, REDIS_REPLY_ERROR);
+inline void RedisReply::SetError(const std::string& str) {
+    return SetStringImpl(str, REDIS_REPLY_ERROR);
 }
 
-inline bool RedisReply::SetInteger(int64_t value) {
+inline void RedisReply::SetInteger(int64_t value) {
     if (_type != REDIS_REPLY_NIL) {
-        return false;
+        Reset();
     }
     _type = REDIS_REPLY_INTEGER;
     _length = 0;
     _data.integer = value;
-    return true;
 }
 
-inline bool RedisReply::SetString(const std::string& str) {
-    return SetBasicString(str, REDIS_REPLY_STRING);
+inline void RedisReply::SetString(const std::string& str) {
+    return SetStringImpl(str, REDIS_REPLY_STRING);
 }
 
 inline const char* RedisReply::c_str() const {