Introduce configuration options in the cluster API (#137)

This PR introduces `valkeyClusterOption` and a new API to allow users
to set all options before connecting synchronously or asynchronously to a cluster.
Example:

valkeyClusterOptions options = {
   .initial_nodes = "127.0.0.1:7000";
};
valkeyClusterContext *cc = valkeyClusterConnectWithOptions(&options)

Some API functions are removed/replaced and some are changed,
see migration-guide.md for information on API changes.

The new option struct enables users to set the initial nodes, callbacks,
timeouts, options, TLS- and event-adapters  instead of using a wide range
of `valkeyClusterSetOptionXxx` functions on a `valkeyClusterContext`.
There are still options that are allowed to be configured during runtime
like `valkeyClusterSetOptionTimeout`, but most other options currently
requires a reconnect.

The async cluster API will no longer require that users calls the
blocking cluster API using `valkeyClusterConnect2(acc->cc)`.
We now provide a new option `VALKEY_OPT_BLOCKING_INITIAL_UPDATE`
to enable a blocking slotmap update after an initial connect using the
`valkeyClusterAsyncConnectWithOptions` API. 

With this change there is a symmetry between the cluster API and
the standalone API, which already provides a `valkeyOptions` struct
to allow users to configure options.

Signed-off-by: Björn Svensson <[email protected]>

Author: bjosv

.github/wordlist.txt

@@ -17,6 +17,7 @@ Autoloading
 backend
 backends
 behaviour
+bitwise
 boolean
 bugfix
 CAS

CMakeLists.txt

@@ -161,7 +161,6 @@ IF(ENABLE_TLS)
     ENDIF()
     FIND_PACKAGE(OpenSSL REQUIRED)
     SET(valkey_tls_sources
-        src/cluster_tls.c
         src/tls.c)
     ADD_LIBRARY(valkey_tls ${valkey_tls_sources})
     ADD_LIBRARY(valkey::valkey_tls ALIAS valkey_tls)

Makefile

@@ -15,8 +15,8 @@ INCLUDE_DIR = include/valkey
 TEST_SRCS = $(TEST_DIR)/client_test.c $(TEST_DIR)/ut_parse_cmd.c $(TEST_DIR)/ut_slotmap_update.c
 TEST_BINS = $(patsubst $(TEST_DIR)/%.c,$(TEST_DIR)/%,$(TEST_SRCS))
 
-SOURCES = $(filter-out $(wildcard $(SRC_DIR)/*tls.c) $(SRC_DIR)/rdma.c, $(wildcard $(SRC_DIR)/*.c))
-HEADERS = $(filter-out $(wildcard $(INCLUDE_DIR)/*tls.h) $(INCLUDE_DIR)/rdma.h, $(wildcard $(INCLUDE_DIR)/*.h))
+SOURCES = $(filter-out $(SRC_DIR)/tls.c $(SRC_DIR)/rdma.c, $(wildcard $(SRC_DIR)/*.c))
+HEADERS = $(filter-out $(INCLUDE_DIR)/tls.h $(INCLUDE_DIR)/rdma.h, $(wildcard $(INCLUDE_DIR)/*.h))
 
 OBJS = $(patsubst $(SRC_DIR)/%.c,$(OBJ_DIR)/%.o,$(SOURCES))
 
@@ -95,7 +95,7 @@ TLS_DYLIB_MAKE_CMD=$(CC) $(OPTIMIZATION) $(PLATFORM_FLAGS) -shared -Wl,-soname,$
 USE_TLS?=0
 
 ifeq ($(USE_TLS),1)
-  TLS_SOURCES = $(wildcard $(SRC_DIR)/*tls.c)
+  TLS_SOURCES = $(SRC_DIR)/tls.c
   TLS_OBJS = $(patsubst $(SRC_DIR)/%.c,$(OBJ_DIR)/%.o,$(TLS_SOURCES))
 
   # This is required for test.c only
@@ -293,7 +293,6 @@ install: $(DYLIBNAME) $(STLIBNAME) $(PKGCONFNAME) $(TLS_INSTALL)
 install-tls: $(TLS_DYLIBNAME) $(TLS_STLIBNAME) $(TLS_PKGCONFNAME)
 	mkdir -p $(INSTALL_INCLUDE_PATH) $(INSTALL_LIBRARY_PATH)
 	$(INSTALL) $(INCLUDE_DIR)/tls.h $(INSTALL_INCLUDE_PATH)
-	$(INSTALL) $(INCLUDE_DIR)/cluster_tls.h $(INSTALL_INCLUDE_PATH)
 	$(INSTALL) $(TLS_DYLIBNAME) $(INSTALL_LIBRARY_PATH)/$(TLS_DYLIB_MINOR_NAME)
 	ln -sf $(TLS_DYLIB_MINOR_NAME) $(INSTALL_LIBRARY_PATH)/$(TLS_ROOT_DYLIB_NAME)
 	ln -sf $(TLS_DYLIB_MINOR_NAME) $(INSTALL_LIBRARY_PATH)/$(TLS_DYLIB_MAJOR_NAME)

docs/cluster.md

@@ -27,11 +27,20 @@ The type `sds` is removed from the public API.
 
 ## Migrating from `hiredis-cluster` 0.14.0
 
+* The cluster client initiation procedure is changed and `valkeyClusterOptions`
+  should be used to specify options when creating a context.
+  See documentation for configuration examples when using the
+  [Synchronous API](cluster.md#synchronous-api) or the
+  [Asynchronous API](cluster.md#asynchronous-api).
+  The [examples](../examples/) directory also contains some common client
+  initiation examples that might be helpful.
+* The default command to update the internal slot map is changed to `CLUSTER SLOTS`.
+  `CLUSTER NODES` can be re-enabled through options using `VALKEY_OPT_USE_CLUSTER_NODES`.
+
 ### Renamed API functions
 
 * `ctx_get_by_node` is renamed to `valkeyClusterGetValkeyContext`.
 * `actx_get_by_node` is renamed to `valkeyClusterGetValkeyAsyncContext`.
-* `redisClusterAsyncSetConnectCallbackNC` is renamed to `valkeyClusterAsyncSetConnectCallback`.
 
 ### Renamed API defines
 
@@ -41,14 +50,32 @@ The type `sds` is removed from the public API.
 
 ### Removed API functions
 
-* `redisClusterSetMaxRedirect` removed and replaced with `valkeyClusterSetOptionMaxRetry`.
-* `redisClusterSetOptionAddNode` removed and replaced with `valkeyClusterSetOptionAddNodes`.
-  (Note the "s" in the end of the function name.)
+* `redisClusterConnect2` removed, use `valkeyClusterConnectWithOptions`.
+* `redisClusterContextInit` removed, use `valkeyClusterConnectWithOptions`.
+* `redisClusterSetConnectCallback` removed, use `valkeyClusterOptions.connect_callback`.
+* `redisClusterSetEventCallback` removed, use `valkeyClusterOptions.event_callback`.
+* `redisClusterSetMaxRedirect` removed, use `valkeyClusterOptions.max_retry`.
+* `redisClusterSetOptionAddNode` removed, use `valkeyClusterOptions.initial_nodes`.
+* `redisClusterSetOptionAddNodes` removed, use `valkeyClusterOptions.initial_nodes`.
 * `redisClusterSetOptionConnectBlock` removed since it was deprecated.
 * `redisClusterSetOptionConnectNonBlock` removed since it was deprecated.
+* `redisClusterSetOptionConnectTimeout` removed, use `valkeyClusterOptions.connect_timeout`.
+* `redisClusterSetOptionMaxRetry` removed, use `valkeyClusterOptions.max_retry`.
+* `redisClusterSetOptionParseSlaves` removed, use `valkeyClusterOptions.flags` and `VALKEY_OPT_USE_REPLICAS`.
+* `redisClusterSetOptionPassword` removed, use `valkeyClusterOptions.password`.
+* `redisClusterSetOptionRouteUseSlots` removed, the use of `CLUSTER SLOTS` is enabled by default.
+* `redisClusterSetOptionUsername` removed, use `valkeyClusterOptions.username`.
+* `redisClusterAsyncSetConnectCallback` removed, but `valkeyClusterOptions.async_connect_callback` can be used which accepts a non-const callback function prototype.
+* `redisClusterAsyncSetConnectCallbackNC` removed, use `valkeyClusterOptions.async_connect_callback`.
+* `redisClusterAsyncSetDisconnectCallback` removed, use `valkeyClusterOptions.async_disconnect_callback`.
 * `parse_cluster_nodes` removed from API, for internal use only.
 * `parse_cluster_slots` removed from API, for internal use only.
-* `redisClusterAsyncSetConnectCallback` is removed, but can be replaced with `valkeyClusterAsyncSetConnectCallback` which accepts the non-const callback function prototype.
+
+### Removed API defines
+
+* `HIRCLUSTER_FLAG_NULL` removed.
+* `HIRCLUSTER_FLAG_ADD_SLAVE` removed, flag can be replaced with an option, see `VALKEY_OPT_USE_REPLICAS`.
+* `HIRCLUSTER_FLAG_ROUTE_USE_SLOTS` removed, the use of `CLUSTER SLOTS` is enabled by default.
 
 ### Removed support for splitting multi-key commands per slot
 

docs/migration-guide.md

@@ -72,12 +72,12 @@ There are also several flags you can specify when using the `valkeyOptions` help
 
 | Flag | Description  |
 | --- | --- |
-| VALKEY\_OPT\_NONBLOCK | Tells libvalkey to make a non-blocking connection. |
-| VALKEY\_OPT\_REUSEADDR | Tells libvalkey to set the [SO_REUSEADDR](https://man7.org/linux/man-pages/man7/socket.7.html) socket option |
-| VALKEY\_OPT\_PREFER\_IPV4<br>VALKEY\_OPT\_PREFER_IPV6<br>VALKEY\_OPT\_PREFER\_IP\_UNSPEC | Informs libvalkey to either prefer IPv4 or IPv6 when invoking [getaddrinfo](https://man7.org/linux/man-pages/man3/gai_strerror.3.html).  `VALKEY_OPT_PREFER_IP_UNSPEC` will cause libvalkey to specify `AF_UNSPEC` in the getaddrinfo call, which means both IPv4 and IPv6 addresses will be searched simultaneously.<br>Libvalkey prefers IPv4 by default. |
-| VALKEY\_OPT\_NO\_PUSH\_AUTOFREE | Tells libvalkey to not install the default RESP3 PUSH handler (which just intercepts and frees the replies).  This is useful in situations where you want to process these messages in-band. |
-| VALKEY\_OPT\_NOAUTOFREEREPLIES | **ASYNC**: tells libvalkey not to automatically invoke `freeReplyObject` after executing the reply callback. |
-| VALKEY\_OPT\_NOAUTOFREE | **ASYNC**: Tells libvalkey not to automatically free the `valkeyAsyncContext` on connection/communication failure, but only if the user makes an explicit call to `valkeyAsyncDisconnect` or `valkeyAsyncFree` |
+| `VALKEY_OPT_NONBLOCK` | Tells libvalkey to make a non-blocking connection. |
+| `VALKEY_OPT_REUSEADDR` | Tells libvalkey to set the [SO_REUSEADDR](https://man7.org/linux/man-pages/man7/socket.7.html) socket option |
+| `VALKEY_OPT_PREFER_IPV4`<br>`VALKEY_OPT_PREFER_IPV6`<br>`VALKEY_OPT_PREFER_IP_UNSPEC` | Informs libvalkey to either prefer IPv4 or IPv6 when invoking [getaddrinfo](https://man7.org/linux/man-pages/man3/gai_strerror.3.html).  `VALKEY_OPT_PREFER_IP_UNSPEC` will cause libvalkey to specify `AF_UNSPEC` in the getaddrinfo call, which means both IPv4 and IPv6 addresses will be searched simultaneously.<br>Libvalkey prefers IPv4 by default. |
+| `VALKEY_OPT_NO_PUSH_AUTOFREE` | Tells libvalkey to not install the default RESP3 PUSH handler (which just intercepts and frees the replies).  This is useful in situations where you want to process these messages in-band. |
+| `VALKEY_OPT_NOAUTOFREEREPLIES` | **ASYNC**: tells libvalkey not to automatically invoke `freeReplyObject` after executing the reply callback. |
+| `VALKEY_OPT_NOAUTOFREE` | **ASYNC**: Tells libvalkey not to automatically free the `valkeyAsyncContext` on connection/communication failure, but only if the user makes an explicit call to `valkeyAsyncDisconnect` or `valkeyAsyncFree` |
 
 ### Executing commands
 
@@ -298,7 +298,7 @@ Libvalkey also has an asynchronous API which supports a great many different eve
 Libvalkey provides an `valkeyAsyncContext` to manage asynchronous connections which works similarly to the synchronous context.
 
 ```c
-valkeyAsyncContext *ac = valkeyAsyncConnect("loalhost", 6379);
+valkeyAsyncContext *ac = valkeyAsyncConnect("localhost", 6379);
 if (ac == NULL) {
     fprintf(stderr, "Error:  Out of memory trying to allocate valkeyAsyncContext\n");
     exit(1);

docs/standalone.md

@@ -1,5 +1,5 @@
 #include <valkey/cluster.h>
-#include <valkey/cluster_tls.h>
+#include <valkey/tls.h>
 
 #include <valkey/adapters/libevent.h>
 
@@ -66,23 +66,22 @@ int main(int argc, char **argv) {
         exit(1);
     }
 
-    valkeyClusterAsyncContext *acc = valkeyClusterAsyncContextInit();
-    assert(acc);
-    valkeyClusterAsyncSetConnectCallback(acc, connectCallback);
-    valkeyClusterAsyncSetDisconnectCallback(acc, disconnectCallback);
-    valkeyClusterSetOptionAddNodes(acc->cc, CLUSTER_NODE_TLS);
-    valkeyClusterSetOptionRouteUseSlots(acc->cc);
-    valkeyClusterSetOptionParseSlaves(acc->cc);
-    valkeyClusterSetOptionEnableTLS(acc->cc, tls);
-
-    if (valkeyClusterConnect2(acc->cc) != VALKEY_OK) {
-        printf("Error: %s\n", acc->cc->errstr);
+    struct event_base *base = event_base_new();
+
+    valkeyClusterOptions options = {0};
+    options.initial_nodes = CLUSTER_NODE_TLS;
+    options.async_connect_callback = connectCallback;
+    options.async_disconnect_callback = disconnectCallback;
+    options.tls = tls;
+    options.tls_init_fn = &valkeyInitiateTLSWithContext;
+    valkeyClusterOptionsUseLibevent(&options, base);
+
+    valkeyClusterAsyncContext *acc = valkeyClusterAsyncConnectWithOptions(&options);
+    if (acc == NULL || acc->err != 0) {
+        printf("Error: %s\n", acc ? acc->errstr : "OOM");
         exit(-1);
     }
 
-    struct event_base *base = event_base_new();
-    valkeyClusterLibeventAttach(acc, base);
-
     int status;
     status = valkeyClusterAsyncCommand(acc, setCallback, (char *)"THE_ID",
                                        "SET %s %s", "key", "value");

examples/cluster-async-tls.c

@@ -49,53 +49,55 @@ void disconnectCallback(const valkeyAsyncContext *ac, int status) {
 int main(int argc, char **argv) {
     (void)argc;
     (void)argv;
+    struct event_base *base = event_base_new();
+
+    valkeyClusterOptions options = {0};
+    options.initial_nodes = "127.0.0.1:7000";
+    options.async_connect_callback = connectCallback;
+    options.async_disconnect_callback = disconnectCallback;
+    valkeyClusterOptionsUseLibevent(&options, base);
+
     printf("Connecting...\n");
-    valkeyClusterAsyncContext *cc =
-        valkeyClusterAsyncConnect("127.0.0.1:7000", VALKEYCLUSTER_FLAG_NULL);
-    if (!cc) {
+    valkeyClusterAsyncContext *acc = valkeyClusterAsyncConnectWithOptions(&options);
+    if (!acc) {
         printf("Error: Allocation failure\n");
         exit(-1);
-    } else if (cc->err) {
-        printf("Error: %s\n", cc->errstr);
+    } else if (acc->err) {
+        printf("Error: %s\n", acc->errstr);
         // handle error
         exit(-1);
     }
 
-    struct event_base *base = event_base_new();
-    valkeyClusterLibeventAttach(cc, base);
-    valkeyClusterAsyncSetConnectCallback(cc, connectCallback);
-    valkeyClusterAsyncSetDisconnectCallback(cc, disconnectCallback);
-
     int status;
-    status = valkeyClusterAsyncCommand(cc, setCallback, (char *)"THE_ID",
+    status = valkeyClusterAsyncCommand(acc, setCallback, (char *)"THE_ID",
                                        "SET %s %s", "key", "value");
     if (status != VALKEY_OK) {
-        printf("error: err=%d errstr=%s\n", cc->err, cc->errstr);
+        printf("error: err=%d errstr=%s\n", acc->err, acc->errstr);
     }
 
-    status = valkeyClusterAsyncCommand(cc, getCallback, (char *)"THE_ID",
+    status = valkeyClusterAsyncCommand(acc, getCallback, (char *)"THE_ID",
                                        "GET %s", "key");
     if (status != VALKEY_OK) {
-        printf("error: err=%d errstr=%s\n", cc->err, cc->errstr);
+        printf("error: err=%d errstr=%s\n", acc->err, acc->errstr);
     }
 
-    status = valkeyClusterAsyncCommand(cc, setCallback, (char *)"THE_ID",
+    status = valkeyClusterAsyncCommand(acc, setCallback, (char *)"THE_ID",
                                        "SET %s %s", "key2", "value2");
     if (status != VALKEY_OK) {
-        printf("error: err=%d errstr=%s\n", cc->err, cc->errstr);
+        printf("error: err=%d errstr=%s\n", acc->err, acc->errstr);
     }
 
-    status = valkeyClusterAsyncCommand(cc, getCallback, (char *)"THE_ID",
+    status = valkeyClusterAsyncCommand(acc, getCallback, (char *)"THE_ID",
                                        "GET %s", "key2");
     if (status != VALKEY_OK) {
-        printf("error: err=%d errstr=%s\n", cc->err, cc->errstr);
+        printf("error: err=%d errstr=%s\n", acc->err, acc->errstr);
     }
 
     printf("Dispatch..\n");
     event_base_dispatch(base);
 
     printf("Done..\n");
-    valkeyClusterAsyncFree(cc);
+    valkeyClusterAsyncFree(acc);
     event_base_free(base);
     return 0;
 }

examples/cluster-async.c

@@ -126,11 +126,8 @@ void disconnectCallback(const valkeyAsyncContext *ac, int status) {
 /* Helper to modify keys using a separate client. */
 void modifyKey(const char *key, const char *value) {
     printf("Modify key: '%s'\n", key);
-    valkeyClusterContext *cc = valkeyClusterContextInit();
-    int status = valkeyClusterSetOptionAddNodes(cc, CLUSTER_NODE);
-    assert(status == VALKEY_OK);
-    status = valkeyClusterConnect2(cc);
-    assert(status == VALKEY_OK);
+    valkeyClusterContext *cc = valkeyClusterConnect(CLUSTER_NODE);
+    assert(cc);
 
     valkeyReply *reply = valkeyClusterCommand(cc, "SET %s %s", key, value);
     assert(reply != NULL);
@@ -142,24 +139,22 @@ void modifyKey(const char *key, const char *value) {
 int main(int argc, char **argv) {
     (void)argc;
     (void)argv;
-    valkeyClusterAsyncContext *acc = valkeyClusterAsyncContextInit();
+    struct event_base *base = event_base_new();
+
+    valkeyClusterOptions options = {0};
+    options.initial_nodes = CLUSTER_NODE;
+    options.async_connect_callback = connectCallback;
+    options.async_disconnect_callback = disconnectCallback;
+    valkeyClusterOptionsUseLibevent(&options, base);
+
+    valkeyClusterAsyncContext *acc = valkeyClusterAsyncContextInit(&options);
     assert(acc);
 
     int status;
-    status = valkeyClusterAsyncSetConnectCallback(acc, connectCallback);
-    assert(status == VALKEY_OK);
-    status = valkeyClusterAsyncSetDisconnectCallback(acc, disconnectCallback);
-    assert(status == VALKEY_OK);
-    status = valkeyClusterSetEventCallback(acc->cc, eventCallback, acc);
-    assert(status == VALKEY_OK);
-    status = valkeyClusterSetOptionAddNodes(acc->cc, CLUSTER_NODE);
-    assert(status == VALKEY_OK);
-
-    struct event_base *base = event_base_new();
-    status = valkeyClusterLibeventAttach(acc, base);
+    status = valkeyClusterAsyncSetEventCallback(acc, eventCallback, acc);
     assert(status == VALKEY_OK);
 
-    status = valkeyClusterAsyncConnect2(acc);
+    status = valkeyClusterAsyncConnect(acc);
     assert(status == VALKEY_OK);
 
     event_base_dispatch(base);

examples/cluster-clientside-caching-async.c

@@ -8,11 +8,11 @@ int main(int argc, char **argv) {
     UNUSED(argv);
     struct timeval timeout = {1, 500000}; // 1.5s
 
-    valkeyClusterContext *cc = valkeyClusterContextInit();
-    valkeyClusterSetOptionAddNodes(cc, "127.0.0.1:7000");
-    valkeyClusterSetOptionConnectTimeout(cc, timeout);
-    valkeyClusterSetOptionRouteUseSlots(cc);
-    valkeyClusterConnect2(cc);
+    valkeyClusterOptions options = {0};
+    options.initial_nodes = "127.0.0.1:7000";
+    options.connect_timeout = &timeout;
+
+    valkeyClusterContext *cc = valkeyClusterConnectWithOptions(&options);
     if (!cc) {
         printf("Error: Allocation failure\n");
         exit(-1);