From: Arran Cudbard-Bell Date: Fri, 28 Jul 2023 19:22:20 +0000 (-0400) Subject: redis: Don't prefix function loading with "read only" X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f6e08a09f48219c0279a38712eac1455d93e2a2d;p=thirdparty%2Ffreeradius-server.git redis: Don't prefix function loading with "read only" --- diff --git a/src/lib/redis/base.h b/src/lib/redis/base.h index e8de377e4be..345c540f8b3 100644 --- a/src/lib/redis/base.h +++ b/src/lib/redis/base.h @@ -47,6 +47,8 @@ extern "C" { #define REDIS_ERROR_NO_SCRIPT_STR "NOSCRIPT" #define REDIS_DEFAULT_PORT 6379 +typedef struct fr_redis_cluster_node_s fr_redis_cluster_node_t; + /** Wrap freeReplyObject so we consistently check for NULL pointers * * Older versions such as 0.10 (which ship with Ubuntu <= 14.10) @@ -95,6 +97,7 @@ typedef enum { */ typedef struct { redisContext *handle; //!< Hiredis context used when issuing commands. + fr_redis_cluster_node_t *node; //!< Node this connection is to. } fr_redis_conn_t; /** Configuration parameters for a redis connection diff --git a/src/lib/redis/cluster.c b/src/lib/redis/cluster.c index 96eb079772f..bd031a44377 100644 --- a/src/lib/redis/cluster.c +++ b/src/lib/redis/cluster.c @@ -1483,6 +1483,7 @@ void *fr_redis_cluster_conn_create(TALLOC_CTX *ctx, void *instance, fr_time_delt conn = talloc_zero(ctx, fr_redis_conn_t); conn->handle = handle; + conn->node = node; talloc_set_destructor(conn, _cluster_conn_free); #ifdef HAVE_REDIS_SSL diff --git a/src/modules/rlm_redis/rlm_redis.c b/src/modules/rlm_redis/rlm_redis.c index bdbbeabd4f8..3a7a38cbc8c 100644 --- a/src/modules/rlm_redis/rlm_redis.c +++ b/src/modules/rlm_redis/rlm_redis.c @@ -142,12 +142,17 @@ static int lua_func_body_parse(TALLOC_CTX *ctx, void *out, void *parent, CONF_IT return 0; } -/** Change the state of a connection to READONLY execute a command and switch to READWRITE +/** Issue a command against redis and get a response + * + * This is a convenience function for dealing with commands which made need to execute against an + * ldap replica. It temporarily places the connection in readonly mode to allow commands to be + * run against ldap replicas, then reverts back to readwrite mode. * * @param[out] status_out Where to write the status from the command. * @param[out] reply_out Where to write the reply associated with the highest priority status. * @param[in] request The current request. * @param[in] conn to issue commands with. + * @param[in] read_only wrap command in READONLY/READWRITE. * @param[in] argc Redis command argument count. * @param[in] argv Redis command arguments. * @param[in] arg_len Optional array of redis command argument length. @@ -156,8 +161,10 @@ static int lua_func_body_parse(TALLOC_CTX *ctx, void *out, void *parent, CONF_IT * - -1 normal failure. * - -2 failure that may leave the connection in a READONLY state. */ -static int redis_command_read_only(fr_redis_rcode_t *status_out, redisReply **reply_out, - request_t *request, fr_redis_conn_t *conn, int argc, char const **argv, size_t arg_len[]) +static int redis_command(fr_redis_rcode_t *status_out, redisReply **reply_out, + request_t *request, fr_redis_conn_t *conn, + bool read_only, + int argc, char const **argv, size_t arg_len[]) { bool maybe_more = false; redisReply *reply; @@ -165,9 +172,12 @@ static int redis_command_read_only(fr_redis_rcode_t *status_out, redisReply **re *reply_out = NULL; - redisAppendCommand(conn->handle, "READONLY"); + if (read_only) redisAppendCommand(conn->handle, "READONLY"); redisAppendCommandArgv(conn->handle, argc, argv, arg_len); - redisAppendCommand(conn->handle, "READWRITE"); + if (read_only) { + redisAppendCommand(conn->handle, "READWRITE"); + } else goto parse_reply; + /* * Process the response for READONLY @@ -192,6 +202,7 @@ static int redis_command_read_only(fr_redis_rcode_t *status_out, redisReply **re fr_redis_reply_free(&reply); +parse_reply: /* * Process the response for the command */ @@ -212,6 +223,8 @@ static int redis_command_read_only(fr_redis_rcode_t *status_out, redisReply **re reply = NULL; *status_out = status; + if (!read_only) return 0; /* No more responses to deal with */ + /* * Process the response for READWRITE */ @@ -449,10 +462,8 @@ static xlat_action_t redis_lua_func_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, for (int i = 2; i < argc; i++) RDEBUG3("[%i] %s", i, argv[i]); REXDENT(); } - if (!func->read_only) { - reply = redisCommandArgv(conn->handle, argc, argv, arg_len); - status = fr_redis_command_status(conn, reply); - } else if (redis_command_read_only(&status, &reply, request, conn, argc, argv, arg_len) == -2) { + if (redis_command(&status, &reply, request, conn, + func->read_only, argc, argv, arg_len) == -2) { state.close_conn = true; } @@ -494,12 +505,9 @@ static xlat_action_t redis_lua_func_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, * synchronous code will need to be rewritten, so for now * we just load the script and try again. */ - if (!func->read_only) { - reply = redisCommandArgv(conn->handle, NUM_ELEMENTS(script_load_argv), - script_load_argv, script_load_arg_len); - status = fr_redis_command_status(conn, reply); - } else if (redis_command_read_only(&status, &reply, request, conn, NUM_ELEMENTS(script_load_argv), - script_load_argv, script_load_arg_len) == -2) { + if (redis_command(&status, &reply, request, conn, func->read_only, + NUM_ELEMENTS(script_load_argv), + script_load_argv, script_load_arg_len) == -2) { state.close_conn = true; } @@ -663,10 +671,7 @@ static xlat_action_t redis_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, REXDENT(); } - if (!read_only) { - reply = redisCommandArgv(conn->handle, argc, argv, arg_len); - status = fr_redis_command_status(conn, reply); - } else if (redis_command_read_only(&status, &reply, request, conn, argc, argv, arg_len) == -2) { + if (redis_command(&status, &reply, request, conn, read_only, argc, argv, arg_len) == -2) { goto close_conn; } @@ -738,10 +743,7 @@ static xlat_action_t redis_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, REXDENT(); } - if (!read_only) { - reply = redisCommandArgv(conn->handle, argc, argv, arg_len); - status = fr_redis_command_status(conn, reply); - } else if (redis_command_read_only(&status, &reply, request, conn, argc, argv, arg_len) == -2) { + if (redis_command(&status, &reply, request, conn, read_only, argc, argv, arg_len) == -2) { state.close_conn = true; } } @@ -818,8 +820,8 @@ static int mod_instantiate(module_inst_ctx_t const *mctx) /* * preload onto every node, even replicas. */ - if (redis_command_read_only(&status, &reply, NULL, conn, - NUM_ELEMENTS(script_load_argv), script_load_argv, script_load_arg_len) == -2) { + if (redis_command(&status, &reply, NULL, conn, false, + NUM_ELEMENTS(script_load_argv), script_load_argv, script_load_arg_len) == -2) { error: fr_pool_connection_release(pool, NULL, conn); talloc_free(nodes);