revision-id: 4eb7a964f21d743d816d41870efd9133d4198469 (fb-prod201903-160-g4eb7a964f21) parent(s): 0a669d663cffceabcfd63e0b45fef9338e198a26 author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2019-09-09 17:56:03 +0300 message: Issue #790: MultiGet-based MRR: Check if the query has been killed The loop in mrr_fill_buffer() should have a check whether the query has been KILL-ed. --- .../suite/rocksdb/r/rocksdb_mrr_debug.result | 40 +++++++++++++++ mysql-test/suite/rocksdb/t/rocksdb_mrr_debug.test | 58 ++++++++++++++++++++++ storage/rocksdb/ha_rocksdb.cc | 19 +++++-- 3 files changed, 113 insertions(+), 4 deletions(-) diff --git a/mysql-test/suite/rocksdb/r/rocksdb_mrr_debug.result b/mysql-test/suite/rocksdb/r/rocksdb_mrr_debug.result new file mode 100644 index 00000000000..ff321b5d269 --- /dev/null +++ b/mysql-test/suite/rocksdb/r/rocksdb_mrr_debug.result @@ -0,0 +1,40 @@ +create table t0(a int); +insert into t0 values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9); +create table t1(a int); +insert into t1 select A.a + B.a* 10 + C.a * 100 from t0 A, t0 B, t0 C; +create table t2 ( +pk int primary key, +col1 int, +filler char(32) +) engine=rocksdb; +insert into t2 select a,a,a from t1; +set global rocksdb_force_flush_memtable_now=1; +set @save_optimizer_switch=@@optimizer_switch; +set optimizer_switch='mrr=on,mrr_cost_based=off,batched_key_access=on'; +explain +select * from t2,t0 where t2.pk=t0.a; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t0 ALL NULL NULL NULL NULL 10 Using where +1 SIMPLE t2 eq_ref PRIMARY PRIMARY 4 test.t0.a 1 Using join buffer (Batched Key Access) +select variable_value into @n_multiget_orig +from information_schema.global_status where variable_name='ROCKSDB_NUMBER_MULTIGET_GET'; +set debug_sync = 'rocksdb.mrr_fill_buffer.loop SIGNAL target_ready WAIT_FOR simulate_kill'; +select * from t2,t0 where t2.pk=t0.a; +connect con1,localhost,root,,; +set debug_sync = 'now WAIT_FOR target_ready'; +set @a= (select id from information_schema.processlist where state='debug sync point: rocksdb.mrr_fill_buffer.loop'); +kill query @a; +connect con2,localhost,root,,; +set debug_sync = 'now SIGNAL simulate_kill'; +connection con1; +connection default; +ERROR 70100: Query execution was interrupted +# Check that we didn't continue till the MultiGet() call. The following will +# return 0: +select variable_value into @n_multiget_new +from information_schema.global_status where variable_name='ROCKSDB_NUMBER_MULTIGET_GET'; +select @n_multiget_new - @n_multiget_orig; +@n_multiget_new - @n_multiget_orig +0 +set optimizer_switch=@save_optimizer_switch; +drop table t0,t1,t2; diff --git a/mysql-test/suite/rocksdb/t/rocksdb_mrr_debug.test b/mysql-test/suite/rocksdb/t/rocksdb_mrr_debug.test new file mode 100644 index 00000000000..f35e57b43da --- /dev/null +++ b/mysql-test/suite/rocksdb/t/rocksdb_mrr_debug.test @@ -0,0 +1,58 @@ +--source include/have_rocksdb.inc +--source include/have_debug_sync.inc + +--enable_connect_log + +create table t0(a int); +insert into t0 values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9); + +create table t1(a int); +insert into t1 select A.a + B.a* 10 + C.a * 100 from t0 A, t0 B, t0 C; + +create table t2 ( + pk int primary key, + col1 int, + filler char(32) +) engine=rocksdb; + +insert into t2 select a,a,a from t1; +set global rocksdb_force_flush_memtable_now=1; + +set @save_optimizer_switch=@@optimizer_switch; +set optimizer_switch='mrr=on,mrr_cost_based=off,batched_key_access=on'; + +explain +select * from t2,t0 where t2.pk=t0.a; + +select variable_value into @n_multiget_orig +from information_schema.global_status where variable_name='ROCKSDB_NUMBER_MULTIGET_GET'; + +set debug_sync = 'rocksdb.mrr_fill_buffer.loop SIGNAL target_ready WAIT_FOR simulate_kill'; +send select * from t2,t0 where t2.pk=t0.a; + +connect (con1,localhost,root,,); +set debug_sync = 'now WAIT_FOR target_ready'; +set @a= (select id from information_schema.processlist where state='debug sync point: rocksdb.mrr_fill_buffer.loop'); +send kill query @a; + +# Note: it seems, just KILL QUERY will already cause debug sync point wait to +# finish. Leave the signal anyway since it doesn't hurt +connect (con2,localhost,root,,); +set debug_sync = 'now SIGNAL simulate_kill'; + +connection con1; +reap; + +connection default; +--error ER_QUERY_INTERRUPTED +reap; + +--echo # Check that we didn't continue till the MultiGet() call. The following will +--echo # return 0: +select variable_value into @n_multiget_new +from information_schema.global_status where variable_name='ROCKSDB_NUMBER_MULTIGET_GET'; + +select @n_multiget_new - @n_multiget_orig; + +set optimizer_switch=@save_optimizer_switch; +drop table t0,t1,t2; diff --git a/storage/rocksdb/ha_rocksdb.cc b/storage/rocksdb/ha_rocksdb.cc index 3bc8b064e39..e490684e79b 100644 --- a/storage/rocksdb/ha_rocksdb.cc +++ b/storage/rocksdb/ha_rocksdb.cc @@ -15322,8 +15322,9 @@ int ha_rocksdb::multi_range_read_init(RANGE_SEQ_IF *seq, void *seq_init_param, mrr_rowid_reader = reader; mrr_n_rowids = SSIZE_MAX-1; // TODO: get rid of this } - mrr_fill_buffer(); - return 0; + + res = mrr_fill_buffer(); + return res; } uint ha_rocksdb::mrr_get_length_per_rec() { @@ -15359,7 +15360,7 @@ int ha_rocksdb::mrr_fill_buffer() { if (n_elements < 1) { DBUG_ASSERT(0); - return 1; // error + return HA_ERR_INTERNAL_ERROR; // error } // TODO: why are we allocating/de-allocating every time buffer is refilled? char *buf = (char *)mrr_buf.buffer; @@ -15380,6 +15381,11 @@ int ha_rocksdb::mrr_fill_buffer() { char *range_ptr; while ((key_size = mrr_rowid_reader->get_next_rowid((uchar*)buf, &range_ptr)) > 0 ) { + DEBUG_SYNC(table->in_use, "rocksdb.mrr_fill_buffer.loop"); + if (table->in_use->killed) { + return HA_ERR_QUERY_INTERRUPTED; + } + elem++; mrr_keys[elem] = rocksdb::Slice(buf, key_size); mrr_range_ptrs[elem] = range_ptr; @@ -15443,7 +15449,12 @@ int ha_rocksdb::multi_range_read_next(char **range_info) { mrr_free_rows(); return HA_ERR_END_OF_FILE; } - mrr_fill_buffer(); + int res; + + if ((res = mrr_fill_buffer())) { + return res; + } + if (!mrr_n_elements) { table->status = STATUS_NOT_FOUND; // not sure if this is necessary? return HA_ERR_END_OF_FILE;