Skip to content

Commit

Permalink
MDEV-14372: Fix and enable rocksdb.information_schema test
Browse files Browse the repository at this point in the history
- 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.
  • Loading branch information
spetrunia committed Jan 12, 2018
1 parent bf77191 commit d32f5be
Show file tree
Hide file tree
Showing 7 changed files with 47 additions and 13 deletions.
12 changes: 6 additions & 6 deletions storage/rocksdb/mysql-test/rocksdb/r/information_schema.result
Original file line number Diff line number Diff line change
@@ -1,28 +1,28 @@
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(*)
3
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
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
3
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';
Expand Down Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion storage/rocksdb/mysql-test/rocksdb/t/disabled.def
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
--binlog_format=row --gtid_mode=ON --enforce_gtid_consistency --log_slave_updates
--binlog_format=row --log_slave_updates
12 changes: 10 additions & 2 deletions storage/rocksdb/mysql-test/rocksdb/t/information_schema.test
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down
27 changes: 26 additions & 1 deletion storage/rocksdb/rdb_datadic.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 4 additions & 1 deletion storage/rocksdb/rdb_datadic.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -1154,7 +1156,8 @@ class Rdb_binlog_manager {
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<Rdb_tbl_def *> m_slave_gtid_info_tbl;
Expand Down
1 change: 0 additions & 1 deletion storage/rocksdb/rdb_i_s.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit d32f5be

Please sign in to comment.