{184305174}: Close displaced DB_AM_RECOVER handles in __ufid_add_dbp

berkdb/dbreg/dbreg_rec.c

@@ -61,6 +61,8 @@ static const char revid[] = "$Id: dbreg_rec.c,v 11.120 2003/10/27 15:54:31 sue E
 static int __dbreg_open_file __P((DB_ENV *,
     DB_TXN *, __dbreg_register_args *, void *));
 
+int gbl_test_force_dbreg_reopen = 0;
+
 int
 __dbreg_register_print(DB_ENV *dbenv, DBT *dbtp, DB_LSN *lsnp, db_recops notused2, void *notused3);
 
@@ -427,6 +429,12 @@ __dbreg_open_file(dbenv, txn, argp, info)
 			goto reopen;
 		}
 
+		if (gbl_test_force_dbreg_reopen) {
+			MUTEX_THREAD_UNLOCK(dbenv, lp->mutexp);
+			(void)__dbreg_revoke_id(dbp, 0, DB_LOGFILEID_INVALID);
+			goto reopen;
+		}
+
 		/*
 		 * We should only get here if we already have the
 		 * dbp from an openfiles pass, in which case, what's

berkdb/dbreg/dbreg_util.c

@@ -456,7 +456,20 @@ __ufid_add_dbp(dbenv, dbp)
 			   master and later assigns a dbreg ID to a btree (see __dbreg_lazy_id).
 			   In this case, we need to take the old dbp off the hashtable so that
 			   __db_close() doesn't mistakenly clear the new dbp. */
-			ufid->dbp->added_to_ufid = 0;
+			DB *old_dbp = ufid->dbp;
+			old_dbp->added_to_ufid = 0;
+			if (F_ISSET(old_dbp, DB_AM_RECOVER)) {
+				/* Close the displaced DB_AM_RECOVER handle to prevent file
+				 * descriptor leaks. These handles accumulate when a regular
+				 * handle displaces a recovery handle in the ufid-hash without
+				 * closing it. When delfiles later deletes these old file
+				 * versions, the leaked handles keep the inodes alive. */
+				ufid->dbp = dbp;
+				dbp->added_to_ufid = 1;
+				Pthread_mutex_unlock(&dbenv->ufid_to_db_lk);
+				__db_close(old_dbp, NULL, DB_NOSYNC);
+				return ret;
+			}
 		}
 	} else {
 		if ((ret = __os_malloc(dbenv, sizeof(*ufid), &ufid)) != 0) {

db/db_tunables.c

@@ -648,6 +648,7 @@ extern int gbl_accept_headroom;
 extern int gbl_db_track_open;
 extern int gbl_clear_ufid_on_db_close;
 extern int gbl_get_peer_fqdn;
+extern int gbl_test_force_dbreg_reopen;
 
 int parse_int64(const char *value, int64_t *num);
 

db/db_tunables.h

@@ -2689,4 +2689,6 @@ REGISTER_TUNABLE("clear_ufid_on_db_close", "Clear ufid hash on db->close", TUNAB
                  INTERNAL, NULL, NULL, NULL, NULL);
 REGISTER_TUNABLE("get_peer_fqdn", "When set, use peer's FQDN", TUNABLE_BOOLEAN, &gbl_get_peer_fqdn, INTERNAL, NULL,
                  NULL, NULL, NULL);
+REGISTER_TUNABLE("test_force_dbreg_reopen", "Force dbreg reopen to test ufid displacement", TUNABLE_INTEGER,
+                 &gbl_test_force_dbreg_reopen, INTERNAL, NULL, NULL, NULL, NULL);
 #endif /* _DB_TUNABLES_H */

tests/berkdb_file_leaks_rep.test/runit

@@ -24,7 +24,7 @@ dbnm=$1
 
 master=`cdb2sql --tabs ${CDB2_OPTIONS} $dbnm default "select host from comdb2_cluster where is_master='Y'"`
 replicant=`cdb2sql --tabs ${CDB2_OPTIONS} $dbnm default "select comdb2_host()"`
-the_other_replicant=`cdb2sql --tabs ${CDB2_OPTIONS} $dbnm default "SELECT host FROM comdb2_cluster WHERE host <> '$master' AND host <> '$replicant'"`
+the_other_replicant=`cdb2sql --tabs ${CDB2_OPTIONS} $dbnm default "SELECT host FROM comdb2_cluster WHERE host <> '$master' AND host <> '$replicant' LIMIT 1"`
 
 cdb2sql ${CDB2_OPTIONS} $dbnm default "CREATE TABLE t1 (i integer, b blob)"
 sleep 2 # wait for the checkpoint thread to do its 1st checkpoint

tests/ufid_handle_leak.test/Makefile

@@ -0,0 +1 @@
+include $(TESTSROOTDIR)/testcase.mk

tests/ufid_handle_leak.test/lrl.options

@@ -0,0 +1,5 @@
+setattr CHECKPOINTTIME 9999999
+setattr MEMPTRICKLEPERCENT 10
+track_db_open 1
+test_force_dbreg_reopen 1
+dtastripe 8

tests/ufid_handle_leak.test/runit

@@ -0,0 +1,73 @@
+#!/usr/bin/env bash
+bash -n "$0" | exit 1
+
+# Test that DB_AM_RECOVER handles displaced from the ufid-hash by
+# __ufid_add_dbp are properly closed, preventing file descriptor leaks.
+#
+# Uses the test_force_dbreg_reopen tunable to force __dbreg_open_file to
+# skip its early-return optimization when a dbreg slot already has a
+# matching handle. This causes __dbreg_do_open to create a DB_AM_RECOVER
+# handle that displaces the existing handle in the ufid-hash — the exact
+# scenario that occurs in prod during schema change replay.
+#
+# Without the fix in __ufid_add_dbp, each displacement leaks a handle
+# (visible as growth in dumpopendbs). With the fix, displaced DB_AM_RECOVER
+# handles are closed and the count stays stable.
+
+source ${TESTSROOTDIR}/tools/cluster_utils.sh
+
+[ -z "${CLUSTER}" ] && { echo "Test requires a cluster"; exit 0; }
+
+dbnm=$1
+
+master=$(cdb2sql --tabs ${CDB2_OPTIONS} $dbnm default \
+    "SELECT host FROM comdb2_cluster WHERE is_master='Y'")
+replicant=$(cdb2sql --tabs ${CDB2_OPTIONS} $dbnm default \
+    "SELECT host FROM comdb2_cluster WHERE is_master='N' LIMIT 1")
+
+echo "master=$master replicant=$replicant"
+
+cdb2sql ${CDB2_OPTIONS} $dbnm default "CREATE TABLE t1 (i INT, b BLOB)"
+sleep 1
+cdb2sql ${CDB2_OPTIONS} $dbnm default "INSERT INTO t1 VALUES(1, x'DEADBEEF')"
+sleep 2
+
+# Measure BEFORE any explicit checkpoint. The test_force_dbreg_reopen
+# tunable hasn't fired yet because CHECKPOINTTIME is set very high.
+before=$(cdb2sql --tabs ${CDB2_OPTIONS} $dbnm --host $replicant \
+    "EXEC PROCEDURE sys.cmd.send('bdb dumpopendbs')" | grep -c 'XXX\.t1')
+echo "Open t1 handles on $replicant before checkpoint: $before"
+
+# Checkpoint generates dbreg_chkpnt log records. With test_force_dbreg_reopen,
+# the replicant's __dbreg_open_file skips its early return and calls
+# __dbreg_do_open, creating a DB_AM_RECOVER handle that displaces the
+# existing regular handle in the ufid-hash.
+cdb2sql $dbnm --host $master "EXEC PROCEDURE sys.cmd.send('bdb checkpoint')"
+sleep 2
+
+# TRUNCATE creates new btree files, generating more dbreg_register records.
+for i in $(seq 1 3); do
+    cdb2sql ${CDB2_OPTIONS} $dbnm default "TRUNCATE TABLE t1"
+    cdb2sql $dbnm --host $master "EXEC PROCEDURE sys.cmd.send('bdb checkpoint')"
+    sleep 2
+done
+
+after=$(cdb2sql --tabs ${CDB2_OPTIONS} $dbnm --host $replicant \
+    "EXEC PROCEDURE sys.cmd.send('bdb dumpopendbs')" | grep -c 'XXX\.t1')
+echo "Open t1 handles on $replicant after checkpoint+truncates: $after"
+
+# With the fix:    displaced DB_AM_RECOVER handles are closed → count stable
+# Without the fix: each checkpoint + truncate leaks handles → count grows
+threshold=$(( before + (before / 4) + 2 ))
+if [ "$after" -gt "$threshold" ]; then
+    echo "FAIL: handle count grew from $before to $after (threshold $threshold)" >&2
+    echo "Leaked DB_AM_RECOVER handles detected" >&2
+    echo "--- dumpopendbs t1 entries ---"
+    cdb2sql --tabs ${CDB2_OPTIONS} $dbnm --host $replicant \
+        "EXEC PROCEDURE sys.cmd.send('bdb dumpopendbs')" | grep 'XXX\.t1'
+    echo "--- end ---"
+    exit 1
+fi
+
+echo "PASS: handle count stable ($before → $after, threshold $threshold)"
+exit 0