Skip to content

Commit 3016acf

Browse files
morgandomorgando
committed
Hold table locks when needed during analyze
Signed-off-by: Morgan Douglas <[email protected]> Update test Signed-off-by: mdouglas47 <[email protected]> Add to impl Signed-off-by: mdouglas47 <[email protected]> Update Signed-off-by: mdouglas47 <[email protected]> Tweak Signed-off-by: mdouglas47 <[email protected]> Tweak Signed-off-by: mdouglas47 <[email protected]> Change name of tunable Signed-off-by: mdouglas47 <[email protected]> Tweak Signed-off-by: mdouglas47 <[email protected]> Tweak Signed-off-by: mdouglas47 <[email protected]> Tweak Signed-off-by: mdouglas47 <[email protected]> Revert Signed-off-by: mdouglas47 <[email protected]> Tweak Signed-off-by: mdouglas47 <[email protected]> Tweak Signed-off-by: mdouglas47 <[email protected]> Tweak Signed-off-by: mdouglas47 <[email protected]> Add expected output for test Signed-off-by: mdouglas47 <[email protected]> Use curtran Signed-off-by: mdouglas47 <[email protected]> Only use curtran if possible Signed-off-by: mdouglas47 <[email protected]> Tweak Signed-off-by: mdouglas47 <[email protected]> Tweak Signed-off-by: mdouglas47 <[email protected]> Consistent naming Signed-off-by: mdouglas47 <[email protected]> Use existing function Signed-off-by: mdouglas47 <[email protected]> Tweak Signed-off-by: mdouglas47 <[email protected]>
1 parent 86a382a commit 3016acf

File tree

11 files changed

+133
-15
lines changed

11 files changed

+133
-15
lines changed

Diff for: ‎db/db_metrics.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -441,12 +441,12 @@ static int64_t refresh_diskspace(struct dbenv *dbenv, tran_type *tran)
441441
for(ndb = 0; ndb < dbenv->num_dbs; ndb++)
442442
{
443443
db = dbenv->dbs[ndb];
444-
total += calc_table_size_tran(tran, db, 0);
444+
total += calc_table_size(db, 0);
445445
}
446446
for(ndb = 0; ndb < dbenv->num_qdbs; ndb++)
447447
{
448448
db = dbenv->qdbs[ndb];
449-
total += calc_table_size_tran(tran, db, 0);
449+
total += calc_table_size(db, 0);
450450
}
451451
total += bdb_logs_size(dbenv->bdb_env, &num_logs);
452452

Diff for: ‎db/db_tunables.c

+1
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ extern int gbl_incoherent_logput_window;
186186
extern int gbl_dump_net_queue_on_partial_write;
187187
extern int gbl_dump_full_net_queue;
188188
extern int gbl_debug_partial_write;
189+
extern int gbl_debug_sleep_on_analyze;
189190
extern int gbl_debug_sleep_on_verify;
190191
extern int gbl_max_clientstats_cache;
191192
extern int gbl_decoupled_logputs;

Diff for: ‎db/db_tunables.h

+4
Original file line numberDiff line numberDiff line change
@@ -2355,6 +2355,10 @@ REGISTER_TUNABLE("test_fdb_io", "Testing fail mode remote sql. (Default: off)",
23552355
TUNABLE_BOOLEAN, &gbl_test_io_errors, INTERNAL, NULL, NULL,
23562356
NULL, NULL);
23572357

2358+
2359+
REGISTER_TUNABLE("debug_sleep_on_analyze", "Sleep for 'n' seconds after releasing views lock in partition analyze (Default: 0)",
2360+
TUNABLE_INTEGER, &gbl_debug_sleep_on_analyze, INTERNAL, NULL, NULL, NULL, NULL);
2361+
23582362
REGISTER_TUNABLE("debug_sleep_in_sql_tick", "Sleep for a second in sql tick. (Default: off)", TUNABLE_BOOLEAN,
23592363
&gbl_debug_sleep_in_sql_tick, INTERNAL | EXPERIMENTAL, NULL, NULL, NULL, NULL);
23602364

Diff for: ‎db/glue.c

+1-6
Original file line numberDiff line numberDiff line change
@@ -5572,7 +5572,7 @@ void start_exclusive_backend_request(struct dbenv *env)
55725572

55735573
void end_backend_request(struct dbenv *env) { bdb_end_request(env->bdb_env); }
55745574

5575-
uint64_t calc_table_size_tran(tran_type *tran, struct dbtable *db, int skip_blobs)
5575+
uint64_t calc_table_size(struct dbtable *db, int skip_blobs)
55765576
{
55775577
int ii;
55785578
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
56055605
return db->totalsize;
56065606
}
56075607

5608-
uint64_t calc_table_size(struct dbtable *db, int skip_blobs)
5609-
{
5610-
return calc_table_size_tran(NULL, db, skip_blobs);
5611-
}
5612-
56135608
void compr_print_stats()
56145609
{
56155610
int ii;

Diff for: ‎db/sql.h

+3
Original file line numberDiff line numberDiff line change
@@ -1520,6 +1520,9 @@ void add_fingerprint_to_rawstats(struct rawnodestats *stats,
15201520
*/
15211521
int clnt_check_bdb_lock_desired(struct sqlclntstate *clnt);
15221522

1523+
tran_type *get_read_only_tran();
1524+
int put_read_only_tran(tran_type * const tran);
1525+
15231526
/**
15241527
* Bdb transaction objects with curtran lockid
15251528
*/

Diff for: ‎db/sqlanalyze.c

+55-7
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@
4646
static int analyze_thread_memory = 1048576;
4747
#endif
4848

49+
int gbl_debug_sleep_on_analyze = 0;
50+
4951
extern void reset_aa_counter(char *tblname);
5052

5153
/* global is-running flag */
@@ -759,20 +761,49 @@ static int analyze_rename_table_for_backup_stats(char *table, struct sqlclntstat
759761

760762
int analyze_regular_table(table_descriptor_t *td, struct sqlclntstate *clnt, char *zErrTab, size_t nErrTab) {
761763
int rc = 0;
764+
int have_schema_lk = 0;
762765
int sampled_table = 0;
763-
struct dbtable *table = get_dbtable_by_name(td->table);
764-
rc = analyze_rename_table_for_backup_stats(td->table, clnt, zErrTab);
766+
tran_type * tran = NULL;
765767

766-
if (rc)
768+
rc = analyze_rename_table_for_backup_stats(td->table, clnt, zErrTab);
769+
if (rc) {
770+
logmsg(LOGMSG_ERROR, "analyze_rename_table_for_backup_stats failed with rc %d\n", rc);
767771
goto err;
768-
769-
/* grab the size of the table */
770-
int64_t totsiz = calc_table_size(table, 1);
772+
}
771773

772774
if (sampled_tables_enabled)
773775
get_sampling_threshold(td->table, &sampling_threshold);
774776

775-
/* sample if enabled */
777+
rdlock_schema_lk();
778+
have_schema_lk = 1;
779+
780+
struct dbtable * const table = get_dbtable_by_name(td->table);
781+
if (!table) {
782+
logmsg(LOGMSG_ERROR, "%s: Cannot find table %s\n", __func__, td->table);
783+
rc = 1;
784+
goto err;
785+
}
786+
787+
tran = get_read_only_tran();
788+
if (!tran) {
789+
logmsg(LOGMSG_ERROR, "%s: Could not get tran\n", __func__);
790+
rc = 1;
791+
goto err;
792+
}
793+
794+
rc = bdb_lock_tablename_read(table->handle, table->tablename, tran);
795+
if (rc) {
796+
logmsg(LOGMSG_ERROR, "%s: Failed to acquire read lock on %s\n", __func__,
797+
table->tablename);
798+
rc = 1;
799+
goto err;
800+
}
801+
802+
unlock_schema_lk();
803+
have_schema_lk = 0;
804+
805+
const int64_t totsiz = calc_table_size(table, 1);
806+
776807
if (sampled_tables_enabled) {
777808
if (totsiz <= sampling_threshold) {
778809
td->scale = 100;
@@ -787,8 +818,19 @@ int analyze_regular_table(table_descriptor_t *td, struct sqlclntstate *clnt, cha
787818
}
788819
}
789820

821+
rc = put_read_only_tran(tran);
822+
tran = NULL;
823+
if (rc) {
824+
logmsg(LOGMSG_ERROR, "%s: Failed to put tran rc(%d)\n", __func__, rc);
825+
goto err;
826+
}
827+
790828
clnt->is_analyze = 1;
791829

830+
if (gbl_debug_sleep_on_analyze) {
831+
sleep(gbl_debug_sleep_on_analyze);
832+
}
833+
792834
/* run analyze as sql query */
793835
char *sql = sqlite3_mprintf("analyzesqlite main.\"%w\"", td->table);
794836
assert(sql != NULL);
@@ -803,6 +845,12 @@ int analyze_regular_table(table_descriptor_t *td, struct sqlclntstate *clnt, cha
803845
if (debug_switch_test_delay_analyze_commit())
804846
sleep(10);
805847
err:
848+
if (have_schema_lk) {
849+
unlock_schema_lk();
850+
}
851+
if (tran) {
852+
put_read_only_tran(tran);
853+
}
806854
if (sampled_table) {
807855
cleanup_sampled_indicies(clnt);
808856
}

Diff for: ‎db/sqlglue.c

+34
Original file line numberDiff line numberDiff line change
@@ -11790,6 +11790,40 @@ void curtran_puttran(tran_type *tran)
1179011790
bdb_tran_abort(thedb->bdb_env, tran, &bdberr);
1179111791
}
1179211792

11793+
tran_type *get_read_only_tran()
11794+
{
11795+
// Get curtran if we can because it is more efficient.
11796+
// If we can't, then just get a normal transaction.
11797+
11798+
tran_type * tran = curtran_gettran();
11799+
if (tran) { return tran; }
11800+
11801+
int bdberr = 0;
11802+
tran = bdb_tran_begin(thedb->bdb_env, NULL, &bdberr);
11803+
if (!tran) {
11804+
logmsg(LOGMSG_ERROR, "%s: bdb_tran_begin failed bdberr(%d)\n", __func__, bdberr);
11805+
}
11806+
return tran;
11807+
}
11808+
11809+
int put_read_only_tran(tran_type * const tran)
11810+
{
11811+
// Aborts the transaction: Since the transaction is read-only,
11812+
// we don't need to create a commit record.
11813+
11814+
if (tran->is_curtran) {
11815+
curtran_puttran(tran);
11816+
return 0;
11817+
}
11818+
11819+
int bdberr;
11820+
const int rc = bdb_tran_abort(thedb->bdb_env, tran, &bdberr);
11821+
if (rc) {
11822+
logmsg(LOGMSG_ERROR, "%s: bdb_tran_abort failed rc(%d) bdberr(%d)\n", __func__, rc, bdberr);
11823+
}
11824+
return rc;
11825+
}
11826+
1179311827
void clone_temp_table(sqlite3_stmt *stmt, struct temptable *tbl)
1179411828
{
1179511829
int rc;

Diff for: ‎tests/analyze.test/t17.req

Whitespace-only changes.

Diff for: ‎tests/analyze.test/t17.req.out

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

Diff for: ‎tests/analyze.test/t17.req.tool

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
t17.sh

Diff for: ‎tests/analyze.test/t17.sh

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
#!/bin/bash
2+
3+
set -e
4+
5+
main() {
6+
# Given
7+
cdb2sql ${CDB2_OPTIONS} ${DBNAME} default "create table t(i int)" > /dev/null
8+
cdb2sql ${CDB2_OPTIONS} ${DBNAME} default "insert into t values(1)" > /dev/null
9+
sendtocluster "exec procedure sys.cmd.send('debug_sleep_on_analyze 5')" > /dev/null
10+
11+
# When
12+
cdb2sql ${CDB2_OPTIONS} ${DBNAME} default "analyze t" &
13+
waitpid=$!
14+
sleep 1
15+
cdb2sql ${CDB2_OPTIONS} ${DBNAME} default "drop table t" > /dev/null
16+
17+
# Then
18+
if ! cdb2sql ${CDB2_OPTIONS} ${DBNAME} default "select 1" > /dev/null;
19+
then
20+
echo "Database not reachable"
21+
return 1
22+
fi
23+
24+
if wait ${waitpid};
25+
then
26+
echo "Expected to fail analyze but passed"
27+
return 1
28+
fi
29+
}
30+
31+
main

0 commit comments

Comments
 (0)