Skip to content

Commit 14818de

Browse files
Luqun LouVarunNagaraju
Luqun Lou
authored and
VarunNagaraju
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 e2e59ed commit 14818de

9 files changed

Lines changed: 250 additions & 4 deletions

File tree

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,44 @@ 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+
) engine=rocksdb 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+
SET SESSION TRANSACTION ISOLATION LEVEL READ COMMITTED;
28+
begin;
29+
insert into t values(101, 101, 'view_routine_usage');
30+
update t set b = 100 where b = 101 and c like 'view_routine_usage';
31+
update t set b = 101 where b = 100 and c like 'view_routine_usage';
32+
select a from t where b = 101 and c like 'view_routine_usage';
33+
a
34+
101
35+
rollback;
36+
drop table t;
37+
create table t(
38+
a int unsigned not null,
39+
b int unsigned not null,
40+
c varchar(64) not null COLLATE utf8_bin,
41+
primary key(a),
42+
key(b,c) comment 'cfname=rev:bc'
43+
) engine=rocksdb DEFAULT CHARSET=utf8 COLLATE=utf8_bin;
44+
Warnings:
45+
Warning 3778 'utf8mb3_bin' is a collation of the deprecated character set UTF8MB3. Please consider using UTF8MB4 with an appropriate collation instead.
46+
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.
47+
Warning 3778 'utf8mb3_bin' is a collation of the deprecated character set UTF8MB3. Please consider using UTF8MB4 with an appropriate collation instead.
48+
begin;
49+
insert into t values(110, 110, 'view_routine_usage');
50+
insert into t values(100, 100, 'view_routine_usage');
51+
update t set b = 101 where b = 100 and c like 'view_routine_usage';
52+
update t set b = 100 where b = 101 and c like 'view_routine_usage';
53+
select a from t where b = 101 and c like 'view_routine_usage';
54+
a
55+
rollback;
56+
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: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,3 +28,58 @@ 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+
) engine=rocksdb DEFAULT CHARSET=utf8 COLLATE=utf8_bin;
42+
43+
SET SESSION TRANSACTION ISOLATION LEVEL READ COMMITTED;
44+
45+
begin;
46+
insert into t values(101, 101, 'view_routine_usage');
47+
# SD(101, 'view_routine_usage',101)
48+
update t set b = 100 where b = 101 and c like 'view_routine_usage';
49+
# dring iterating, writebatchwithindex may return "out of range" record after
50+
# checking with upper bounds. sometimes the "out of range" record is a SD record.
51+
# For SD record, its value slice will be empty. Try to decode a key slice
52+
# which contains varchar with empty value slice, decoder will crash due missing
53+
# upack_info in value slice
54+
update t set b = 101 where b = 100 and c like 'view_routine_usage';
55+
select a from t where b = 101 and c like 'view_routine_usage';
56+
rollback;
57+
58+
drop table t;
59+
60+
61+
#
62+
# check bounds for writebatch(rev cf), all data changes are in writebatch
63+
#
64+
create table t(
65+
a int unsigned not null,
66+
b int unsigned not null,
67+
c varchar(64) not null COLLATE utf8_bin,
68+
primary key(a),
69+
key(b,c) comment 'cfname=rev:bc'
70+
) engine=rocksdb DEFAULT CHARSET=utf8 COLLATE=utf8_bin;
71+
72+
begin;
73+
insert into t values(110, 110, 'view_routine_usage');
74+
insert into t values(100, 100, 'view_routine_usage');
75+
# SD(100, 'view_routine_usage',100)
76+
update t set b = 101 where b = 100 and c like 'view_routine_usage';
77+
# during iterating, we don't check with lower bound in writebatchwithindex
78+
# in rev cf,
79+
update t set b = 100 where b = 101 and c like 'view_routine_usage';
80+
select a from t where b = 101 and c like 'view_routine_usage';
81+
rollback;
82+
83+
drop table t;
84+
85+

‎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
@@ -1355,6 +1355,11 @@ static MYSQL_THDVAR_BOOL(
13551355
"Enable rocksdb iterator upper/lower bounds in read options.", nullptr,
13561356
nullptr, true);
13571357

1358+
static MYSQL_THDVAR_BOOL(
1359+
check_iterate_bounds, PLUGIN_VAR_OPCMDARG,
1360+
"Check rocksdb iterator upper/lower bounds during iterating.", nullptr,
1361+
nullptr, true);
1362+
13581363
static const char *const DEFAULT_READ_FREE_RPL_TABLES = ".*";
13591364

13601365
static std::regex_constants::syntax_option_type get_regex_flags() {
@@ -2673,6 +2678,7 @@ static struct SYS_VAR *rocksdb_system_variables[] = {
26732678
MYSQL_SYSVAR(commit_in_the_middle),
26742679
MYSQL_SYSVAR(blind_delete_primary_key),
26752680
MYSQL_SYSVAR(enable_iterate_bounds),
2681+
MYSQL_SYSVAR(check_iterate_bounds),
26762682
#if defined(ROCKSDB_INCLUDE_RFR) && ROCKSDB_INCLUDE_RFR
26772683
MYSQL_SYSVAR(read_free_rpl_tables),
26782684
MYSQL_SYSVAR(read_free_rpl),
@@ -16053,11 +16059,13 @@ bool ha_rocksdb::can_assume_tracked(THD *thd) {
1605316059
bool ha_rocksdb::check_bloom_and_set_bounds(
1605416060
THD *thd, const Rdb_key_def &kd, const rocksdb::Slice &eq_cond,
1605516061
size_t bound_len, uchar *const lower_bound, uchar *const upper_bound,
16056-
rocksdb::Slice *lower_bound_slice, rocksdb::Slice *upper_bound_slice) {
16062+
rocksdb::Slice *lower_bound_slice, rocksdb::Slice *upper_bound_slice,
16063+
bool *check_iterate_bounds) {
1605716064
bool can_use_bloom = can_use_bloom_filter(thd, kd, eq_cond);
1605816065
if (!can_use_bloom && (THDVAR(thd, enable_iterate_bounds))) {
1605916066
setup_iterator_bounds(kd, eq_cond, bound_len, lower_bound, upper_bound,
1606016067
lower_bound_slice, upper_bound_slice);
16068+
*check_iterate_bounds = THDVAR(thd, check_iterate_bounds);
1606116069
}
1606216070
return can_use_bloom;
1606316071
}

‎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)