From 8088482dd64b352d402a9cd6299a94f50015d1ae Mon Sep 17 00:00:00 2001 From: Ziyue Yang Date: Thu, 2 Apr 2020 11:51:45 -0700 Subject: [PATCH] Fix a division by zero after #6262 (#6633) Summary: With https://github.com/facebook/rocksdb/issues/6262, UBSAN fails with "division by zero": [ RUN ] Timestamp/DBBasicTestWithTimestampCompressionSettings.PutAndGetWithCompaction/3 internal_repo_rocksdb/repo/table/block_based/block_based_table_builder.cc:1066:39: runtime error: division by zero #0 0x7ffb3117b071 in rocksdb::BlockBasedTableBuilder::WriteRawBlock(rocksdb::Slice const&, rocksdb::CompressionType, rocksdb::BlockHandle*, bool) internal_repo_rocksdb/repo/table/block_based/block_based_table_builder.cc:1066 https://github.com/facebook/rocksdb/issues/1 0x7ffb311775e1 in rocksdb::BlockBasedTableBuilder::WriteBlock(rocksdb::Slice const&, rocksdb::BlockHandle*, bool) internal_repo_rocksdb/repo/table/block_based/block_based_table_builder.cc:848 https://github.com/facebook/rocksdb/issues/2 0x7ffb311771a2 in rocksdb::BlockBasedTableBuilder::WriteBlock(rocksdb::BlockBuilder*, rocksdb::BlockHandle*, bool) internal_repo_rocksdb/repo/table/block_based/block_based_table_builder.cc:832 This is caused by not returning immediately after CompressAndVerifyBlock call in WriteBlock when rep_->status == kBuffered. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6633 Test Plan: Run all existing test. Reviewed By: anand1976 Differential Revision: D20808366 fbshipit-source-id: 09f24b7c0fbaf4c7a8fc48cac61fa6fcb9b85811 --- table/block_based/block_based_table_builder.cc | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/table/block_based/block_based_table_builder.cc b/table/block_based/block_based_table_builder.cc index 4aa586edf..8e77d938d 100644 --- a/table/block_based/block_based_table_builder.cc +++ b/table/block_based/block_based_table_builder.cc @@ -839,6 +839,13 @@ void BlockBasedTableBuilder::WriteBlock(const Slice& raw_block_contents, Rep* r = rep_; Slice block_contents; CompressionType type; + if (r->state == Rep::State::kBuffered) { + assert(is_data_block); + assert(!r->data_block_and_keys_buffers.empty()); + r->data_block_and_keys_buffers.back().first = raw_block_contents.ToString(); + r->data_begin_offset += r->data_block_and_keys_buffers.back().first.size(); + return; + } CompressAndVerifyBlock(raw_block_contents, is_data_block, *(r->compression_ctxs[0]), r->verify_ctxs[0].get(), r->compressed_output, block_contents, type, r->status); @@ -888,14 +895,6 @@ void BlockBasedTableBuilder::CompressAndVerifyBlock( r->ioptions.env, ShouldReportDetailedTime(r->ioptions.env, r->ioptions.statistics)); - if (r->state == Rep::State::kBuffered) { - assert(is_data_block); - assert(!r->data_block_and_keys_buffers.empty()); - r->data_block_and_keys_buffers.back().first = raw_block_contents.ToString(); - r->data_begin_offset += r->data_block_and_keys_buffers.back().first.size(); - return; - } - if (raw_block_contents.size() < kCompressionSizeLimit) { const CompressionDict* compression_dict; if (!is_data_block || r->compression_dict == nullptr) { @@ -1060,6 +1059,9 @@ void BlockBasedTableBuilder::WriteRawBlock(const Slice& block_contents, } if (r->compression_opts.parallel_threads > 1) { if (!r->pc_rep->finished) { + assert(r->pc_rep->raw_bytes_compressed + + r->pc_rep->raw_bytes_curr_block > + 0); r->pc_rep->curr_compression_ratio = (r->pc_rep->curr_compression_ratio * r->pc_rep->raw_bytes_compressed +