Tweaking the current_connections metric

This patch excludes connections owned by either sockpool or the in-process cache from
the current_connections metric. This is one of the metrics that we use internally to
monitor database capacity. Since it is very little overhead for libevent based network
to maintain idle connections, it makes sense to exclude donated connections from the metric.

Signed-off-by: Rivers Zhang <hzhang320@bloomberg.net>

db/appsock_handler.c

@@ -55,6 +55,8 @@ char appsock_unknown_old[] = "-1 #unknown command\n";
 char appsock_unknown[] = "Error: -1 #unknown command\n";
 char appsock_supported[] = "supported\n";
 int32_t active_appsock_conns = 0;
+int32_t pooled_appsock_conns = 0; /* number of connections owned by sockpool or local-cache */
+int32_t appsock_conns = 0;
 int64_t gbl_denied_appsock_connection_count = 0;
 
 pthread_mutex_t appsock_conn_lk = PTHREAD_MUTEX_INITIALIZER;
@@ -113,6 +115,7 @@ void appsock_quick_stat(void)
 {
     logmsg(LOGMSG_USER, "num appsock connections %llu\n", total_appsock_conns);
     logmsg(LOGMSG_USER, "num active appsock connections %d\n", active_appsock_conns);
+    logmsg(LOGMSG_USER, "num pooled appsock connections %d\n", pooled_appsock_conns);
     logmsg(LOGMSG_USER, "num appsock commands    %llu\n", total_toks);
 }
 
@@ -352,8 +355,8 @@ void appsock_handler_start(struct dbenv *dbenv, SBUF2 *sb, int admin)
 
             logmsg(
                 LOGMSG_WARN,
-                "%s: Maximum appsock connection limit approaching (%d/%d).\n",
-                __func__, active_appsock_conns, max);
+                "%s: Maximum appsock connection limit approaching (%d/%d/%d).\n",
+                __func__, (active_appsock_conns - pooled_appsock_conns), active_appsock_conns, max);
             lastprint = now;
 
             if ((now - last_thread_dump_time) > 10) {

db/db_metrics.c

@@ -570,7 +570,8 @@ int refresh_metrics(void)
     stats.sql_cost = gbl_nsql_steps + gbl_nnewsql_steps;
     stats.sql_count = gbl_nsql + gbl_nnewsql;
     stats.sql_ssl_count = gbl_nnewsql_ssl;
-    stats.current_connections = net_get_num_current_non_appsock_accepts(thedb->handle_sibling) + active_appsock_conns;
+    stats.current_connections = net_get_num_current_non_appsock_accepts(thedb->handle_sibling) +
+                                (active_appsock_conns - pooled_appsock_conns);
 
     rc = bdb_get_lock_counters(thedb->bdb_env, &stats.deadlocks,
                                &stats.locks_aborted, &stats.lockwaits,

db/sqlinterfaces.c

@@ -6065,6 +6065,10 @@ static const char* connstate_str(enum connection_state s) {
 void clnt_change_state(struct sqlclntstate *clnt, enum connection_state state) {
     clnt->state_start_time = comdb2_time_epochms();
     Pthread_mutex_lock(&clnt->state_lk);
+    if (clnt->state == CONNECTION_RESET && state != CONNECTION_RESET)
+        ATOMIC_ADD32(pooled_appsock_conns, -1);
+    else if (clnt->state != CONNECTION_RESET && state == CONNECTION_RESET)
+        ATOMIC_ADD32(pooled_appsock_conns, 1);
     clnt->state = state;
     Pthread_mutex_unlock(&clnt->state_lk);
 }

net/net_appsock.h

@@ -36,6 +36,7 @@ typedef void(*run_on_base_fn)(void *);
 void run_on_base(struct event_base *, run_on_base_fn, void *);
 
 extern int32_t active_appsock_conns;
+extern int32_t pooled_appsock_conns;
 extern int64_t gbl_denied_appsock_connection_count;
 extern int gbl_libevent_appsock;
 

plugins/newsql/newsql_evbuffer.c

@@ -114,6 +114,8 @@ static void free_newsql_appdata_evbuffer(int dummyfd, short what, void *arg)
 {
     struct newsql_appdata_evbuffer *appdata = arg;
     struct sqlclntstate *clnt = &appdata->clnt;
+    if (clnt->state == CONNECTION_RESET)
+        ATOMIC_ADD32(pooled_appsock_conns, -1);
     int fd = appdata->fd;
     rem_sql_evbuffer(clnt);
     rem_appsock_connection_evbuffer(clnt);

tests/comdb2sys.test/runit

@@ -128,6 +128,11 @@ rc2=$?
 
 ./testtransactionstate $a_dbn
 rc3=$?
+
+./test_current_connections $a_dbn
+rc4=$?
+
+[[ $rc4 -ne 0 ]] && rc3=$rc4
 [[ $rc3 -ne 0 ]] && rc2=$rc3 
 [[ $rc2 -ne 0 ]] && rc=$rc2
 

tests/comdb2sys.test/test_current_connections

@@ -0,0 +1,30 @@
+#!/usr/bin/env bash
+
+DB=$1
+default=default
+cdb2sql=cdb2sql
+set -e
+
+hostflags=
+
+if [[ -n "$CLUSTER" ]]; then
+    read -ra nodes <<< "$CLUSTER"
+    hostflags="--host ${nodes[0]}"
+fi
+
+
+sql='SELECT SLEEP(2)'
+for i in $(seq 1 10); do
+    $cdb2sql ${CDB2_OPTIONS} $hostflags $DB $default "$sql" &
+done
+
+wait
+
+n=`$cdb2sql --tabs ${CDB2_OPTIONS} $hostflags $DB $default "SELECT value FROM comdb2_metrics WHERE name='current_connections'"`
+if [[ $n -ne 1 ]]; then
+    echo current_connections $n >&2
+    exit 1
+fi
+
+echo $(basename $0) success
+exit 0