Introduce fmt dependency to simplify string formatting

CMakeLists.txt

@@ -116,6 +116,7 @@ include(cmake/zlib.cmake)
 include(cmake/zstd.cmake)
 include(cmake/rocksdb.cmake)
 include(cmake/libevent.cmake)
+include(cmake/fmt.cmake)
 
 if (USE_LUAJIT)
 include(cmake/luajit.cmake)
@@ -132,6 +133,7 @@ list(APPEND EXTERNAL_LIBS event_with_headers)
 list(APPEND EXTERNAL_LIBS lz4)
 list(APPEND EXTERNAL_LIBS zstd)
 list(APPEND EXTERNAL_LIBS zlib_with_headers)
+list(APPEND EXTERNAL_LIBS fmt)
 if (USE_LUAJIT)
 list(APPEND EXTERNAL_LIBS luajit)
 else()

NOTICE

@@ -189,6 +189,39 @@ zlib is a massively spiffy yet delicately unobtrusive compression library. It is
        misrepresented as being the original software.
     3. This notice may not be removed or altered from any source distribution.
 
+For the library fmt(https://github.com/fmtlib/fmt):
+
+{fmt} is an open-source formatting library providing a fast and safe alternative to C stdio and C++ iostreams.
+It is under the MIT license with an optional exception:
+
+    Copyright (c) 2012 - present, Victor Zverovich
+
+    Permission is hereby granted, free of charge, to any person obtaining
+    a copy of this software and associated documentation files (the
+    "Software"), to deal in the Software without restriction, including
+    without limitation the rights to use, copy, modify, merge, publish,
+    distribute, sublicense, and/or sell copies of the Software, and to
+    permit persons to whom the Software is furnished to do so, subject to
+    the following conditions:
+
+    The above copyright notice and this permission notice shall be
+    included in all copies or substantial portions of the Software.
+
+    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+    EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+    MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+    NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
+    LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
+    OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+    WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+    --- Optional exception to the license ---
+
+    As an exception, if, as a result of your compiling your source code, portions
+    of this Software are embedded into a machine-executable object form of such
+    source code, you may redistribute such embedded portions in such object form
+    without including the above copyright and permission notices.
+
 For src/types/geohash.*, src/common/rand.*, src/common/sha1.*, src/storage/scripting.* and
 some functions in src/common/util.cc are modified from Redis.
 

cmake/fmt.cmake

@@ -0,0 +1,27 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+include_guard()
+
+include(cmake/utils.cmake)
+
+FetchContent_DeclareGitHubWithMirror(fmt
+  fmtlib/fmt 9.1.0
+  MD5=e6754011ff56bfc37631fcc90961e377
+)
+
+FetchContent_MakeAvailableWithArgs(fmt)

src/cluster/cluster.cc

@@ -26,6 +26,7 @@
 #include <memory>
 
 #include "commands/redis_cmd.h"
+#include "fmt/format.h"
 #include "parse_util.h"
 #include "replication.h"
 #include "server/server.h"
@@ -313,7 +314,7 @@ Status Cluster::ImportSlot(Redis::Connection *conn, int slot, int state) {
   switch (state) {
     case kImportStart:
       if (!svr_->slot_import_->Start(conn->GetFD(), slot)) {
-        return Status(Status::NotOK, "Can't start importing slot " + std::to_string(slot));
+        return {Status::NotOK, fmt::format("Can't start importing slot {}", slot)};
       }
       // Set link importing
       conn->SetImporting();
@@ -330,20 +331,20 @@ Status Cluster::ImportSlot(Redis::Connection *conn, int slot, int state) {
       if (!svr_->slot_import_->Success(slot)) {
         LOG(ERROR) << "[import] Failed to set slot importing success, maybe slot is wrong"
                    << ", received slot: " << slot << ", current slot: " << svr_->slot_import_->GetSlot();
-        return Status(Status::NotOK, "Failed to set slot " + std::to_string(slot) + " importing success");
+        return {Status::NotOK, fmt::format("Failed to set slot {} importing success", slot)};
       }
       LOG(INFO) << "[import] Succeed to import slot " << slot;
       break;
     case kImportFailed:
       if (!svr_->slot_import_->Fail(slot)) {
         LOG(ERROR) << "[import] Failed to set slot importing error, maybe slot is wrong"
                    << ", received slot: " << slot << ", current slot: " << svr_->slot_import_->GetSlot();
-        return Status(Status::NotOK, "Failed to set slot " + std::to_string(slot) + " importing error");
+        return {Status::NotOK, fmt::format("Failed to set slot {} importing error", slot)};
       }
       LOG(INFO) << "[import] Failed to import slot " << slot;
       break;
     default:
-      return Status(Status::NotOK, errInvalidImportState);
+      return {Status::NotOK, errInvalidImportState};
   }
   return Status::OK();
 }
@@ -469,9 +470,9 @@ std::string Cluster::GenNodesDescription() {
     // Generate slots info when occur different node with start or end of slot
     if (i == kClusterSlots || n != slots_nodes_[i]) {
       if (start == i - 1) {
-        n->slots_info_ += std::to_string(start) + " ";
+        n->slots_info_ += fmt::format("{} ", start);
       } else {
-        n->slots_info_ += std::to_string(start) + "-" + std::to_string(i - 1) + " ";
+        n->slots_info_ += fmt::format("{}-{} ", start, i - 1);
       }
       if (i == kClusterSlots) break;
       n = slots_nodes_[i];
@@ -486,8 +487,7 @@ std::string Cluster::GenNodesDescription() {
     std::string node_str;
     // ID, host, port
     node_str.append(n->id_ + " ");
-    node_str.append(n->host_ + ":" + std::to_string(n->port_) + "@" + std::to_string(n->port_ + kClusterPortIncr) +
-                    " ");
+    node_str.append(fmt::format("{}:{}@{} ", n->host_, n->port_, n->port_ + kClusterPortIncr));
 
     // Flags
     if (n->id_ == myid_) node_str.append("myself,");
@@ -499,8 +499,7 @@ std::string Cluster::GenNodesDescription() {
 
     // Ping sent, pong received, config epoch, link status
     auto now = Util::GetTimeStampMS();
-    node_str.append(std::to_string(now - 1) + " " + std::to_string(now) + " " + std::to_string(version_) + " " +
-                    "connected");
+    node_str.append(fmt::format("{} {} {} connected", now - 1, now, version_));
 
     // Slots
     if (n->slots_info_.size() > 0) n->slots_info_.pop_back();  // Trim space
@@ -642,24 +641,24 @@ Status Cluster::CanExecByMySelf(const Redis::CommandAttributes *attributes, cons
     int cur_slot = GetSlotNumFromKey(cmd_tokens[i]);
     if (slot == -1) slot = cur_slot;
     if (slot != cur_slot) {
-      return Status(Status::RedisExecErr, "CROSSSLOT Attempted to access keys that don't hash to the same slot");
+      return {Status::RedisExecErr, "CROSSSLOT Attempted to access keys that don't hash to the same slot"};
     }
   }
   if (slot == -1) return Status::OK();
 
   if (slots_nodes_[slot] == nullptr) {
-    return Status(Status::ClusterDown, "CLUSTERDOWN Hash slot not served");
+    return {Status::ClusterDown, "CLUSTERDOWN Hash slot not served"};
   } else if (myself_ && myself_ == slots_nodes_[slot]) {
     // We use central controller to manage the topology of the cluster.
     // Server can't change the topology directly, so we record the migrated slots
     // to move the requests of the migrated slots to the destination node.
     if (migrated_slots_.count(slot)) {  // I'm not serving the migrated slot
-      return Status(Status::RedisExecErr, "MOVED " + std::to_string(slot) + " " + migrated_slots_[slot]);
+      return {Status::RedisExecErr, fmt::format("MOVED {} {}", slot, migrated_slots_[slot])};
     }
     // To keep data consistency, slot will be forbidden write while sending the last incremental data.
     // During this phase, the requests of the migrating slot has to be rejected.
     if (attributes->is_write() && IsWriteForbiddenSlot(slot)) {
-      return Status(Status::RedisExecErr, "Can't write to slot being migrated which is in write forbidden phase");
+      return {Status::RedisExecErr, "Can't write to slot being migrated which is in write forbidden phase"};
     }
     return Status::OK();  // I'm serving this slot
   } else if (myself_ && myself_->importing_slot_ == slot && conn->IsImporting()) {
@@ -677,7 +676,7 @@ Status Cluster::CanExecByMySelf(const Redis::CommandAttributes *attributes, cons
              nodes_.find(myself_->master_id_) != nodes_.end() && nodes_[myself_->master_id_] == slots_nodes_[slot]) {
     return Status::OK();  // My mater is serving this slot
   } else {
-    std::string ip_port = slots_nodes_[slot]->host_ + ":" + std::to_string(slots_nodes_[slot]->port_);
-    return Status(Status::RedisExecErr, "MOVED " + std::to_string(slot) + " " + ip_port);
+    return {Status::RedisExecErr,
+            fmt::format("MOVED {} {}:{}", slot, slots_nodes_[slot]->host_, slots_nodes_[slot]->port_)};
   }
 }

src/cluster/replication.cc

@@ -34,6 +34,7 @@
 
 #include "event_util.h"
 #include "fd_util.h"
+#include "fmt/format.h"
 #include "rocksdb_crc32c.h"
 #include "server/redis_reply.h"
 #include "server/server.h"
@@ -797,13 +798,13 @@ Status ReplicationThread::fetchFile(int sock_fd, evbuffer *evbuf, const std::str
     UniqueEvbufReadln line(evbuf, EVBUFFER_EOL_CRLF_STRICT);
     if (!line) {
       if (evbuffer_read(evbuf, sock_fd, -1) <= 0) {
-        return Status(Status::NotOK, std::string("read size: ") + strerror(errno));
+        return {Status::NotOK, fmt::format("read size: {}", strerror(errno))};
       }
       continue;
     }
     if (line[0] == '-') {
       std::string msg(line.get());
-      return Status(Status::NotOK, msg);
+      return {Status::NotOK, msg};
     }
     file_size = line.length > 0 ? std::strtoull(line.get(), nullptr, 10) : 0;
     break;
@@ -812,7 +813,7 @@ Status ReplicationThread::fetchFile(int sock_fd, evbuffer *evbuf, const std::str
   // Write to tmp file
   auto tmp_file = Engine::Storage::ReplDataManager::NewTmpFile(storage_, dir, file);
   if (!tmp_file) {
-    return Status(Status::NotOK, "unable to create tmp file");
+    return {Status::NotOK, "unable to create tmp file"};
   }
 
   size_t remain = file_size;
@@ -823,22 +824,20 @@ Status ReplicationThread::fetchFile(int sock_fd, evbuffer *evbuf, const std::str
       auto data_len = evbuffer_remove(evbuf, data, remain > 16 * 1024 ? 16 * 1024 : remain);
       if (data_len == 0) continue;
       if (data_len < 0) {
-        return Status(Status::NotOK, "read sst file data error");
+        return {Status::NotOK, "read sst file data error"};
       }
       tmp_file->Append(rocksdb::Slice(data, data_len));
       tmp_crc = rocksdb::crc32c::Extend(tmp_crc, data, data_len);
       remain -= data_len;
     } else {
       if (evbuffer_read(evbuf, sock_fd, -1) <= 0) {
-        return Status(Status::NotOK, std::string("read sst file: ") + strerror(errno));
+        return {Status::NotOK, fmt::format("read sst file: {}", strerror(errno))};
       }
     }
   }
   // Verify file crc checksum if crc is not 0
   if (crc && crc != tmp_crc) {
-    char err_buf[64];
-    snprintf(err_buf, sizeof(err_buf), "CRC mismatched, %u was expected but got %u", crc, tmp_crc);
-    return Status(Status::NotOK, err_buf);
+    return {Status::NotOK, fmt::format("CRC mismatched, {} was expected but got {}", crc, tmp_crc)};
   }
   // File is OK, rename to formal name
   auto s = Engine::Storage::ReplDataManager::SwapTmpFile(storage_, dir, file);

src/cluster/slot_import.cc

@@ -153,5 +153,5 @@ void SlotImport::GetImportInfo(std::string *info) {
       break;
   }
 
-  *info = "importing_slot: " + std::to_string(import_slot_) + "\r\n" + "import_state: " + import_stat + "\r\n";
+  *info = fmt::format("importing_slot: {}\r\nimport_state: {}\r\n", import_slot_, import_stat);
 }

src/cluster/slot_migrate.cc

@@ -24,6 +24,7 @@
 #include <utility>
 
 #include "event_util.h"
+#include "fmt/format.h"
 #include "storage/batch_extractor.h"
 
 static std::map<RedisType, std::string> type_to_cmd = {
@@ -481,7 +482,7 @@ bool SlotMigrate::CheckResponseWithCounts(int sock_fd, int total) {
   while (true) {
     // Read response data from socket buffer to the event buffer
     if (evbuffer_read(evbuf.get(), sock_fd, -1) <= 0) {
-      LOG(ERROR) << "[migrate] Failed to read response, Err: " + std::string(strerror(errno));
+      LOG(ERROR) << "[migrate] Failed to read response, Err: " << strerror(errno);
       return false;
     }
 
@@ -499,8 +500,7 @@ bool SlotMigrate::CheckResponseWithCounts(int sock_fd, int total) {
           }
 
           if (line[0] == '-') {
-            LOG(ERROR) << "[migrate] Got invalid response: " + std::string(line.get())
-                       << ", line length: " << line.length;
+            LOG(ERROR) << "[migrate] Got invalid response: " << line.get() << ", line length: " << line.length;
             stat_ = Error;
           } else if (line[0] == '$') {
             auto parse_result = ParseInt<uint64_t>(std::string(line.get() + 1, line.length - 1), 10);
@@ -985,6 +985,6 @@ void SlotMigrate::GetMigrateInfo(std::string *info) {
       break;
   }
 
-  *info = "migrating_slot: " + std::to_string(slot) + "\r\n" + "destination_node: " + dst_node_ + "\r\n" +
-          "migrating_state: " + task_state + "\r\n";
+  *info =
+      fmt::format("migrating_slot: {}\r\ndestination_node: {}\r\nmigrating_state: {}\r\n", slot, dst_node_, task_state);
 }