From d32f5be307d5593bf36d243de275af88ff8ee92a Mon Sep 17 00:00:00 2001 From: Sergei Petrunia Date: Fri, 12 Jan 2018 15:11:56 +0300 Subject: [PATCH] MDEV-14372: Fix and enable rocksdb.information_schema test - Make Rdb_binlog_manager::unpack_value to not have a stack overrun when it is reading invalid data (which it currently does as we in MariaDB do not store binlog coordinates under BINLOG_INFO_INDEX_NUMBER, see comments in MDEV-14892 for details). - We may need to store these coordinates in the future, so instead of removing the call of this function, let's make it work properly for all possible inputs. --- .../rocksdb/r/information_schema.result | 30 +++++++++---------- .../rocksdb/mysql-test/rocksdb/t/disabled.def | 1 - .../rocksdb/t/information_schema-master.opt | 2 +- .../rocksdb/t/information_schema.test | 12 ++++++-- storage/rocksdb/rdb_datadic.cc | 27 ++++++++++++++++- storage/rocksdb/rdb_datadic.h | 5 +++- storage/rocksdb/rdb_i_s.cc | 1 - 7 files changed, 56 insertions(+), 22 deletions(-) diff --git a/storage/rocksdb/mysql-test/rocksdb/r/information_schema.result b/storage/rocksdb/mysql-test/rocksdb/r/information_schema.result index 291effa832c..717fb0b970c 100644 --- a/storage/rocksdb/mysql-test/rocksdb/r/information_schema.result +++ b/storage/rocksdb/mysql-test/rocksdb/r/information_schema.result @@ -1,6 +1,20 @@ DROP TABLE IF EXISTS t1; DROP TABLE IF EXISTS t2; DROP TABLE IF EXISTS t3; +create table t1 (a int) engine=rocksdb; +drop table t1; +select * from INFORMATION_SCHEMA.ROCKSDB_GLOBAL_INFO; +TYPE NAME VALUE +MAX_INDEX_ID MAX_INDEX_ID max_index_id +CF_FLAGS 0 default [0] +CF_FLAGS 1 __system__ [0] +DDL_DROP_INDEX_ONGOING cf_id:0,index_id:max_index_id +select count(*) from INFORMATION_SCHEMA.ROCKSDB_GLOBAL_INFO; +count(*) +4 +select VALUE into @keysIn from INFORMATION_SCHEMA.ROCKSDB_COMPACTION_STATS where CF_NAME = 'default' and LEVEL = 'Sum' and TYPE = 'KeyIn'; +CREATE TABLE t1 (i1 INT, i2 INT, PRIMARY KEY (i1)) ENGINE = ROCKSDB; +INSERT INTO t1 VALUES (1, 1), (2, 2), (3, 3); select * from INFORMATION_SCHEMA.ROCKSDB_GLOBAL_INFO; TYPE NAME VALUE MAX_INDEX_ID MAX_INDEX_ID max_index_id @@ -9,20 +23,6 @@ CF_FLAGS 1 __system__ [0] select count(*) from INFORMATION_SCHEMA.ROCKSDB_GLOBAL_INFO; count(*) 3 -select VALUE into @keysIn from INFORMATION_SCHEMA.ROCKSDB_COMPACTION_STATS where CF_NAME = 'default' and LEVEL = 'Sum' and TYPE = 'KeyIn'; -CREATE TABLE t1 (i1 INT, i2 INT, PRIMARY KEY (i1)) ENGINE = ROCKSDB; -INSERT INTO t1 VALUES (1, 1), (2, 2), (3, 3); -select * from INFORMATION_SCHEMA.ROCKSDB_GLOBAL_INFO; -TYPE NAME VALUE -BINLOG FILE master-bin.000001 -BINLOG POS 1066 -BINLOG GTID uuid:5 -MAX_INDEX_ID MAX_INDEX_ID max_index_id -CF_FLAGS 0 default [0] -CF_FLAGS 1 __system__ [0] -select count(*) from INFORMATION_SCHEMA.ROCKSDB_GLOBAL_INFO; -count(*) -6 set global rocksdb_force_flush_memtable_now = true; set global rocksdb_compact_cf='default'; select case when VALUE-@keysIn >= 3 then 'true' else 'false' end from INFORMATION_SCHEMA.ROCKSDB_COMPACTION_STATS where CF_NAME = 'default' and LEVEL = 'Sum' and TYPE = 'KeyIn'; @@ -66,7 +66,7 @@ SHOW GLOBAL VARIABLES LIKE 'ROCKSDB_PAUSE_BACKGROUND_WORK'; Variable_name Value rocksdb_pause_background_work ON DROP TABLE t3; -cf_id:0,index_id:267 +cf_id:0,index_id:264 SET GLOBAL ROCKSDB_PAUSE_BACKGROUND_WORK=0; SHOW GLOBAL VARIABLES LIKE 'ROCKSDB_PAUSE_BACKGROUND_WORK'; Variable_name Value diff --git a/storage/rocksdb/mysql-test/rocksdb/t/disabled.def b/storage/rocksdb/mysql-test/rocksdb/t/disabled.def index dc2e04b93d7..359d1d3e082 100644 --- a/storage/rocksdb/mysql-test/rocksdb/t/disabled.def +++ b/storage/rocksdb/mysql-test/rocksdb/t/disabled.def @@ -72,7 +72,6 @@ blind_delete_without_tx_api: MDEV-12286: rocksdb.blind_delete_without_tx_api tes ## Tests that fail for some other reason ## -information_schema : MariaRocks: requires GTIDs mysqlbinlog_gtid_skip_empty_trans_rocksdb : MariaRocks: requires GTIDs rpl_row_triggers : MariaRocks: requires GTIDs diff --git a/storage/rocksdb/mysql-test/rocksdb/t/information_schema-master.opt b/storage/rocksdb/mysql-test/rocksdb/t/information_schema-master.opt index 40b14167e17..86379847638 100644 --- a/storage/rocksdb/mysql-test/rocksdb/t/information_schema-master.opt +++ b/storage/rocksdb/mysql-test/rocksdb/t/information_schema-master.opt @@ -1 +1 @@ ---binlog_format=row --gtid_mode=ON --enforce_gtid_consistency --log_slave_updates +--binlog_format=row --log_slave_updates diff --git a/storage/rocksdb/mysql-test/rocksdb/t/information_schema.test b/storage/rocksdb/mysql-test/rocksdb/t/information_schema.test index c8d9b538529..8746e589046 100644 --- a/storage/rocksdb/mysql-test/rocksdb/t/information_schema.test +++ b/storage/rocksdb/mysql-test/rocksdb/t/information_schema.test @@ -9,6 +9,13 @@ DROP TABLE IF EXISTS t2; DROP TABLE IF EXISTS t3; --enable_warnings +# MariaDB: the following is for handling the case where the tests +# is started on a totally empty datadir, where no MyRocks table has +# ever been created). In that case, there is no MAX_INDEX_ID. +# Create/drop a table so that we do have MAX_INDEX_ID. +create table t1 (a int) engine=rocksdb; +drop table t1; + --let $max_index_id = query_get_value(SELECT * from INFORMATION_SCHEMA.ROCKSDB_GLOBAL_INFO where type = 'MAX_INDEX_ID', VALUE, 1) --replace_result $max_index_id max_index_id select * from INFORMATION_SCHEMA.ROCKSDB_GLOBAL_INFO; @@ -19,9 +26,10 @@ select VALUE into @keysIn from INFORMATION_SCHEMA.ROCKSDB_COMPACTION_STATS where CREATE TABLE t1 (i1 INT, i2 INT, PRIMARY KEY (i1)) ENGINE = ROCKSDB; INSERT INTO t1 VALUES (1, 1), (2, 2), (3, 3); ---let $MASTER_UUID = query_get_value(SELECT @@SERVER_UUID, @@SERVER_UUID, 1) +# No binlog coordinates in MariaDB: --let $MASTER_UUID = query_get_value(SELECT @@SERVER_UUID, @@SERVER_UUID, 1) --let $max_index_id = query_get_value(SELECT * from INFORMATION_SCHEMA.ROCKSDB_GLOBAL_INFO where type = 'MAX_INDEX_ID', VALUE, 1) ---replace_result $MASTER_UUID uuid $max_index_id max_index_id +# No binlog coordinates in MariaDB: --replace_result $MASTER_UUID uuid $max_index_id max_index_id +--replace_result $max_index_id max_index_id select * from INFORMATION_SCHEMA.ROCKSDB_GLOBAL_INFO; select count(*) from INFORMATION_SCHEMA.ROCKSDB_GLOBAL_INFO; diff --git a/storage/rocksdb/rdb_datadic.cc b/storage/rocksdb/rdb_datadic.cc index f6f1406e80c..b2f5af705a3 100644 --- a/storage/rocksdb/rdb_datadic.cc +++ b/storage/rocksdb/rdb_datadic.cc @@ -4261,7 +4261,7 @@ bool Rdb_binlog_manager::read(char *const binlog_name, std::string value; rocksdb::Status status = m_dict->get_value(m_key_slice, &value); if (status.ok()) { - if (!unpack_value((const uchar *)value.c_str(), binlog_name, binlog_pos, + if (!unpack_value((const uchar *)value.c_str(), value.size(), binlog_name, binlog_pos, binlog_gtid)) ret = true; } @@ -4331,35 +4331,60 @@ Rdb_binlog_manager::pack_value(uchar *const buf, const char *const binlog_name, @return true on error */ bool Rdb_binlog_manager::unpack_value(const uchar *const value, + size_t value_size_arg, char *const binlog_name, my_off_t *const binlog_pos, char *const binlog_gtid) const { uint pack_len = 0; + intmax_t value_size= value_size_arg; DBUG_ASSERT(binlog_pos != nullptr); + if ((value_size -= Rdb_key_def::VERSION_SIZE) < 0) + return true; // read version const uint16_t version = rdb_netbuf_to_uint16(value); + pack_len += Rdb_key_def::VERSION_SIZE; if (version != Rdb_key_def::BINLOG_INFO_INDEX_NUMBER_VERSION) return true; + if ((value_size -= sizeof(uint16)) < 0) + return true; + // read binlog file name length const uint16_t binlog_name_len = rdb_netbuf_to_uint16(value + pack_len); pack_len += sizeof(uint16); + + if (binlog_name_len >= (FN_REFLEN+1)) + return true; + + if ((value_size -= binlog_name_len) < 0) + return true; + if (binlog_name_len) { // read and set binlog name memcpy(binlog_name, value + pack_len, binlog_name_len); binlog_name[binlog_name_len] = '\0'; pack_len += binlog_name_len; + if ((value_size -= sizeof(uint32)) < 0) + return true; // read and set binlog pos *binlog_pos = rdb_netbuf_to_uint32(value + pack_len); pack_len += sizeof(uint32); + if ((value_size -= sizeof(uint16)) < 0) + return true; // read gtid length const uint16_t binlog_gtid_len = rdb_netbuf_to_uint16(value + pack_len); pack_len += sizeof(uint16); + + if (binlog_gtid_len >= GTID_BUF_LEN) + return true; + if ((value_size -= binlog_gtid_len) < 0) + return true; + if (binlog_gtid && binlog_gtid_len > 0) { // read and set gtid memcpy(binlog_gtid, value + pack_len, binlog_gtid_len); diff --git a/storage/rocksdb/rdb_datadic.h b/storage/rocksdb/rdb_datadic.h index 5796132de39..223f61edb43 100644 --- a/storage/rocksdb/rdb_datadic.h +++ b/storage/rocksdb/rdb_datadic.h @@ -46,6 +46,8 @@ class Rdb_field_packing; class Rdb_cf_manager; class Rdb_ddl_manager; +const uint32_t GTID_BUF_LEN = 60; + /* @brief Field packing context. @@ -1154,7 +1156,8 @@ private: rocksdb::Slice pack_value(uchar *const buf, const char *const binlog_name, const my_off_t &binlog_pos, const char *const binlog_gtid) const; - bool unpack_value(const uchar *const value, char *const binlog_name, + bool unpack_value(const uchar *const value, size_t value_size, + char *const binlog_name, my_off_t *const binlog_pos, char *const binlog_gtid) const; std::atomic m_slave_gtid_info_tbl; diff --git a/storage/rocksdb/rdb_i_s.cc b/storage/rocksdb/rdb_i_s.cc index 424a9e6c1f4..f8ddcb00fb3 100644 --- a/storage/rocksdb/rdb_i_s.cc +++ b/storage/rocksdb/rdb_i_s.cc @@ -729,7 +729,6 @@ static int rdb_i_s_global_info_fill_table( DBUG_ASSERT(tables != nullptr); static const uint32_t INT_BUF_LEN = 21; - static const uint32_t GTID_BUF_LEN = 60; static const uint32_t CF_ID_INDEX_BUF_LEN = 60; int ret = 0;