Skip to content

Commit 40f4f32

Browse files
Cleanup rdb_hexdump (#1442)
Summary: Currently, `rdb_hexdump` has a third argument `maxsize` with a default value of 0, meaning unlimited output length, and there is also a `RDB_MAX_HEXDUMP` constant for the callers to use. However, only a part of the callers use it, the rest use the default or something else altogether (i.e. `FN_REFLEN`) Change as follows: - Default to `RDB_MAX_HEXDUMP` `maxsize` instead of unlimited. This should cover all the reasonable cases, and prevent runaway dumping in the case of corrupted `data_len` values. - Bump `RDB_MAX_HEXDUMP_LEN` from 1000 to 7000. This way it can dump any MyRocks key (3072 byte maximum). Make it `constexpr`. Minor cleanups: - Where `std::string` is used as an input for `rdb_hexdump`, pass its buffer through `data()`, not through `c_str()`, because it's used as a sized buffer and not as a zero-terminated C string - Mark `rdb_hexdump` as `nodiscard`, make its constants `constexpr`, remove redundant `const` from by-value parameters. Pull Request resolved: #1442 Differential Revision: D55594025 fbshipit-source-id: d048078
1 parent 9d620d1 commit 40f4f32

File tree

7 files changed

+125
-129
lines changed

7 files changed

+125
-129
lines changed

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

Lines changed: 98 additions & 98 deletions
Large diffs are not rendered by default.

storage/rocksdb/ha_rocksdb.cc

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6645,7 +6645,7 @@ class Rdb_snapshot_status : public Rdb_tx_list_walker {
66456645
: "NOT FOUND; CF_ID: " + std::to_string(txn.m_cf_id);
66466646

66476647
txn_data.waiting_key =
6648-
rdb_hexdump(txn.m_waiting_key.c_str(), txn.m_waiting_key.length());
6648+
rdb_hexdump(txn.m_waiting_key.data(), txn.m_waiting_key.length(), 0);
66496649

66506650
txn_data.exclusive_lock = txn.m_exclusive;
66516651

@@ -11484,20 +11484,17 @@ int ha_rocksdb::check(THD *const thd MY_ATTRIBUTE((__unused__)),
1148411484

1148511485
print_and_error : {
1148611486
std::string buf;
11487-
buf = rdb_hexdump(rowkey_copy.ptr(), rowkey_copy.length(),
11488-
RDB_MAX_HEXDUMP_LEN);
11487+
buf = rdb_hexdump(rowkey_copy.ptr(), rowkey_copy.length());
1148911488
// NO_LINT_DEBUG
1149011489
LogPluginErrMsg(ERROR_LEVEL, ER_LOG_PRINTF_MSG,
1149111490
"CHECKTABLE %s: rowkey: %s", table_name, buf.c_str());
1149211491

11493-
buf = rdb_hexdump(m_retrieved_record.data(), m_retrieved_record.size(),
11494-
RDB_MAX_HEXDUMP_LEN);
11492+
buf = rdb_hexdump(m_retrieved_record.data(), m_retrieved_record.size());
1149511493
// NO_LINT_DEBUG
1149611494
LogPluginErrMsg(ERROR_LEVEL, ER_LOG_PRINTF_MSG,
1149711495
"CHECKTABLE %s: record: %s", table_name, buf.c_str());
1149811496

11499-
buf = rdb_hexdump(sec_key_copy.ptr(), sec_key_copy.length(),
11500-
RDB_MAX_HEXDUMP_LEN);
11497+
buf = rdb_hexdump(sec_key_copy.ptr(), sec_key_copy.length());
1150111498
// NO_LINT_DEBUG
1150211499
LogPluginErrMsg(ERROR_LEVEL, ER_LOG_PRINTF_MSG,
1150311500
"CHECKTABLE %s: index: %s", table_name, buf.c_str());

storage/rocksdb/rdb_compact_filter.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,8 +189,7 @@ class Rdb_compact_filter : public rocksdb::CompactionFilter {
189189
Rdb_string_reader reader(&existing_value);
190190
if (!reader.read(m_ttl_offset) || reader.read_uint64(&ttl_timestamp)) {
191191
std::string buf;
192-
buf = rdb_hexdump(existing_value.data(), existing_value.size(),
193-
RDB_MAX_HEXDUMP_LEN);
192+
buf = rdb_hexdump(existing_value.data(), existing_value.size());
194193
rdb_fatal_error(
195194
"Decoding ttl from PK value failed in compaction filter, "
196195
"for index (%u,%u), val: %s",

storage/rocksdb/rdb_datadic.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1764,7 +1764,7 @@ void Rdb_key_def::report_checksum_mismatch(const bool is_key,
17641764
"Checksum mismatch in %s of key-value pair for index 0x%x",
17651765
is_key ? "key" : "value", get_index_number());
17661766

1767-
const std::string buf = rdb_hexdump(data, data_size, RDB_MAX_HEXDUMP_LEN);
1767+
const auto buf = rdb_hexdump(data, data_size);
17681768
// NO_LINT_DEBUG
17691769
LogPluginErrMsg(ERROR_LEVEL, ER_LOG_PRINTF_MSG,
17701770
"Data with incorrect checksum (%" PRIu64 " bytes): %s",

storage/rocksdb/rdb_i_s.cc

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2191,8 +2191,8 @@ static int rdb_i_s_lock_info_fill_table(
21912191
for (const auto &lock : lock_info) {
21922192
const uint32_t cf_id = lock.first;
21932193
const auto &key_lock_info = lock.second;
2194-
const auto key_hexstr = rdb_hexdump(key_lock_info.key.c_str(),
2195-
key_lock_info.key.length(), FN_REFLEN);
2194+
const auto key_hexstr =
2195+
rdb_hexdump(key_lock_info.key.data(), key_lock_info.key.length());
21962196

21972197
for (const auto &id : key_lock_info.ids) {
21982198
tables->table->field[RDB_LOCKS_FIELD::COLUMN_FAMILY_ID]->store(cf_id,
@@ -2300,10 +2300,10 @@ static int rdb_i_s_trx_info_fill_table(
23002300
const std::vector<Rdb_trx_info> &all_trx_info = rdb_get_all_trx_info();
23012301

23022302
for (const auto &info : all_trx_info) {
2303-
auto name_hexstr =
2304-
rdb_hexdump(info.name.c_str(), info.name.length(), NAME_LEN);
2305-
auto key_hexstr = rdb_hexdump(info.waiting_key.c_str(),
2306-
info.waiting_key.length(), FN_REFLEN);
2303+
const auto name_hexstr =
2304+
rdb_hexdump(info.name.data(), info.name.length(), NAME_LEN);
2305+
const auto key_hexstr =
2306+
rdb_hexdump(info.waiting_key.data(), info.waiting_key.length());
23072307

23082308
tables->table->field[RDB_TRX_FIELD::TRANSACTION_ID]->store(info.trx_id,
23092309
true);

storage/rocksdb/rdb_utils.cc

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -220,17 +220,18 @@ const std::vector<std::string> parse_into_tokens(const std::string &s,
220220
return tokens;
221221
}
222222

223-
static const std::size_t rdb_hex_bytes_per_char = 2;
224-
static const std::array<char, 16> rdb_hexdigit = {{'0', '1', '2', '3', '4', '5',
225-
'6', '7', '8', '9', 'a', 'b',
226-
'c', 'd', 'e', 'f'}};
223+
constexpr std::size_t rdb_hex_bytes_per_char = 2;
224+
constexpr std::array<char, 16> rdb_hexdigit = {{'0', '1', '2', '3', '4', '5',
225+
'6', '7', '8', '9', 'a', 'b',
226+
'c', 'd', 'e', 'f'}};
227227

228228
/*
229-
Convert data into a hex string with optional maximum length.
230-
If the data is larger than the maximum length trancate it and append "..".
229+
Convert data into a hex string. If the data is larger than the maximum length
230+
trancate it and append "..". If maxsize is zero, the output length is
231+
unlimited.
231232
*/
232-
std::string rdb_hexdump(const char *data, const std::size_t data_len,
233-
const std::size_t maxsize) {
233+
std::string rdb_hexdump(const char *data, std::size_t data_len,
234+
std::size_t maxsize) {
234235
// Count the elements in the string
235236
std::size_t elems = data_len;
236237
// Calculate the amount of output needed

storage/rocksdb/rdb_utils.h

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -149,10 +149,9 @@ namespace myrocks {
149149
rdb_check_mutex_call_result(__PRETTY_FUNCTION__, false, \
150150
mysql_mutex_unlock(&m))
151151

152-
/*
153-
Generic constant.
154-
*/
155-
const size_t RDB_MAX_HEXDUMP_LEN = 1000;
152+
// The default limit for rdb_hexdump output length, happens to fit the longest
153+
// possible keys (16K)
154+
constexpr size_t RDB_MAX_HEXDUMP_LEN = 2 * 16 * 1024;
156155

157156
/*
158157
Helper function to get an NULL terminated uchar* out of a given MySQL String.
@@ -340,9 +339,9 @@ const std::vector<std::string> parse_into_tokens(const std::string &s,
340339
Helper functions to populate strings.
341340
*/
342341

343-
std::string rdb_hexdump(const char *data, const std::size_t data_len,
344-
const std::size_t maxsize = 0)
345-
MY_ATTRIBUTE((__nonnull__));
342+
[[nodiscard]] std::string rdb_hexdump(
343+
const char *data, std::size_t data_len,
344+
std::size_t maxsize = RDB_MAX_HEXDUMP_LEN);
346345

347346
/*
348347
Helper function to return dir + '/' + file

0 commit comments

Comments
 (0)