Skip to content

Commit

Permalink
disk size should always positive
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Luqun Lou authored and facebook-github-bot committed Oct 21, 2023
1 parent 230003a commit f1f8a0b
Showing 1 changed file with 5 additions and 3 deletions.
8 changes: 5 additions & 3 deletions storage/rocksdb/properties_collector.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down

0 comments on commit f1f8a0b

Please sign in to comment.