Skip to content

Commit 8efbe05

Browse files
Luqun Louinikep
Luqun Lou
authored and
inikep
committed
check upper and lower bound for writebatchwithindex
Upstream commit ID: facebook/mysql-5.6@0bfdb05 PS-8951: Merge percona-202305 (https://jira.percona.com/browse/PS-8951) Summary: rocksdb use Writebatchwithindex(WBWI) to support read your own data. But there are two issues for implementations: 1. facebook/rocksdb#11606: Rocksdb may return deleted row or out of range row during iterating WBWI, see code https://github.com/facebook/rocksdb/blob/main/utilities/write_batch_with_index/write_batch_with_index_internal.cc#L311, when it return due out of range(bound), its DeltaIterator may still valid but point to out of range row. 2. facebook/rocksdb#11607: it doesn't check lower_bound_ even lower_bound_ values is passed to rocksdb with readoptions. see https://github.com/facebook/rocksdb/blob/main/utilities/write_batch_with_index/write_batch_with_index_internal.cc#L306 To workaround these issue, add a variable rocksdb_check_iterate_bounds to control whether we should check iterate bounds and check these bounds inside myrocks if rocksdb_check_iterate_bounds is true. Differential Revision: D46908478 fbshipit-source-id: 765f562928a3ad117d23a177b1b2d9e551b0c0ae
1 parent 6c75f02 commit 8efbe05

9 files changed

Lines changed: 247 additions & 4 deletions

File tree

‎mysql-test/suite/rocksdb/r/iterator_bounds.result‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,43 @@ select j from t order by j desc;
1313
j
1414
1
1515
drop table t;
16+
create table t(
17+
a int unsigned not null,
18+
b int unsigned not null,
19+
c varchar(64) not null COLLATE utf8_bin,
20+
primary key(a),
21+
key(b,c)
22+
) DEFAULT CHARSET=utf8 COLLATE=utf8_bin;
23+
Warnings:
24+
Warning 3778 'utf8mb3_bin' is a collation of the deprecated character set UTF8MB3. Please consider using UTF8MB4 with an appropriate collation instead.
25+
Warning 3719 'utf8' is currently an alias for the character set UTF8MB3, but will be an alias for UTF8MB4 in a future release. Please consider using UTF8MB4 in order to be unambiguous.
26+
Warning 3778 'utf8mb3_bin' is a collation of the deprecated character set UTF8MB3. Please consider using UTF8MB4 with an appropriate collation instead.
27+
begin;
28+
insert into t values(101, 101, 'view_routine_usage');
29+
update t set b = 100 where b = 101 and c like 'view_routine_usage';
30+
update t set b = 101 where b = 100 and c like 'view_routine_usage';
31+
select a from t where b = 101 and c like 'view_routine_usage';
32+
a
33+
101
34+
rollback;
35+
drop table t;
36+
create table t(
37+
a int unsigned not null,
38+
b int unsigned not null,
39+
c varchar(64) not null COLLATE utf8_bin,
40+
primary key(a),
41+
key(b,c) comment 'rev:bc'
42+
) DEFAULT CHARSET=utf8 COLLATE=utf8_bin;
43+
Warnings:
44+
Warning 3778 'utf8mb3_bin' is a collation of the deprecated character set UTF8MB3. Please consider using UTF8MB4 with an appropriate collation instead.
45+
Warning 3719 'utf8' is currently an alias for the character set UTF8MB3, but will be an alias for UTF8MB4 in a future release. Please consider using UTF8MB4 in order to be unambiguous.
46+
Warning 3778 'utf8mb3_bin' is a collation of the deprecated character set UTF8MB3. Please consider using UTF8MB4 with an appropriate collation instead.
47+
begin;
48+
insert into t values(110, 110, 'view_routine_usage');
49+
insert into t values(100, 100, 'view_routine_usage');
50+
update t set b = 101 where b = 100 and c like 'view_routine_usage';
51+
update t set b = 100 where b = 101 and c like 'view_routine_usage';
52+
select a from t where b = 101 and c like 'view_routine_usage';
53+
a
54+
rollback;
55+
drop table t;

‎mysql-test/suite/rocksdb/r/rocksdb.result‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -921,6 +921,7 @@ rocksdb_cache_index_and_filter_blocks ON
921921
rocksdb_cache_index_and_filter_with_high_priority ON
922922
rocksdb_cancel_manual_compactions OFF
923923
rocksdb_charge_memory OFF
924+
rocksdb_check_iterate_bounds ON
924925
rocksdb_checksums_pct 100
925926
rocksdb_collect_sst_properties ON
926927
rocksdb_column_default_value_as_expression ON

‎mysql-test/suite/rocksdb/t/iterator_bounds.test‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,3 +28,56 @@ select j from t order by j asc;
2828
select j from t order by j desc;
2929

3030
drop table t;
31+
32+
#
33+
# check bounds for writebatch(forward cf), all data changes are in writebatch
34+
#
35+
create table t(
36+
a int unsigned not null,
37+
b int unsigned not null,
38+
c varchar(64) not null COLLATE utf8_bin,
39+
primary key(a),
40+
key(b,c)
41+
) DEFAULT CHARSET=utf8 COLLATE=utf8_bin;
42+
43+
begin;
44+
insert into t values(101, 101, 'view_routine_usage');
45+
# SD(101, 'view_routine_usage',101)
46+
update t set b = 100 where b = 101 and c like 'view_routine_usage';
47+
# dring iterating, writebatchwithindex may return "out of range" record after
48+
# checking with upper bounds. sometimes the "out of range" record is a SD record.
49+
# For SD record, its value slice will be empty. Try to decode a key slice
50+
# which contains varchar with empty value slice, decoder will crash due missing
51+
# upack_info in value slice
52+
update t set b = 101 where b = 100 and c like 'view_routine_usage';
53+
select a from t where b = 101 and c like 'view_routine_usage';
54+
rollback;
55+
56+
drop table t;
57+
58+
59+
#
60+
# check bounds for writebatch(rev cf), all data changes are in writebatch
61+
#
62+
create table t(
63+
a int unsigned not null,
64+
b int unsigned not null,
65+
c varchar(64) not null COLLATE utf8_bin,
66+
primary key(a),
67+
key(b,c) comment 'rev:bc'
68+
) DEFAULT CHARSET=utf8 COLLATE=utf8_bin;
69+
70+
begin;
71+
insert into t values(110, 110, 'view_routine_usage');
72+
insert into t values(100, 100, 'view_routine_usage');
73+
# SD(100, 'view_routine_usage',100)
74+
update t set b = 101 where b = 100 and c like 'view_routine_usage';
75+
# during iterating, we don't check with lower bound in writebatchwithindex
76+
# in rev cf,
77+
update t set b = 100 where b = 101 and c like 'view_routine_usage';
78+
select a from t where b = 101 and c like 'view_routine_usage';
79+
rollback;
80+
81+
drop table t;
82+
83+

‎mysql-test/suite/rocksdb_sys_vars/r/rocksdb_check_iterate_bounds_basic.result‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
CREATE TABLE valid_values (value varchar(255)) ENGINE=myisam;
2+
INSERT INTO valid_values VALUES(1);
3+
INSERT INTO valid_values VALUES(0);
4+
INSERT INTO valid_values VALUES('on');
5+
CREATE TABLE invalid_values (value varchar(255)) ENGINE=myisam;
6+
INSERT INTO invalid_values VALUES('\'aaa\'');
7+
INSERT INTO invalid_values VALUES('\'bbb\'');
8+
SET @start_global_value = @@global.ROCKSDB_CHECK_ITERATE_BOUNDS;
9+
SELECT @start_global_value;
10+
@start_global_value
11+
1
12+
SET @start_session_value = @@session.ROCKSDB_CHECK_ITERATE_BOUNDS;
13+
SELECT @start_session_value;
14+
@start_session_value
15+
1
16+
'# Setting to valid values in global scope#'
17+
"Trying to set variable @@global.ROCKSDB_CHECK_ITERATE_BOUNDS to 1"
18+
SET @@global.ROCKSDB_CHECK_ITERATE_BOUNDS = 1;
19+
SELECT @@global.ROCKSDB_CHECK_ITERATE_BOUNDS;
20+
@@global.ROCKSDB_CHECK_ITERATE_BOUNDS
21+
1
22+
"Setting the global scope variable back to default"
23+
SET @@global.ROCKSDB_CHECK_ITERATE_BOUNDS = DEFAULT;
24+
SELECT @@global.ROCKSDB_CHECK_ITERATE_BOUNDS;
25+
@@global.ROCKSDB_CHECK_ITERATE_BOUNDS
26+
1
27+
"Trying to set variable @@global.ROCKSDB_CHECK_ITERATE_BOUNDS to 0"
28+
SET @@global.ROCKSDB_CHECK_ITERATE_BOUNDS = 0;
29+
SELECT @@global.ROCKSDB_CHECK_ITERATE_BOUNDS;
30+
@@global.ROCKSDB_CHECK_ITERATE_BOUNDS
31+
0
32+
"Setting the global scope variable back to default"
33+
SET @@global.ROCKSDB_CHECK_ITERATE_BOUNDS = DEFAULT;
34+
SELECT @@global.ROCKSDB_CHECK_ITERATE_BOUNDS;
35+
@@global.ROCKSDB_CHECK_ITERATE_BOUNDS
36+
1
37+
"Trying to set variable @@global.ROCKSDB_CHECK_ITERATE_BOUNDS to on"
38+
SET @@global.ROCKSDB_CHECK_ITERATE_BOUNDS = on;
39+
SELECT @@global.ROCKSDB_CHECK_ITERATE_BOUNDS;
40+
@@global.ROCKSDB_CHECK_ITERATE_BOUNDS
41+
1
42+
"Setting the global scope variable back to default"
43+
SET @@global.ROCKSDB_CHECK_ITERATE_BOUNDS = DEFAULT;
44+
SELECT @@global.ROCKSDB_CHECK_ITERATE_BOUNDS;
45+
@@global.ROCKSDB_CHECK_ITERATE_BOUNDS
46+
1
47+
'# Setting to valid values in session scope#'
48+
"Trying to set variable @@session.ROCKSDB_CHECK_ITERATE_BOUNDS to 1"
49+
SET @@session.ROCKSDB_CHECK_ITERATE_BOUNDS = 1;
50+
SELECT @@session.ROCKSDB_CHECK_ITERATE_BOUNDS;
51+
@@session.ROCKSDB_CHECK_ITERATE_BOUNDS
52+
1
53+
"Setting the session scope variable back to default"
54+
SET @@session.ROCKSDB_CHECK_ITERATE_BOUNDS = DEFAULT;
55+
SELECT @@session.ROCKSDB_CHECK_ITERATE_BOUNDS;
56+
@@session.ROCKSDB_CHECK_ITERATE_BOUNDS
57+
1
58+
"Trying to set variable @@session.ROCKSDB_CHECK_ITERATE_BOUNDS to 0"
59+
SET @@session.ROCKSDB_CHECK_ITERATE_BOUNDS = 0;
60+
SELECT @@session.ROCKSDB_CHECK_ITERATE_BOUNDS;
61+
@@session.ROCKSDB_CHECK_ITERATE_BOUNDS
62+
0
63+
"Setting the session scope variable back to default"
64+
SET @@session.ROCKSDB_CHECK_ITERATE_BOUNDS = DEFAULT;
65+
SELECT @@session.ROCKSDB_CHECK_ITERATE_BOUNDS;
66+
@@session.ROCKSDB_CHECK_ITERATE_BOUNDS
67+
1
68+
"Trying to set variable @@session.ROCKSDB_CHECK_ITERATE_BOUNDS to on"
69+
SET @@session.ROCKSDB_CHECK_ITERATE_BOUNDS = on;
70+
SELECT @@session.ROCKSDB_CHECK_ITERATE_BOUNDS;
71+
@@session.ROCKSDB_CHECK_ITERATE_BOUNDS
72+
1
73+
"Setting the session scope variable back to default"
74+
SET @@session.ROCKSDB_CHECK_ITERATE_BOUNDS = DEFAULT;
75+
SELECT @@session.ROCKSDB_CHECK_ITERATE_BOUNDS;
76+
@@session.ROCKSDB_CHECK_ITERATE_BOUNDS
77+
1
78+
'# Testing with invalid values in global scope #'
79+
"Trying to set variable @@global.ROCKSDB_CHECK_ITERATE_BOUNDS to 'aaa'"
80+
SET @@global.ROCKSDB_CHECK_ITERATE_BOUNDS = 'aaa';
81+
Got one of the listed errors
82+
SELECT @@global.ROCKSDB_CHECK_ITERATE_BOUNDS;
83+
@@global.ROCKSDB_CHECK_ITERATE_BOUNDS
84+
1
85+
"Trying to set variable @@global.ROCKSDB_CHECK_ITERATE_BOUNDS to 'bbb'"
86+
SET @@global.ROCKSDB_CHECK_ITERATE_BOUNDS = 'bbb';
87+
Got one of the listed errors
88+
SELECT @@global.ROCKSDB_CHECK_ITERATE_BOUNDS;
89+
@@global.ROCKSDB_CHECK_ITERATE_BOUNDS
90+
1
91+
SET @@global.ROCKSDB_CHECK_ITERATE_BOUNDS = @start_global_value;
92+
SELECT @@global.ROCKSDB_CHECK_ITERATE_BOUNDS;
93+
@@global.ROCKSDB_CHECK_ITERATE_BOUNDS
94+
1
95+
SET @@session.ROCKSDB_CHECK_ITERATE_BOUNDS = @start_session_value;
96+
SELECT @@session.ROCKSDB_CHECK_ITERATE_BOUNDS;
97+
@@session.ROCKSDB_CHECK_ITERATE_BOUNDS
98+
1
99+
DROP TABLE valid_values;
100+
DROP TABLE invalid_values;

‎mysql-test/suite/rocksdb_sys_vars/t/rocksdb_check_iterate_bounds_basic.test‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
--source include/have_rocksdb.inc
2+
3+
CREATE TABLE valid_values (value varchar(255)) ENGINE=myisam;
4+
INSERT INTO valid_values VALUES(1);
5+
INSERT INTO valid_values VALUES(0);
6+
INSERT INTO valid_values VALUES('on');
7+
8+
CREATE TABLE invalid_values (value varchar(255)) ENGINE=myisam;
9+
INSERT INTO invalid_values VALUES('\'aaa\'');
10+
INSERT INTO invalid_values VALUES('\'bbb\'');
11+
12+
--let $sys_var=ROCKSDB_CHECK_ITERATE_BOUNDS
13+
--let $read_only=0
14+
--let $session=1
15+
--source ../include/rocksdb_sys_var.inc
16+
17+
DROP TABLE valid_values;
18+
DROP TABLE invalid_values;

‎storage/rocksdb/ha_rocksdb.cc‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1360,6 +1360,11 @@ static MYSQL_THDVAR_BOOL(
13601360
"Enable rocksdb iterator upper/lower bounds in read options.", nullptr,
13611361
nullptr, true);
13621362

1363+
static MYSQL_THDVAR_BOOL(
1364+
check_iterate_bounds, PLUGIN_VAR_OPCMDARG,
1365+
"Check rocksdb iterator upper/lower bounds during iterating.", nullptr,
1366+
nullptr, true);
1367+
13631368
static const char *const DEFAULT_READ_FREE_RPL_TABLES = ".*";
13641369

13651370
static std::regex_constants::syntax_option_type get_regex_flags() {
@@ -2678,6 +2683,7 @@ static struct SYS_VAR *rocksdb_system_variables[] = {
26782683
MYSQL_SYSVAR(commit_in_the_middle),
26792684
MYSQL_SYSVAR(blind_delete_primary_key),
26802685
MYSQL_SYSVAR(enable_iterate_bounds),
2686+
MYSQL_SYSVAR(check_iterate_bounds),
26812687
#if defined(ROCKSDB_INCLUDE_RFR) && ROCKSDB_INCLUDE_RFR
26822688
MYSQL_SYSVAR(read_free_rpl_tables),
26832689
MYSQL_SYSVAR(read_free_rpl),
@@ -16071,11 +16077,13 @@ bool ha_rocksdb::can_assume_tracked(THD *thd) {
1607116077
bool ha_rocksdb::check_bloom_and_set_bounds(
1607216078
THD *thd, const Rdb_key_def &kd, const rocksdb::Slice &eq_cond,
1607316079
size_t bound_len, uchar *const lower_bound, uchar *const upper_bound,
16074-
rocksdb::Slice *lower_bound_slice, rocksdb::Slice *upper_bound_slice) {
16080+
rocksdb::Slice *lower_bound_slice, rocksdb::Slice *upper_bound_slice,
16081+
bool *check_iterate_bounds) {
1607516082
bool can_use_bloom = can_use_bloom_filter(thd, kd, eq_cond);
1607616083
if (!can_use_bloom && (THDVAR(thd, enable_iterate_bounds))) {
1607716084
setup_iterator_bounds(kd, eq_cond, bound_len, lower_bound, upper_bound,
1607816085
lower_bound_slice, upper_bound_slice);
16086+
*check_iterate_bounds = THDVAR(thd, check_iterate_bounds);
1607916087
}
1608016088
return can_use_bloom;
1608116089
}

‎storage/rocksdb/ha_rocksdb.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -649,7 +649,8 @@ class ha_rocksdb : public my_core::handler, public blob_buffer {
649649
static bool check_bloom_and_set_bounds(
650650
THD *thd, const Rdb_key_def &kd, const rocksdb::Slice &eq_cond,
651651
size_t bound_len, uchar *const lower_bound, uchar *const upper_bound,
652-
rocksdb::Slice *lower_bound_slice, rocksdb::Slice *upper_bound_slice);
652+
rocksdb::Slice *lower_bound_slice, rocksdb::Slice *upper_bound_slice,
653+
bool *check_iterate_bounds);
653654
static bool can_use_bloom_filter(THD *thd, const Rdb_key_def &kd,
654655
const rocksdb::Slice &eq_cond);
655656

‎storage/rocksdb/rdb_iterator.cc‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,8 @@ Rdb_iterator_base::Rdb_iterator_base(THD *thd,
4040
m_scan_it_snapshot(nullptr),
4141
m_scan_it_lower_bound(nullptr),
4242
m_scan_it_upper_bound(nullptr),
43-
m_prefix_buf(nullptr) {}
43+
m_prefix_buf(nullptr),
44+
m_check_iterate_bounds(false) {}
4445

4546
Rdb_iterator_base::~Rdb_iterator_base() {
4647
release_scan_iterator();
@@ -134,7 +135,8 @@ void Rdb_iterator_base::setup_scan_iterator(const rocksdb::Slice *const slice,
134135
m_thd, *m_kd, eq_cond,
135136
std::max(eq_cond_len, (uint)Rdb_key_def::INDEX_NUMBER_SIZE),
136137
m_scan_it_lower_bound, m_scan_it_upper_bound,
137-
&m_scan_it_lower_bound_slice, &m_scan_it_upper_bound_slice)) {
138+
&m_scan_it_lower_bound_slice, &m_scan_it_upper_bound_slice,
139+
&m_check_iterate_bounds)) {
138140
skip_bloom = false;
139141
}
140142

@@ -214,6 +216,8 @@ int Rdb_iterator_base::next_with_direction(bool move_forward, bool skip_next) {
214216
const auto &kd = *m_kd;
215217
Rdb_transaction *const tx = get_tx_from_thd(m_thd);
216218

219+
const rocksdb::Comparator *kd_comp = kd.get_cf()->GetComparator();
220+
217221
for (;;) {
218222
DEBUG_SYNC(m_thd, "rocksdb.check_flags_nwd");
219223
if (thd_killed(m_thd)) {
@@ -251,6 +255,23 @@ int Rdb_iterator_base::next_with_direction(bool move_forward, bool skip_next) {
251255
break;
252256
}
253257

258+
// Check specified lower/upper bounds
259+
// For example, retrieved key is 00077
260+
// in cf, lower_bound: 0076 and uppper bound: 0078
261+
// cf->Compare(0077, 0078) > 0 ==> False
262+
// cf->Compare(0077, 0076) < 0 ==> False
263+
// in rev cf, lower_bound: 0078 and uppper bound: 0076
264+
// revcf->Compare(0077, 0076) > 0 ==> False
265+
// revcf->Compare(0077, 0078) < 0 ==> False
266+
if (m_check_iterate_bounds &&
267+
((!m_scan_it_upper_bound_slice.empty() &&
268+
kd_comp->Compare(key, m_scan_it_upper_bound_slice) > 0) ||
269+
(!m_scan_it_lower_bound_slice.empty() &&
270+
kd_comp->Compare(key, m_scan_it_lower_bound_slice) < 0))) {
271+
rc = HA_ERR_END_OF_FILE;
272+
break;
273+
}
274+
254275
// Record is not visible due to TTL, move to next record.
255276
if (m_pkd->has_ttl() && rdb_should_hide_ttl_rec(kd, value, tx)) {
256277
continue;

‎storage/rocksdb/rdb_iterator.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ class Rdb_iterator_base : public Rdb_iterator {
120120

121121
uchar *m_prefix_buf;
122122
rocksdb::Slice m_prefix_tuple;
123+
bool m_check_iterate_bounds;
123124
};
124125

125126
class Rdb_iterator_partial : public Rdb_iterator_base {

0 commit comments

Comments
 (0)