From f1f8a0b789404bee81de0fbdcee5740a9aa50a5f Mon Sep 17 00:00:00 2001 From: Luqun Lou Date: Sat, 21 Oct 2023 13:09:46 -0700 Subject: [PATCH] disk size should always positive Summary: some MTR failed in ubsan due to num of rows/records calculated in records_in_range() is a negative values which is caused by m_actual_disk_size is a negative value ``` storage/rocksdb/ha_rocksdb.cc:: runtime error: -56.8272 is outside the range of representable values of type 'unsigned long long' #0 myrocks::ha_rocksdb::records_in_range_internal(unsigned int, key_range*, key_range*, long, long, unsigned long long*, unsigned long long*) /data/sandcastle/boxes/trunk-git-mysql/storage/rocksdb/ha_rocksdb.cc:14855:16 #1 myrocks::ha_rocksdb::records_in_range(unsigned int, key_range*, key_range*) /data/sandcastle/boxes/trunk-git-mysql/storage/rocksdb/ha_rocksdb.cc:14760:3 #2 handler::multi_range_read_info_const(unsigned int, RANGE_SEQ_IF*, void*, unsigned int, unsigned int*, unsigned int*, Cost_estimate*) /data/sandcastle/boxes/trunk-git-mysql/sql/handler.cc:6608:26 #3 myrocks::ha_rocksdb::multi_range_read_info_const(unsigned int, RANGE_SEQ_IF*, void*, unsigned int, unsigned int*, unsigned int*, Cost_estimate*) /data/sandcastle/boxes/trunk-git-mysql/storage/rocksdb/ha_rocksdb.cc:19002:18 #4 check_quick_select(THD*, RANGE_OPT_P ``` Due to m_actual_disk_size is an estimated value, always reset to 0 if it i becomes negative. Differential Revision: D50531919 fbshipit-source-id: e1fc02e03040c9ee05bf8015b59c87ada0a16469 --- storage/rocksdb/properties_collector.cc | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/storage/rocksdb/properties_collector.cc b/storage/rocksdb/properties_collector.cc index e55474af1027..e816ea83f8d7 100644 --- a/storage/rocksdb/properties_collector.cc +++ b/storage/rocksdb/properties_collector.cc @@ -474,10 +474,10 @@ void Rdb_index_stats::merge(const Rdb_index_stats &s, const bool increment, /* The Data_length and Avg_row_length are trailing statistics, meaning - they don't get updated for the current SST until the next SST is - written. So, if rocksdb reports the data_length as 0, + they don't get updated for the current data block until the next data + block is written. So, if rocksdb reports the data_length as 0, we make a reasoned estimate for the data_file_length for the - index in the current SST. + index in the current data block. */ m_actual_disk_size += s.m_actual_disk_size ? s.m_actual_disk_size : estimated_data_len * s.m_rows; @@ -493,6 +493,8 @@ void Rdb_index_stats::merge(const Rdb_index_stats &s, const bool increment, m_data_size -= s.m_data_size; m_actual_disk_size -= s.m_actual_disk_size ? s.m_actual_disk_size : estimated_data_len * s.m_rows; + // actual disk size should always >=0 + if (m_actual_disk_size < 0) m_actual_disk_size = 0; m_entry_deletes -= s.m_entry_deletes; m_entry_single_deletes -= s.m_entry_single_deletes; m_entry_merges -= s.m_entry_merges;