Skip to content

Commit 63b3ad1

Browse files
inikepinikep
authored
Merge pull request #5623 from inikep/PS-9666-8.4
PS-9666 [8.4]: insert with unique_checks=OFF into table with TTL crashing MyRocks
2 parents a4f8716 + ce49ae8 commit 63b3ad1

File tree

6 files changed

+74
-27
lines changed

6 files changed

+74
-27
lines changed

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,15 @@ PARTITION p1 VALUES IN (33, 44, 7, 10, 68, 78, 72, 2, 24, 73, 50, 56, 83,
2525
PARTITION p2 VALUES IN (90, 11, 87, 25, 97, 93, 47, 41, 92, 37, 67, 20, 43,
2626
42, 53, 62, 13));
2727
DROP TABLE t;
28+
#
29+
# PS-9666: insert with unique_checks=OFF into table with TTL crashing MyRocks
30+
# https://perconadev.atlassian.net/browse/PS-9666
31+
#
32+
CREATE TABLE t1 (
33+
`id` varbinary(32) NOT NULL,
34+
`ttl_ts` bigint unsigned NOT NULL,
35+
PRIMARY KEY (`id`)
36+
) ENGINE=ROCKSDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci COMMENT='ttl_duration=31536000;ttl_col=ttl_ts;';
37+
SET unique_checks=OFF;
38+
INSERT INTO t1 (`id`, `ttl_ts`) VALUES ('helloworld', 1738737225);
39+
DROP TABLE t1;

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,3 +52,22 @@ TRUNCATE TABLE t;
5252
--enable_result_log
5353

5454
DROP TABLE t;
55+
56+
57+
58+
--echo #
59+
--echo # PS-9666: insert with unique_checks=OFF into table with TTL crashing MyRocks
60+
--echo # https://perconadev.atlassian.net/browse/PS-9666
61+
--echo #
62+
63+
CREATE TABLE t1 (
64+
`id` varbinary(32) NOT NULL,
65+
`ttl_ts` bigint unsigned NOT NULL,
66+
PRIMARY KEY (`id`)
67+
) ENGINE=ROCKSDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci COMMENT='ttl_duration=31536000;ttl_col=ttl_ts;';
68+
69+
SET unique_checks=OFF;
70+
71+
INSERT INTO t1 (`id`, `ttl_ts`) VALUES ('helloworld', 1738737225);
72+
73+
DROP TABLE t1;

‎storage/rocksdb/ha_rocksdb.cc‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7739,7 +7739,7 @@ static int rdb_dbug_set_ttl_read_filter_ts();
77397739
snapshots when filtering keys.
77407740
*/
77417741
bool rdb_should_hide_ttl_rec(const Rdb_key_def &kd,
7742-
const rocksdb::Slice &ttl_rec_val,
7742+
const rocksdb::Slice *const ttl_rec_val,
77437743
Rdb_transaction *tx) {
77447744
assert(kd.has_ttl());
77457745
assert(kd.m_ttl_rec_offset != UINT_MAX);
@@ -7767,7 +7767,13 @@ bool rdb_should_hide_ttl_rec(const Rdb_key_def &kd,
77677767
return false;
77687768
}
77697769

7770-
Rdb_string_reader reader(&ttl_rec_val);
7770+
// Case: No value supplied, this happens when the key is not found, so we'll
7771+
// just return that it should be filtered
7772+
if (!ttl_rec_val) {
7773+
return true;
7774+
}
7775+
7776+
Rdb_string_reader reader(ttl_rec_val);
77717777

77727778
/*
77737779
Find where the 8-byte ttl is for each record in this index.
@@ -7779,7 +7785,7 @@ bool rdb_should_hide_ttl_rec(const Rdb_key_def &kd,
77797785
8 byte ttl field in front. Don't filter the record out, and log an error.
77807786
*/
77817787
std::string buf;
7782-
buf = rdb_hexdump(ttl_rec_val.data(), ttl_rec_val.size(),
7788+
buf = rdb_hexdump(ttl_rec_val->data(), ttl_rec_val->size(),
77837789
RDB_MAX_HEXDUMP_LEN);
77847790
const GL_INDEX_ID gl_index_id = kd.get_gl_index_id();
77857791
LogPluginErrMsg(
@@ -17263,10 +17269,10 @@ void rdb_tx_release_lock(Rdb_transaction *tx, const Rdb_key_def &kd,
1726317269
tx->release_lock(kd, std::string(key.data(), key.size()), force);
1726417270
}
1726517271

17266-
int rdb_tx_set_status_error(Rdb_transaction *tx, const rocksdb::Status &s,
17272+
int rdb_tx_set_status_error(Rdb_transaction &tx, const rocksdb::Status &s,
1726717273
const Rdb_key_def &kd,
1726817274
const Rdb_tbl_def *const tbl_def) {
17269-
return tx->set_status_error(tx->get_thd(), s, kd, tbl_def);
17275+
return tx.set_status_error(tx.get_thd(), s, kd, tbl_def);
1727017276
}
1727117277

1727217278
static bool parse_fault_injection_file_type(const std::string &type_str,

‎storage/rocksdb/ha_rocksdb.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1179,11 +1179,11 @@ inline void rocksdb_smart_prev(bool seek_backward,
11791179
bool is_valid_iterator(rocksdb::Iterator *scan_it);
11801180

11811181
bool rdb_should_hide_ttl_rec(const Rdb_key_def &kd,
1182-
const rocksdb::Slice &ttl_rec_val,
1182+
const rocksdb::Slice *const ttl_rec_val,
11831183
Rdb_transaction *tx);
11841184

11851185
bool rdb_tx_started(Rdb_transaction *tx);
1186-
int rdb_tx_set_status_error(Rdb_transaction *tx, const rocksdb::Status &s,
1186+
int rdb_tx_set_status_error(Rdb_transaction &tx, const rocksdb::Status &s,
11871187
const Rdb_key_def &kd,
11881188
const Rdb_tbl_def *const tbl_def);
11891189

‎storage/rocksdb/rdb_iterator.cc‎

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ int Rdb_iterator_base::next_with_direction(bool move_forward, bool skip_next) {
295295
}
296296

297297
// Record is not visible due to TTL, move to next record.
298-
if (m_pkd.has_ttl() && rdb_should_hide_ttl_rec(m_kd, value, tx)) {
298+
if (m_pkd.has_ttl() && rdb_should_hide_ttl_rec(m_kd, &value, tx)) {
299299
continue;
300300
}
301301

@@ -357,10 +357,29 @@ int Rdb_iterator_base::seek(enum ha_rkey_function find_flag,
357357
return rc;
358358
}
359359

360+
int Rdb_iterator_base::convert_get_status(myrocks::Rdb_transaction &tx,
361+
const rocksdb::Status &s,
362+
rocksdb::PinnableSlice *value,
363+
bool skip_ttl_check) const {
364+
int rc = HA_EXIT_SUCCESS;
365+
if (!s.IsNotFound() && !s.ok()) {
366+
return rdb_tx_set_status_error(tx, s, m_kd, m_tbl_def);
367+
}
368+
369+
const bool hide_ttl_rec =
370+
!skip_ttl_check && m_kd.has_ttl() &&
371+
rdb_should_hide_ttl_rec(m_kd, s.IsNotFound() ? nullptr : value, &tx);
372+
373+
if (hide_ttl_rec || s.IsNotFound()) {
374+
return HA_ERR_KEY_NOT_FOUND;
375+
}
376+
377+
return rc;
378+
}
379+
360380
int Rdb_iterator_base::get(const rocksdb::Slice *key,
361381
rocksdb::PinnableSlice *value, Rdb_lock_type type,
362382
bool skip_ttl_check, bool skip_wait) {
363-
int rc = HA_EXIT_SUCCESS;
364383
Rdb_transaction *const tx = get_tx_from_thd(m_thd);
365384
rocksdb::Status s;
366385
if (type == RDB_LOCK_NONE) {
@@ -374,20 +393,7 @@ int Rdb_iterator_base::get(const rocksdb::Slice *key,
374393
"rocksdb_return_status_corrupted",
375394
{ s = rocksdb::Status::Corruption(); });
376395

377-
if (!s.IsNotFound() && !s.ok()) {
378-
return rdb_tx_set_status_error(tx, s, m_kd, m_tbl_def);
379-
}
380-
381-
if (s.IsNotFound()) {
382-
return HA_ERR_KEY_NOT_FOUND;
383-
}
384-
385-
if (!skip_ttl_check && m_kd.has_ttl() &&
386-
rdb_should_hide_ttl_rec(m_kd, *value, tx)) {
387-
return HA_ERR_KEY_NOT_FOUND;
388-
}
389-
390-
return rc;
396+
return convert_get_status(*tx, s, value, skip_ttl_check);
391397
}
392398

393399
Rdb_iterator_partial::Rdb_iterator_partial(THD *thd, const Rdb_key_def &kd,
@@ -657,7 +663,7 @@ int Rdb_iterator_partial::materialize_prefix() {
657663
return HA_EXIT_SUCCESS;
658664
} else if (!s.IsNotFound()) {
659665
thd_proc_info(m_thd, old_proc_info);
660-
return rdb_tx_set_status_error(tx, s, m_kd, m_tbl_def);
666+
return rdb_tx_set_status_error(*tx, s, m_kd, m_tbl_def);
661667
}
662668

663669
rocksdb::WriteOptions options;
@@ -669,7 +675,7 @@ int Rdb_iterator_partial::materialize_prefix() {
669675
// Write sentinel key with empty value.
670676
s = wb->Put(m_kd.get_cf(), cur_prefix_key, rocksdb::Slice());
671677
if (!s.ok()) {
672-
rc = rdb_tx_set_status_error(tx, s, m_kd, m_tbl_def);
678+
rc = rdb_tx_set_status_error(*tx, s, m_kd, m_tbl_def);
673679
rdb_tx_release_lock(tx, m_kd, cur_prefix_key, true /* force */);
674680
thd_proc_info(m_thd, old_proc_info);
675681
return rc;
@@ -711,7 +717,7 @@ int Rdb_iterator_partial::materialize_prefix() {
711717
rocksdb::Slice((const char *)m_sk_tails.ptr(),
712718
m_sk_tails.get_current_pos()));
713719
if (!s.ok()) {
714-
rc = rdb_tx_set_status_error(tx, s, m_kd, m_tbl_def);
720+
rc = rdb_tx_set_status_error(*tx, s, m_kd, m_tbl_def);
715721
goto exit;
716722
}
717723

@@ -724,7 +730,7 @@ int Rdb_iterator_partial::materialize_prefix() {
724730

725731
s = rdb_get_rocksdb_db()->Write(options, optimize, wb.get());
726732
if (!s.ok()) {
727-
rc = rdb_tx_set_status_error(tx, s, m_kd, m_tbl_def);
733+
rc = rdb_tx_set_status_error(*tx, s, m_kd, m_tbl_def);
728734
goto exit;
729735
}
730736

‎storage/rocksdb/rdb_iterator.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,10 @@ class Rdb_iterator_base : public Rdb_iterator {
6868
const int bytes_changed_by_succ,
6969
const rocksdb::Slice &end_key);
7070
int next_with_direction(bool move_forward, bool skip_next);
71+
[[nodiscard]] int convert_get_status(myrocks::Rdb_transaction &tx,
72+
const rocksdb::Status &status,
73+
rocksdb::PinnableSlice *value,
74+
bool skip_ttl_check) const;
7175

7276
public:
7377
Rdb_iterator_base(THD *thd, const Rdb_key_def &kd, const Rdb_key_def &pkd,

0 commit comments

Comments
 (0)