1
0
mirror of https://github.com/facebook/rocksdb.git synced 2026-10-23 08:09:02 +03:00
Files
rocksdb/table/two_level_iterator.cc
Josh Kang 4b983da055 Spell out special member functions (rule of five) (#15387)
Summary:
Pull Request resolved: https://github.com/facebook/rocksdb/pull/15387

## Summary

RocksDB's `.clang-tidy` enables `cppcoreguidelines-special-member-functions`,
`cppcoreguidelines-virtual-class-destructor` and the noexcept-move checks,
and clang warns `-Wdeprecated-copy-with-user-provided-dtor`. A full scan
reports 476 findings. In most of them, a class declares a destructor (or
copy operations) but leaves the rest of copy/move implicit. That has two
costs. Readers can't tell whether a class may be copied or moved. And
resource-owning classes keep an implicit copy that would double-free if
anyone ever used it. This diff resolves 453 of the 476.

Approach, picked per class from least to most invasive:
1. An empty user-provided destructor becomes `= default`. This covers most
   interface classes, such as `virtual ~X() {}` -> `= default`. If the
   class is still copied, that would only rename the deprecated-copy
   warning, so the destructor is removed instead when nothing needs it
   (`Customizable`, `TableFactory` and `InternalKeyComparator` inherit a
   virtual one; `IteratorWrapperBase` isn't polymorphic), and
   `MemTableRep::KeyComparator` gets defaulted copy and move.
2. Classes whose copy is already deleted get explicitly deleted move
   operations too. Move was already unavailable, so this only documents
   it.
3. Resource-owning classes that are never copied get deleted copy and move
   operations. Examples: the C API wrapper structs, which run a
   `destructor_` callback (edited in `tools/c_api_gen/c_base.cc` and
   regenerated into `db/c.cc`); `ShardedCache`, which frees its shards;
   iterator wrappers; and RAII handles.
4. Value types that are copied or moved get the missing operations as
   `= default`, which keeps their current behavior.
5. Exception specifications of existing functions are unchanged, except
   for `WriteBatch`'s move assignment (below): no other `noexcept` is
   added or removed. Only the move operations this diff
   newly declares (`Comparator`, `FileSampledStats` and the thread-local
   `Entry`) are `noexcept`; they copy only scalars or atomics.

`WriteBatch`'s copy and move assignment destroyed `*this` and
placement-new'ed a new `WriteBatch` over it. That is undefined behavior
when `*this` is a subclass, and if the copy constructor threw, `*this`
was left destroyed and was later destroyed again. Move assignment now
assigns member-wise and ends in the same state as the move constructor,
which leaves `has_key_with_ts_`, the timestamp-size tracking and the
other fields it doesn't copy at their defaults. `rep_` is swapped through
a temporary, because a plain string move-assignment can keep the old
buffer (e.g. a large batch reset with `batch = WriteBatch()` would keep
its memory). Nothing in it can throw, so it is now `noexcept`. Copy
assignment copies into a temporary and moves it in, so a throwing copy
leaves `*this` unchanged.

Bugs this exposed and fixed:
- `RestoreAfterCopyOrCreateWorkItem` (backup engine): the hand-written
  move constructor and assignment moved only some fields, so `from_file`
  and `to_file` were empty after `push_back(std::move(...))`. A restore
  that hit a checksum mismatch then reported "While restoring  -> : ..."
  without file names. It now uses the compiler-generated moves, which
  are still `noexcept`.
- `ExternalSstFileIngestionJob` lived in a `std::vector`, and its
  destructor frees raw-owned pointers, so the implicit copy used on
  vector reallocation would double-free. Its file batches also point into
  its own inline file storage, so neither a copy nor a move is safe after
  `Prepare()`. Copy and move are now deleted, and `DBImpl` holds jobs
  through `std::unique_ptr`.

Compatibility: no public class loses an operation users can call. Public
interfaces mostly only get `virtual ~X() = default;`.
`WriteBatch`'s move assignment becomes `noexcept`, which is not part of
the mangled name, so the ABI doesn't change.
`SubcompactionJobInfo` and `CompactionJobInfo` intentionally stay C++20
aggregates, so users' aggregate and designated initializers keep
compiling. `Configurable`, `CompareInterface`, `WriteBatchBase`,
`ObjectLibrary::Entry` and `MemTableRep::KeyComparator` declare defaulted
copy and move, and `Customizable` no longer declares a destructor. As a
result, `Configurable` and `Customizable` subclasses that declare no
destructor or copy operations of their own now move their
registered-options list instead of copying it. In-tree, these are the
mutable-options adapters, the table factories, the memory allocators and
test classes; all are held through smart pointers or used in place, and
`Clone()` copies, so none is moved by value.

Not changed (23): `IODebugContext` (custom deep copy plus a
`std::shared_mutex`), `DBWithColumnFamilies` in db_bench (stored in a
`std::vector` with non-movable members), `Snapshot`'s protected virtual
destructor (intended), `ManagedSnapshot`, the `SubcompactionJobInfo` and
`CompactionJobInfo` aggregates, and the 16 noexcept-move-constructor and
noexcept-swap findings on existing functions, including `autovector`'s
move assignment (adding `noexcept` would change their exception
specifications). Copying `ManagedSnapshot` already double-releases the
snapshot. Fixing that needs a real move implementation and is a behavior
change, so it is left for a separate diff.

Reviewed By: rban1

Differential Revision: D123186309

fbshipit-source-id: 6f8e57e233e12199a007407130bb9a1f6d52e8d1
2026-10-12 16:00:09 -07:00

229 lines
6.9 KiB
C++

// Copyright (c) 2011-present, Facebook, Inc. All rights reserved.
// This source code is licensed under both the GPLv2 (found in the
// COPYING file in the root directory) and Apache 2.0 License
// (found in the LICENSE.Apache file in the root directory).
//
// Copyright (c) 2011 The LevelDB Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. See the AUTHORS file for names of contributors.
#include "table/two_level_iterator.h"
#include "db/pinned_iterators_manager.h"
#include "memory/arena.h"
#include "rocksdb/options.h"
#include "rocksdb/table.h"
#include "table/block_based/block.h"
#include "table/format.h"
namespace ROCKSDB_NAMESPACE {
namespace {
class TwoLevelIndexIterator : public InternalIteratorBase<IndexValue> {
public:
explicit TwoLevelIndexIterator(
TwoLevelIteratorState* state,
InternalIteratorBase<IndexValue>* first_level_iter);
// No copying allowed
TwoLevelIndexIterator(const TwoLevelIndexIterator&) = delete;
TwoLevelIndexIterator& operator=(const TwoLevelIndexIterator&) = delete;
TwoLevelIndexIterator(TwoLevelIndexIterator&&) = delete;
TwoLevelIndexIterator& operator=(TwoLevelIndexIterator&&) = delete;
~TwoLevelIndexIterator() override {
first_level_iter_.DeleteIter(false /* is_arena_mode */);
second_level_iter_.DeleteIter(false /* is_arena_mode */);
delete state_;
}
void Seek(const Slice& target) override;
void SeekForPrev(const Slice& target) override;
void SeekToFirst() override;
void SeekToLast() override;
void Next() override;
void Prev() override;
bool Valid() const override { return second_level_iter_.Valid(); }
Slice key() const override {
assert(Valid());
return second_level_iter_.key();
}
Slice user_key() const override {
assert(Valid());
return second_level_iter_.user_key();
}
IndexValue value() const override {
assert(Valid());
return second_level_iter_.value();
}
Status status() const override {
if (!first_level_iter_.status().ok()) {
assert(second_level_iter_.iter() == nullptr);
return first_level_iter_.status();
} else if (second_level_iter_.iter() != nullptr &&
!second_level_iter_.status().ok()) {
return second_level_iter_.status();
} else {
return status_;
}
}
void SetPinnedItersMgr(
PinnedIteratorsManager* /*pinned_iters_mgr*/) override {}
bool IsKeyPinned() const override { return false; }
bool IsValuePinned() const override { return false; }
private:
void SaveError(const Status& s) {
if (status_.ok() && !s.ok()) {
status_ = s;
}
}
void SkipEmptyDataBlocksForward();
void SkipEmptyDataBlocksBackward();
void SetSecondLevelIterator(InternalIteratorBase<IndexValue>* iter);
void InitDataBlock();
TwoLevelIteratorState* state_;
IteratorWrapperBase<IndexValue> first_level_iter_;
IteratorWrapperBase<IndexValue> second_level_iter_; // May be nullptr
Status status_;
// If second_level_iter is non-nullptr, then "data_block_handle_" holds the
// "index_value" passed to block_function_ to create the second_level_iter.
BlockHandle data_block_handle_;
};
TwoLevelIndexIterator::TwoLevelIndexIterator(
TwoLevelIteratorState* state,
InternalIteratorBase<IndexValue>* first_level_iter)
: state_(state), first_level_iter_(first_level_iter) {}
void TwoLevelIndexIterator::Seek(const Slice& target) {
first_level_iter_.Seek(target);
InitDataBlock();
if (second_level_iter_.iter() != nullptr) {
second_level_iter_.Seek(target);
}
SkipEmptyDataBlocksForward();
}
void TwoLevelIndexIterator::SeekForPrev(const Slice& target) {
first_level_iter_.Seek(target);
InitDataBlock();
if (second_level_iter_.iter() != nullptr) {
second_level_iter_.SeekForPrev(target);
}
if (!Valid()) {
if (!first_level_iter_.Valid() && first_level_iter_.status().ok()) {
first_level_iter_.SeekToLast();
InitDataBlock();
if (second_level_iter_.iter() != nullptr) {
second_level_iter_.SeekForPrev(target);
}
}
SkipEmptyDataBlocksBackward();
}
}
void TwoLevelIndexIterator::SeekToFirst() {
first_level_iter_.SeekToFirst();
InitDataBlock();
if (second_level_iter_.iter() != nullptr) {
second_level_iter_.SeekToFirst();
}
SkipEmptyDataBlocksForward();
}
void TwoLevelIndexIterator::SeekToLast() {
first_level_iter_.SeekToLast();
InitDataBlock();
if (second_level_iter_.iter() != nullptr) {
second_level_iter_.SeekToLast();
}
SkipEmptyDataBlocksBackward();
}
void TwoLevelIndexIterator::Next() {
assert(Valid());
second_level_iter_.Next();
SkipEmptyDataBlocksForward();
}
void TwoLevelIndexIterator::Prev() {
assert(Valid());
second_level_iter_.Prev();
SkipEmptyDataBlocksBackward();
}
void TwoLevelIndexIterator::SkipEmptyDataBlocksForward() {
while (second_level_iter_.iter() == nullptr ||
(!second_level_iter_.Valid() && second_level_iter_.status().ok())) {
// Move to next block
if (!first_level_iter_.Valid()) {
SetSecondLevelIterator(nullptr);
return;
}
first_level_iter_.Next();
InitDataBlock();
if (second_level_iter_.iter() != nullptr) {
second_level_iter_.SeekToFirst();
}
}
}
void TwoLevelIndexIterator::SkipEmptyDataBlocksBackward() {
while (second_level_iter_.iter() == nullptr ||
(!second_level_iter_.Valid() && second_level_iter_.status().ok())) {
// Move to next block
if (!first_level_iter_.Valid()) {
SetSecondLevelIterator(nullptr);
return;
}
first_level_iter_.Prev();
InitDataBlock();
if (second_level_iter_.iter() != nullptr) {
second_level_iter_.SeekToLast();
}
}
}
void TwoLevelIndexIterator::SetSecondLevelIterator(
InternalIteratorBase<IndexValue>* iter) {
InternalIteratorBase<IndexValue>* old_iter = second_level_iter_.Set(iter);
delete old_iter;
}
void TwoLevelIndexIterator::InitDataBlock() {
if (!first_level_iter_.Valid()) {
SetSecondLevelIterator(nullptr);
} else {
BlockHandle handle = first_level_iter_.value().handle;
if (second_level_iter_.iter() != nullptr &&
!second_level_iter_.status().IsIncomplete() &&
handle.offset() == data_block_handle_.offset()) {
// second_level_iter is already constructed with this iterator, so
// no need to change anything
} else {
InternalIteratorBase<IndexValue>* iter =
state_->NewSecondaryIterator(handle);
data_block_handle_ = handle;
SetSecondLevelIterator(iter);
if (iter == nullptr) {
status_ = Status::Corruption("Missing block for partition " +
handle.ToString());
}
}
}
}
} // namespace
InternalIteratorBase<IndexValue>* NewTwoLevelIterator(
TwoLevelIteratorState* state,
InternalIteratorBase<IndexValue>* first_level_iter) {
return new TwoLevelIndexIterator(state, first_level_iter);
}
} // namespace ROCKSDB_NAMESPACE