Hold table locks when needed during analyze

db/db_metrics.c

@@ -441,12 +441,12 @@ static int64_t refresh_diskspace(struct dbenv *dbenv, tran_type *tran)
     for(ndb = 0; ndb < dbenv->num_dbs; ndb++)
     {
         db = dbenv->dbs[ndb];
-        total += calc_table_size_tran(tran, db, 0);
+        total += calc_table_size(db, 0);
     }
     for(ndb = 0; ndb < dbenv->num_qdbs; ndb++)
     {
         db = dbenv->qdbs[ndb];
-        total += calc_table_size_tran(tran, db, 0);
+        total += calc_table_size(db, 0);
     }
     total += bdb_logs_size(dbenv->bdb_env, &num_logs);
 

db/db_tunables.c

@@ -186,6 +186,7 @@ extern int gbl_incoherent_logput_window;
 extern int gbl_dump_net_queue_on_partial_write;
 extern int gbl_dump_full_net_queue;
 extern int gbl_debug_partial_write;
+extern int gbl_debug_sleep_on_analyze;
 extern int gbl_debug_sleep_on_verify;
 extern int gbl_max_clientstats_cache;
 extern int gbl_decoupled_logputs;

db/db_tunables.h

@@ -2360,6 +2360,10 @@ REGISTER_TUNABLE("test_fdb_io", "Testing fail mode remote sql.  (Default: off)",
                  TUNABLE_BOOLEAN, &gbl_test_io_errors, INTERNAL, NULL, NULL,
                  NULL, NULL);
 
+
+REGISTER_TUNABLE("debug_sleep_on_analyze", "Sleep for 'n' seconds after releasing views lock in partition analyze (Default: 0)",
+                 TUNABLE_INTEGER, &gbl_debug_sleep_on_analyze, INTERNAL, NULL, NULL, NULL, NULL);
+
 REGISTER_TUNABLE("debug_sleep_in_sql_tick", "Sleep for a second in sql tick.  (Default: off)", TUNABLE_BOOLEAN,
                  &gbl_debug_sleep_in_sql_tick, INTERNAL | EXPERIMENTAL, NULL, NULL, NULL, NULL);
 

db/glue.c

@@ -5572,7 +5572,7 @@ void start_exclusive_backend_request(struct dbenv *env)
 
 void end_backend_request(struct dbenv *env) { bdb_end_request(env->bdb_env); }
 
-uint64_t calc_table_size_tran(tran_type *tran, struct dbtable *db, int skip_blobs)
+uint64_t calc_table_size(struct dbtable *db, int skip_blobs)
 {
     int ii;
     uint64_t size_without_blobs = 0;
@@ -5605,11 +5605,6 @@ uint64_t calc_table_size_tran(tran_type *tran, struct dbtable *db, int skip_blob
         return db->totalsize;
 }
 
-uint64_t calc_table_size(struct dbtable *db, int skip_blobs)
-{
-    return calc_table_size_tran(NULL, db, skip_blobs);
-}
-
 void compr_print_stats()
 {
     int ii;

db/sql.h

@@ -1520,6 +1520,9 @@ void add_fingerprint_to_rawstats(struct rawnodestats *stats,
  */
 int clnt_check_bdb_lock_desired(struct sqlclntstate *clnt);
 
+tran_type *get_read_only_tran();
+int put_read_only_tran(tran_type * const tran);
+
 /**
  * Bdb transaction objects with curtran lockid
  */

db/sqlanalyze.c

@@ -46,6 +46,8 @@
 static int analyze_thread_memory = 1048576;
 #endif
 
+int gbl_debug_sleep_on_analyze = 0;
+
 extern void reset_aa_counter(char *tblname);
 
 /* global is-running flag */
@@ -759,20 +761,49 @@ static int analyze_rename_table_for_backup_stats(char *table, struct sqlclntstat
 
 int analyze_regular_table(table_descriptor_t *td, struct sqlclntstate *clnt, char *zErrTab, size_t nErrTab) {
     int rc = 0;
+    int have_schema_lk = 0;
     int sampled_table = 0;
-    struct dbtable *table = get_dbtable_by_name(td->table);
-    rc = analyze_rename_table_for_backup_stats(td->table, clnt, zErrTab);
+    tran_type * tran = NULL;
 
-    if (rc) 
+    rc = analyze_rename_table_for_backup_stats(td->table, clnt, zErrTab);
+    if (rc) {
+        logmsg(LOGMSG_ERROR, "analyze_rename_table_for_backup_stats failed with rc %d\n", rc);
         goto err;
-
-    /* grab the size of the table */
-    int64_t totsiz = calc_table_size(table, 1);
+    }
 
     if (sampled_tables_enabled)
         get_sampling_threshold(td->table, &sampling_threshold);
 
-    /* sample if enabled */
+    rdlock_schema_lk();
+    have_schema_lk = 1;
+
+    struct dbtable * const table = get_dbtable_by_name(td->table);
+    if (!table) {
+        logmsg(LOGMSG_ERROR, "%s: Cannot find table %s\n", __func__, td->table);
+        rc = 1;
+        goto err;
+    }
+
+    tran = get_read_only_tran();
+    if (!tran) {
+        logmsg(LOGMSG_ERROR, "%s: Could not get tran\n", __func__);
+        rc = 1;
+        goto err;
+    }
+
+    rc = bdb_lock_tablename_read(table->handle, table->tablename, tran);
+    if (rc) {
+        logmsg(LOGMSG_ERROR, "%s: Failed to acquire read lock on %s\n", __func__,
+               table->tablename);
+        rc = 1;
+        goto err;
+    }
+
+    unlock_schema_lk();
+    have_schema_lk = 0;
+
+    const int64_t totsiz = calc_table_size(table, 1);
+
     if (sampled_tables_enabled) {
         if (totsiz <= sampling_threshold) {
             td->scale = 100;
@@ -787,8 +818,19 @@ int analyze_regular_table(table_descriptor_t *td, struct sqlclntstate *clnt, cha
         }
     }
 
+    rc = put_read_only_tran(tran);
+    tran = NULL;
+    if (rc) {
+        logmsg(LOGMSG_ERROR, "%s: Failed to put tran rc(%d)\n", __func__, rc);
+        goto err;
+    }
+
     clnt->is_analyze = 1;
 
+    if (gbl_debug_sleep_on_analyze) {
+        sleep(gbl_debug_sleep_on_analyze);
+    }
+
     /* run analyze as sql query */
     char *sql = sqlite3_mprintf("analyzesqlite main.\"%w\"", td->table);
     assert(sql != NULL);
@@ -803,6 +845,12 @@ int analyze_regular_table(table_descriptor_t *td, struct sqlclntstate *clnt, cha
     if (debug_switch_test_delay_analyze_commit())
         sleep(10);
 err:
+    if (have_schema_lk) {
+        unlock_schema_lk();
+    }
+    if (tran) {
+        put_read_only_tran(tran);
+    }
     if (sampled_table) {
         cleanup_sampled_indicies(clnt);
     }

db/sqlglue.c

@@ -11790,6 +11790,40 @@ void curtran_puttran(tran_type *tran)
     bdb_tran_abort(thedb->bdb_env, tran, &bdberr);
 }
 
+tran_type *get_read_only_tran()
+{
+    // Get curtran if we can because it is more efficient.
+    // If we can't, then just get a normal transaction.
+
+    tran_type * tran = curtran_gettran();
+    if (tran) { return tran; }
+
+    int bdberr = 0;
+    tran = bdb_tran_begin(thedb->bdb_env, NULL, &bdberr);
+    if (!tran) {
+        logmsg(LOGMSG_ERROR, "%s: bdb_tran_begin failed bdberr(%d)\n", __func__, bdberr);
+    }
+    return tran;
+}
+
+int put_read_only_tran(tran_type * const tran)
+{
+    // Aborts the transaction: Since the transaction is read-only,
+    // we don't need to create a commit record.
+
+    if (tran->is_curtran) {
+        curtran_puttran(tran);
+        return 0;
+    }
+
+    int bdberr;
+    const int rc = bdb_tran_abort(thedb->bdb_env, tran, &bdberr);
+    if (rc) {
+        logmsg(LOGMSG_ERROR, "%s: bdb_tran_abort failed rc(%d) bdberr(%d)\n", __func__, rc, bdberr);
+    }
+    return rc;
+}
+
 void clone_temp_table(sqlite3_stmt *stmt, struct temptable *tbl)
 {
     int rc;

tests/analyze.test/t17.req.out

@@ -0,0 +1 @@
+[analyze t] failed with rc -5 Analyze could not run because of internal problems

tests/analyze.test/t17.req.tool

@@ -0,0 +1 @@
+t17.sh

tests/analyze.test/t17.sh

@@ -0,0 +1,33 @@
+#!/bin/bash
+
+source ${TESTSROOTDIR}/tools/runit_common.sh
+
+set -e
+
+main() {
+	# Given
+	cdb2sql ${CDB2_OPTIONS} ${DBNAME} default "create table t(i int)" > /dev/null
+	cdb2sql ${CDB2_OPTIONS} ${DBNAME} default "insert into t values(1)" > /dev/null
+	sendtocluster "exec procedure sys.cmd.send('debug_sleep_on_analyze 5')" > /dev/null
+
+	# When
+	cdb2sql ${CDB2_OPTIONS} ${DBNAME} default "analyze t" &
+	waitpid=$!
+	sleep 1
+	cdb2sql ${CDB2_OPTIONS} ${DBNAME} default "drop table t" > /dev/null
+
+	# Then
+	if ! cdb2sql ${CDB2_OPTIONS} ${DBNAME} default "select 1" > /dev/null;
+	then
+		echo "Database not reachable"
+		return 1
+	fi
+
+	if wait ${waitpid};
+	then
+		echo "Expected to fail analyze but passed"
+		return 1
+	fi
+}
+
+main