øMerge branch 'keydbpro' into merge_os_apr_13

keydb.conf

@@ -2070,3 +2070,12 @@ server-threads 2
 #
 # By default KeyDB sets this to 2.
 replica-weighting-factor 2
+
+# Should KeyDB make active attempts at balancing clients across threads?  This can impact
+# performance accepting new clients.  By default this is enabled.  If disabled there is still
+# a best effort from the kernel to distribute across threads with SO_REUSEPORT but it will not
+# be as fair.
+#
+# By default this is enabled
+#
+active-client-balancing yes

src/config.cpp

@@ -2779,6 +2779,7 @@ standardConfig configs[] = {
     createBoolConfig("replica-announced", NULL, MODIFIABLE_CONFIG, g_pserver->replica_announced, 1, NULL, NULL),
     createBoolConfig("enable-async-commands", NULL, MODIFIABLE_CONFIG, g_pserver->enable_async_commands, 1, NULL, NULL),
     createBoolConfig("multithread-load-enabled", NULL, MODIFIABLE_CONFIG, g_pserver->multithread_load_enabled, 0, NULL, NULL),
+    createBoolConfig("active-client-balancing", NULL, MODIFIABLE_CONFIG, g_pserver->active_client_balancing, 1, NULL, NULL),
 
     /* String Configs */
     createStringConfig("aclfile", NULL, IMMUTABLE_CONFIG, ALLOW_EMPTY_STRING, g_pserver->acl_filename, "", NULL, NULL),

src/networking.cpp

@@ -1319,7 +1319,7 @@ void acceptOnThread(connection *conn, int flags, char *cip)
     int ielCur = ielFromEventLoop(serverTL->el);
     bool fBootLoad = (g_pserver->loading == LOADING_BOOT);
 
-    int ielTarget = 0;
+    int ielTarget = ielCur;
     if (fBootLoad)
     {
         ielTarget = IDX_EVENT_LOOP_MAIN;    // During load only the main thread is active
@@ -1330,7 +1330,7 @@ void acceptOnThread(connection *conn, int flags, char *cip)
         while (cserver.cthreads > 1 && ielTarget == IDX_EVENT_LOOP_MAIN)
             ielTarget = rand() % cserver.cthreads;
     }
-    else
+    else if (g_pserver->active_client_balancing)
     {
         // Cluster connections are more transient, so its not worth the cost to balance
         //  we can trust that SO_REUSEPORT is doing its job of distributing connections
@@ -1405,6 +1405,8 @@ void acceptTLSHandler(aeEventLoop *el, int fd, void *privdata, int mask) {
         serverLog(LL_VERBOSE,"Accepted %s:%d", cip, cport);
 
         acceptOnThread(connCreateAcceptedTLS(cfd, g_pserver->tls_auth_clients), 0, cip);
+        if (aeLockContention() >= 2)
+            break;
     }
 }
 

src/server.cpp

@@ -3772,7 +3772,7 @@ static void initNetworkingThread(int iel, int fReusePort)
     makeThreadKillable();
 
     for (int j = 0; j < g_pserver->rgthreadvar[iel].tlsfd.count; j++) {
-        if (aeCreateFileEvent(g_pserver->rgthreadvar[iel].el, g_pserver->rgthreadvar[iel].tlsfd.fd[j], AE_READABLE,
+        if (aeCreateFileEvent(g_pserver->rgthreadvar[iel].el, g_pserver->rgthreadvar[iel].tlsfd.fd[j], AE_READABLE|AE_READ_THREADSAFE,
             acceptTLSHandler,NULL) == AE_ERR)
             {
                 serverPanic(

src/server.h

@@ -2696,6 +2696,7 @@ struct redisServer {
 
     int enable_async_commands;
     int multithread_load_enabled = 0;
+    int active_client_balancing = 1;
 
     long long repl_batch_offStart = -1;
     long long repl_batch_idxStart = -1;

src/tls.cpp

@@ -491,6 +491,12 @@ bool tlsCheckAgainstAllowlist(const char * client){
     return false;
 }
 
+/* ASN1_STRING_get0_data was introduced in OPENSSL 1.1.1
+ * use ASN1_STRING_data for older versions where it is not available */
+#if OPENSSL_VERSION_NUMBER < 0x10100000L
+#define ASN1_STRING_get0_data ASN1_STRING_data 
+#endif 
+
 bool tlsValidateCertificateName(tls_connection* conn){
     if (g_pserver->tls_allowlist.empty())
         return true;    // Empty list implies acceptance of all

tests/unit/tls-name-validation.tcl

@@ -1,3 +1,6 @@
+# only run this test if tls is enabled
+if {$::tls} {
+    package require tls
     test {TLS: Able to connect with no allowlist} {
         start_server {tags {"tls"}} {
             catch {r PING} e
@@ -109,3 +112,9 @@ test {TLS: Able to match against URI SAN} {
             assert_match {PONG} $e
         }
     }
+} else {
+    start_server {} {
+        # just a dummy server so that the test doesn't panic if tls is disabled
+        # otherwise the test will try to bind to a server that just isn't there
+    }
+}