From a074d46a5a83cd1b846089705e88ef14fc727ee9 Mon Sep 17 00:00:00 2001 From: Akanksha Mahajan Date: Wed, 4 Aug 2021 17:11:47 -0700 Subject: [PATCH] Fix clang failure (#8621) Summary: Fixed clang failure because of memory leak Pull Request resolved: https://github.com/facebook/rocksdb/pull/8621 Test Plan: CircleCI clang job Reviewed By: pdillinger Differential Revision: D30114337 Pulled By: akankshamahajan15 fbshipit-source-id: 16572b9bcbaa053c2ab7bc1c344148d0e6f8039c --- .../block_based/block_based_table_builder.cc | 38 +++++++++++-------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/table/block_based/block_based_table_builder.cc b/table/block_based/block_based_table_builder.cc index 189a5dd99..07c545656 100644 --- a/table/block_based/block_based_table_builder.cc +++ b/table/block_based/block_based_table_builder.cc @@ -1501,24 +1501,30 @@ Status BlockBasedTableBuilder::InsertBlockInCache(const Slice& block_contents, const size_t read_amp_bytes_per_bit = rep_->table_options.read_amp_bytes_per_bit; - TBlocklike* block_holder = BlocklikeTraits::Create( - std::move(results), read_amp_bytes_per_bit, - rep_->ioptions.statistics.get(), - false /*rep_->blocks_definitely_zstd_compressed*/, - rep_->table_options.filter_policy.get()); + // TODO akanksha:: Dedup below code by calling + // BlockBasedTable::PutDataBlockToCache. + std::unique_ptr block_holder( + BlocklikeTraits::Create( + std::move(results), read_amp_bytes_per_bit, + rep_->ioptions.statistics.get(), + false /*rep_->blocks_definitely_zstd_compressed*/, + rep_->table_options.filter_policy.get())); - if (block_holder->own_bytes()) { - size_t charge = block_holder->ApproximateMemoryUsage(); - s = block_cache->Insert(key, block_holder, charge, - &DeleteEntryCached); + assert(block_holder->own_bytes()); + size_t charge = block_holder->ApproximateMemoryUsage(); + s = block_cache->Insert( + key, block_holder.get(), + BlocklikeTraits::GetCacheItemHelper(block_type), charge, + nullptr, Cache::Priority::LOW); - if (s.ok()) { - BlockBasedTable::UpdateCacheInsertionMetrics( - block_type, nullptr /*get_context*/, charge, s.IsOkOverwritten(), - rep_->ioptions.stats); - } else { - RecordTick(rep_->ioptions.stats, BLOCK_CACHE_ADD_FAILURES); - } + if (s.ok()) { + // Release ownership of block_holder. + block_holder.release(); + BlockBasedTable::UpdateCacheInsertionMetrics( + block_type, nullptr /*get_context*/, charge, s.IsOkOverwritten(), + rep_->ioptions.stats); + } else { + RecordTick(rep_->ioptions.stats, BLOCK_CACHE_ADD_FAILURES); } } return s;