]> git.ipfire.org Git - thirdparty/ccache.git/commitdiff
Touch up Redis storage code slightly
authorJoel Rosdahl <joel@rosdahl.net>
Sat, 10 Jul 2021 19:24:35 +0000 (21:24 +0200)
committerJoel Rosdahl <joel@rosdahl.net>
Mon, 12 Jul 2021 11:24:17 +0000 (13:24 +0200)
Note: Unlike atoi, std::stoi throws an exception on conversion error so
use Util::parse_unsigned instead which throws an exception that is
caught correctly.

cmake/Findhiredis.cmake
doc/MANUAL.adoc
misc/upload-redis
src/storage/secondary/RedisStorage.cpp
src/storage/secondary/RedisStorage.hpp
test/suites/secondary_redis.bash

index 5667d32d4cf2d8fdf461201502b20cc8a01520e8..ec5ff2351ff2993054eb33dd7c0d964e54f03f17 100644 (file)
@@ -10,7 +10,8 @@ endif()
 
 include(FindPackageHandleStandardArgs)
 find_package_handle_standard_args(
-  hiredis "please install libhiredis"
+  hiredis
+  "please install libhiredis or disable with -DREDIS_STORAGE_BACKEND=OFF"
   HIREDIS_INCLUDE_DIR HIREDIS_LIBRARY)
 mark_as_advanced(HIREDIS_INCLUDE_DIR HIREDIS_LIBRARY)
 
index 09392d7602e824d1027a53ec3578895d6929ecef..6d3e5316e8655809ffa94694720abfc541a87294 100644 (file)
@@ -796,8 +796,8 @@ Examples:
 +
 * `file:///shared/nfs/directory`
 * `file:///shared/nfs/one|read-only file:///shared/nfs/two`
-* `http://example.org/cache`
-* `redis://server.example.com`
+* `http://example.com/cache`
+* `redis://example.com`
 
 [[config_sloppiness]] *sloppiness* (*CCACHE_SLOPPINESS*)::
 
@@ -945,7 +945,7 @@ Note that ccache will not perform any cleanup of the HTTP storage.
 Examples:
 
 * `http://localhost:8080/`
-* `http://example.org/cache`
+* `http://example.com/cache`
 
 Known issues and limitations:
 
@@ -958,24 +958,22 @@ Known issues and limitations:
 
 URL format: `redis://HOST[:PORT]` or `redis://UNIX_SOCKET_PATH`
 
-This backend stores data in a Redis (or Redis-compatible) server.
-There are implementations for both memory and disk based storage.
+This backend stores data in a https://redis.io[Redis] (or Redis-compatible)
+server. There are implementations for both memory-based and disk-based storage.
 
-See <https://redis.io>
-
-Note that ccache will not perform any cleanup of the Redis storage.
-But you can configure LRU eviction: <https://redis.io/topics/lru-cache>
+Note that ccache will not perform any cleanup of the Redis storage, but you can
+https://redis.io/topics/lru-cache[configure LRU eviction].
 
 Examples:
 
-* `redis://localhost:6379`
+* `redis://localhost:6379|connect-timeout=50`
 * `redis:///var/run/redis/redis-server.sock`
 
 Optional attributes:
 
-* *connect-timeout*: Timeout (in ms) for network connection.
-* *operation-timeout*: Timeout (in ms) for Redis commands.
-* *username*: Username for the Redis AUTH command. (Requires ACL)
+* *connect-timeout*: Timeout (in ms) for network connection. The default is 100.
+* *operation-timeout*: Timeout (in ms) for Redis commands. The default is 10000.
+* *username*: Username for the Redis AUTH command (requires Redis 6 or newer).
 * *password*: Password for the Redis AUTH command.
 
 == Cache size management
index 6fe4335fb0df2eed9ec3c9e1d8f7322f252790ce..c91ade38b6ebde0064088c870ea256fe4bf35371 100755 (executable)
@@ -1,7 +1,7 @@
 #!/usr/bin/env python3
 
-# this script will upload the contents of the cache,
-# from primary storage to the redis secondary storage
+# This script uploads the contents of the cache from primary storage to a Redis
+# secondary storage.
 
 import redis
 import os
@@ -16,7 +16,9 @@ else:
     host, port, sock = config, 6379, None
 username = os.getenv("REDIS_USERNAME")
 password = os.getenv("REDIS_PASSWORD")
-context = redis.Redis(host=host, port=port, unix_socket_path=sock, password=password)
+context = redis.Redis(
+    host=host, port=port, unix_socket_path=sock, password=password
+)
 
 CCACHE_MANIFEST = b"cCmF"
 CCACHE_RESULT = b"cCrS"
index 4dd9eb0b5c8c2292b7a8f07f6e78ae29f6069ecc..8588efa876779db18b839a1f27b29ee2954ee3a6 100644 (file)
 
 #include <hiredis/hiredis.h>
 
+#ifdef HAVE_SYS_TIME_H
+#  include <sys/time.h>
+#endif
+
 namespace storage {
 namespace secondary {
 
-const struct timeval DEFAULT_CONNECT_TIMEOUT = {0, 100 * 1000};  // 100 ms
-const struct timeval DEFAULT_OPERATION_TIMEOUT = {10, 0 * 1000}; // 10 sec
+const uint64_t DEFAULT_CONNECT_TIMEOUT_MS = 100;
+const uint64_t DEFAULT_OPERATION_TIMEOUT_MS = 10000;
+const int DEFAULT_PORT = 6379;
 
 static struct timeval
-milliseconds_to_timeval(const std::string& msec)
+milliseconds_to_timeval(const uint64_t ms)
 {
-  int ms = std::stoi(msec);
   struct timeval tv;
   tv.tv_sec = ms / 1000;
   tv.tv_usec = (ms % 1000) * 1000;
   return tv;
 }
 
-static std::string
-timeval_to_string(struct timeval tv)
-{
-  return FMT("{:.3f}s", tv.tv_sec + tv.tv_usec / 1000000.0);
-}
-
-static nonstd::optional<struct timeval>
-parse_timeout_attribute(const AttributeMap& attributes, const std::string& name)
+static uint64_t
+parse_timeout_attribute(const AttributeMap& attributes,
+                        const std::string& name,
+                        const uint64_t default_value)
 {
   const auto it = attributes.find(name);
   if (it == attributes.end()) {
-    return nonstd::nullopt;
+    return default_value;
+  } else {
+    return Util::parse_unsigned(it->second, 1, 1000 * 3600, "timeout");
   }
-  return milliseconds_to_timeval(it->second);
 }
 
 static nonstd::optional<std::string>
@@ -68,22 +69,23 @@ parse_string_attribute(const AttributeMap& attributes, const std::string& name)
 
 RedisStorage::RedisStorage(const Url& url, const AttributeMap& attributes)
   : m_url(url),
-    m_connect_timeout(parse_timeout_attribute(attributes, "connect-timeout")),
-    m_operation_timeout(
-      parse_timeout_attribute(attributes, "operation-timeout")),
+    m_prefix("ccache"), // TODO: attribute
+    m_context(nullptr),
+    m_connect_timeout(parse_timeout_attribute(
+      attributes, "connect-timeout", DEFAULT_CONNECT_TIMEOUT_MS)),
+    m_operation_timeout(parse_timeout_attribute(
+      attributes, "operation-timeout", DEFAULT_OPERATION_TIMEOUT_MS)),
     m_username(parse_string_attribute(attributes, "username")),
-    m_password(parse_string_attribute(attributes, "password"))
+    m_password(parse_string_attribute(attributes, "password")),
+    m_connected(false),
+    m_invalid(false)
 {
-  m_prefix = "ccache"; // TODO: attribute
-  m_context = nullptr;
-  m_connected = false;
-  m_invalid = false;
 }
 
 RedisStorage::~RedisStorage()
 {
-  disconnect();
   if (m_context) {
+    LOG_RAW("Redis disconnect");
     redisFree(m_context);
     m_context = nullptr;
   }
@@ -104,56 +106,57 @@ RedisStorage::connect()
       m_connected = true;
       return REDIS_OK;
     }
-    LOG("Redis reconnect err: {}", m_context->errstr);
+    LOG("Redis reconnection error: {}", m_context->errstr);
     redisFree(m_context);
     m_context = nullptr;
   }
 
   ASSERT(m_url.scheme() == "redis");
-  std::string host = m_url.host();
-  std::string port = m_url.port();
-  std::string sock = m_url.path();
-  if (m_connect_timeout) {
-    LOG("Redis connect timeout {}", timeval_to_string(*m_connect_timeout));
-  }
-  struct timeval connect_timeout =
-    m_connect_timeout ? *m_connect_timeout : DEFAULT_CONNECT_TIMEOUT;
+  const auto& host = m_url.host();
+  const auto& port = m_url.port();
+  const auto& sock = m_url.path();
+  const auto connect_timeout = milliseconds_to_timeval(m_connect_timeout);
   if (!host.empty()) {
-    int p = port.empty() ? 6379 : std::stoi(port);
+    const int p = port.empty() ? DEFAULT_PORT
+                               : Util::parse_unsigned(port, 1, 65535, "port");
+    LOG("Redis connecting to {}:{} (timeout {} ms)",
+        host.c_str(),
+        p,
+        m_connect_timeout);
     m_context = redisConnectWithTimeout(host.c_str(), p, connect_timeout);
   } else if (!sock.empty()) {
+    LOG("Redis connecting to {} (timeout {} ms)",
+        sock.c_str(),
+        m_connect_timeout);
     m_context = redisConnectUnixWithTimeout(sock.c_str(), connect_timeout);
   } else {
-    LOG("Redis invalid url: {}", m_url.str());
+    LOG("Invalid Redis URL: {}", m_url.str());
     m_invalid = true;
     return REDIS_ERR;
   }
 
   if (!m_context) {
-    LOG("Redis connect {} err NULL", m_url.str());
+    LOG_RAW("Redis connection error (NULL context)");
     m_invalid = true;
     return REDIS_ERR;
   } else if (m_context->err) {
-    LOG("Redis connect {} err: {}", m_url.str(), m_context->errstr);
+    LOG("Redis connection error: {}", m_context->errstr);
     m_invalid = true;
     return m_context->err;
   } else {
     if (m_context->connection_type == REDIS_CONN_TCP) {
-      LOG(
-        "Redis connect tcp {}:{} OK", m_context->tcp.host, m_context->tcp.port);
+      LOG("Redis connection to {}:{} OK",
+          m_context->tcp.host,
+          m_context->tcp.port);
     }
     if (m_context->connection_type == REDIS_CONN_UNIX) {
-      LOG("Redis connect unix {} OK", m_context->unix_sock.path);
+      LOG("Redis connection to {} OK", m_context->unix_sock.path);
     }
     m_connected = true;
 
-    if (m_operation_timeout) {
-      LOG("Redis timeout {}", timeval_to_string(*m_operation_timeout));
-    }
-    struct timeval operation_timeout =
-      m_operation_timeout ? *m_operation_timeout : DEFAULT_OPERATION_TIMEOUT;
-    if (redisSetTimeout(m_context, operation_timeout) != REDIS_OK) {
-      LOG_RAW("Failed to set timeout");
+    if (redisSetTimeout(m_context, milliseconds_to_timeval(m_operation_timeout))
+        != REDIS_OK) {
+      LOG_RAW("Failed to set operation timeout");
     }
 
     return auth();
@@ -194,7 +197,7 @@ RedisStorage::auth()
 inline bool
 is_error(int err)
 {
-  return (err != REDIS_OK);
+  return err != REDIS_OK;
 }
 
 inline bool
@@ -202,24 +205,13 @@ is_timeout(int err)
 {
 #ifdef REDIS_ERR_TIMEOUT
   // Only returned for hiredis version 1.0.0 and above
-  return (err == REDIS_ERR_TIMEOUT);
+  return err == REDIS_ERR_TIMEOUT;
 #else
   (void)err;
   return false;
 #endif
 }
 
-void
-RedisStorage::disconnect()
-{
-  if (m_connected) {
-    // Note: only the async API actually disconnects from the server
-    //       the connection is eventually cleaned up in redisFree()
-    LOG_RAW("Redis disconnect");
-    m_connected = false;
-  }
-}
-
 nonstd::expected<nonstd::optional<std::string>, SecondaryStorage::Error>
 RedisStorage::get(const Digest& key)
 {
index 2a12ea1fa3d77f49f926aa5f27f3105ba238d12b..04d27de6129aac41cf7aab48861256d49f35df55 100644 (file)
 
 #include <third_party/url.hpp>
 
-#ifdef HAVE_SYS_TIME_H
-#  include <sys/time.h>
-#endif
-
 struct redisContext;
 
 namespace storage {
@@ -49,8 +45,8 @@ private:
   Url m_url;
   std::string m_prefix;
   redisContext* m_context;
-  const nonstd::optional<struct timeval> m_connect_timeout;
-  const nonstd::optional<struct timeval> m_operation_timeout;
+  const uint64_t m_connect_timeout;
+  const uint64_t m_operation_timeout;
   const nonstd::optional<std::string> m_username;
   const nonstd::optional<std::string> m_password;
   bool m_connected;
@@ -58,7 +54,6 @@ private:
 
   int connect();
   int auth();
-  void disconnect();
   std::string get_key_string(const Digest& digest) const;
 };
 
index 6eb568eae66771d0981ead0c162a99458723d731..cbaa2e0b1b14440f6d8ce8f096fe5beb9c9595a6 100644 (file)
@@ -10,13 +10,15 @@ SUITE_secondary_redis_PROBE() {
 }
 
 SUITE_secondary_redis_SETUP() {
+    redis_url=redis://localhost:7777
+
     unset CCACHE_NODIRECT
-    export CCACHE_SECONDARY_STORAGE="redis://localhost:7777"
+    export CCACHE_SECONDARY_STORAGE="$redis_url"
 
     generate_code 1 test.c
 }
 
-expect_number_of_cache_entries() {
+expect_number_of_redis_cache_entries() {
     local expected=$1
     local url=$2
     local actual
@@ -28,15 +30,18 @@ expect_number_of_cache_entries() {
 }
 
 SUITE_secondary_redis() {
-    if $HOST_OS_APPLE; then
-         # no coreutils on darwin by default, perl rather than gtimeout
-         function timeout() { perl -e 'alarm shift; exec @ARGV' "$@"; }
+    if ! command -v timeout >/dev/null; then
+         timeout() { perl -e 'alarm shift; exec @ARGV' "$@"; }
     fi
-    timeout 10 redis-server --bind localhost --port 7777 &
-    sleep 0.1 # wait for boot
-    redis-cli -p 7777 ping
 
-    secondary="redis://localhost:7777"
+    timeout 10 redis-server --bind localhost --port 7777 >/dev/null &
+
+    # Wait for boot.
+    i=0
+    while [ $i -lt 100 ] && ! redis-cli -p 7777 ping >&/dev/null; do
+        sleep 0.1
+        i=$((i + 1))
+    done
 
     # -------------------------------------------------------------------------
     TEST "Base case"
@@ -45,25 +50,25 @@ SUITE_secondary_redis() {
     expect_stat 'cache hit (direct)' 0
     expect_stat 'cache miss' 1
     expect_stat 'files in cache' 2
-    expect_number_of_cache_entries 2 "$secondary" # result + manifest
+    expect_number_of_redis_cache_entries 2 "$redis_url" # result + manifest
 
     $CCACHE_COMPILE -c test.c
     expect_stat 'cache hit (direct)' 1
     expect_stat 'cache miss' 1
     expect_stat 'files in cache' 2
-    expect_number_of_cache_entries 2 "$secondary" # result + manifest
+    expect_number_of_redis_cache_entries 2 "$redis_url" # result + manifest
 
     $CCACHE -C >/dev/null
     expect_stat 'files in cache' 0
-    expect_number_of_cache_entries 2 "$secondary" # result + manifest
+    expect_number_of_redis_cache_entries 2 "$redis_url" # result + manifest
 
     $CCACHE_COMPILE -c test.c
     expect_stat 'cache hit (direct)' 2
     expect_stat 'cache miss' 1
     expect_stat 'files in cache' 0
-    expect_number_of_cache_entries 2 "$secondary" # result + manifest
+    expect_number_of_redis_cache_entries 2 "$redis_url" # result + manifest
 
-    redis-cli -p 7777 flushdb
+    redis-cli -p 7777 flushdb >/dev/null
 
     # -------------------------------------------------------------------------
     TEST "Read-only"
@@ -72,11 +77,11 @@ SUITE_secondary_redis() {
     expect_stat 'cache hit (direct)' 0
     expect_stat 'cache miss' 1
     expect_stat 'files in cache' 2
-    expect_number_of_cache_entries 2 "$secondary" # result + manifest
+    expect_number_of_redis_cache_entries 2 "$redis_url" # result + manifest
 
     $CCACHE -C >/dev/null
     expect_stat 'files in cache' 0
-    expect_number_of_cache_entries 2 "$secondary" # result + manifest
+    expect_number_of_redis_cache_entries 2 "$redis_url" # result + manifest
 
     CCACHE_SECONDARY_STORAGE+="|read-only"
 
@@ -84,12 +89,12 @@ SUITE_secondary_redis() {
     expect_stat 'cache hit (direct)' 1
     expect_stat 'cache miss' 1
     expect_stat 'files in cache' 0
-    expect_number_of_cache_entries 2 "$secondary" # result + manifest
+    expect_number_of_redis_cache_entries 2 "$redis_url" # result + manifest
 
     echo 'int x;' >> test.c
     $CCACHE_COMPILE -c test.c
     expect_stat 'cache hit (direct)' 1
     expect_stat 'cache miss' 2
     expect_stat 'files in cache' 2
-    expect_number_of_cache_entries 2 "$secondary" # result + manifest
+    expect_number_of_redis_cache_entries 2 "$redis_url" # result + manifest
 }