mirror of
https://github.com/facebook/rocksdb.git
synced 2026-11-07 03:22:34 -05:00
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
215 lines
9.2 KiB
C++
215 lines
9.2 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.
|
|
|
|
#pragma once
|
|
#include <memory>
|
|
|
|
#include "db/range_tombstone_fragmenter.h"
|
|
#if USE_COROUTINES
|
|
#include "folly/coro/Coroutine.h"
|
|
#include "folly/coro/Task.h"
|
|
#endif
|
|
#include "rocksdb/slice_transform.h"
|
|
#include "rocksdb/table_reader_caller.h"
|
|
#include "table/get_context.h"
|
|
#include "table/internal_iterator.h"
|
|
#include "table/multiget_context.h"
|
|
#include "util/coro_utils.h"
|
|
|
|
namespace ROCKSDB_NAMESPACE {
|
|
|
|
class Iterator;
|
|
struct ParsedInternalKey;
|
|
class Slice;
|
|
class Arena;
|
|
struct ReadOptions;
|
|
struct TableProperties;
|
|
class GetContext;
|
|
class MultiGetContext;
|
|
|
|
// A Table (also referred to as SST) is a sorted map from strings to strings.
|
|
// Tables are immutable and persistent. A Table may be safely accessed from
|
|
// multiple threads without external synchronization. Table readers are used
|
|
// for reading various types of table formats supported by rocksdb including
|
|
// BlockBasedTable, PlainTable and CuckooTable format.
|
|
class TableReader {
|
|
public:
|
|
virtual ~TableReader() = default;
|
|
|
|
// Returns a new iterator over the table contents.
|
|
// The result of NewIterator() is initially invalid (caller must
|
|
// call one of the Seek methods on the iterator before using it).
|
|
//
|
|
// read_options: Must outlive the returned iterator.
|
|
// arena: If not null, the arena needs to be used to allocate the Iterator.
|
|
// When destroying the iterator, the caller will not call "delete"
|
|
// but Iterator::~Iterator() directly. The destructor needs to destroy
|
|
// all the states but those allocated in arena.
|
|
// skip_filters: disables checking the bloom filters even if they exist. This
|
|
// option is effective only for block-based table format.
|
|
// compaction_readahead_size: its value will only be used if caller =
|
|
// kCompaction
|
|
virtual InternalIterator* NewIterator(
|
|
const ReadOptions& read_options, const SliceTransform* prefix_extractor,
|
|
Arena* arena, bool skip_filters, TableReaderCaller caller,
|
|
size_t compaction_readahead_size = 0,
|
|
bool allow_unprepared_value = false) = 0;
|
|
|
|
// read_options.snapshot needs to outlive this call.
|
|
virtual FragmentedRangeTombstoneIterator* NewRangeTombstoneIterator(
|
|
const ReadOptions& /*read_options*/) {
|
|
return nullptr;
|
|
}
|
|
|
|
virtual FragmentedRangeTombstoneIterator* NewRangeTombstoneIterator(
|
|
SequenceNumber /* read_seqno */, const Slice* /* timestamp */) {
|
|
return nullptr;
|
|
}
|
|
|
|
// Given a key, return an approximate byte offset in the file where
|
|
// the data for that key begins (or would begin if the key were
|
|
// present in the file). The returned value is in terms of file
|
|
// bytes, and so includes effects like compression of the underlying data.
|
|
// E.g., the approximate offset of the last key in the table will
|
|
// be close to the file length.
|
|
// TODO(peterd): Since this function is only used for approximate size
|
|
// from beginning of file, reduce code duplication by removing this
|
|
// function and letting ApproximateSize take optional start and end, so
|
|
// that absolute start and end can be specified and optimized without
|
|
// key / index work.
|
|
virtual uint64_t ApproximateOffsetOf(const ReadOptions& read_options,
|
|
const Slice& key,
|
|
TableReaderCaller caller) = 0;
|
|
|
|
// Given start and end keys, return the approximate data size in the file
|
|
// between the keys. The returned value is in terms of file bytes, and so
|
|
// includes effects like compression of the underlying data and applicable
|
|
// portions of metadata including filters and indexes. Nullptr for start or
|
|
// end (or both) indicates absolute start or end of the table.
|
|
virtual uint64_t ApproximateSize(const ReadOptions& read_options,
|
|
const Slice& start, const Slice& end,
|
|
TableReaderCaller caller) = 0;
|
|
|
|
struct Anchor {
|
|
Anchor(const Slice& _user_key, size_t _range_size)
|
|
: user_key(_user_key.ToStringView()), range_size(_range_size) {}
|
|
std::string user_key;
|
|
size_t range_size;
|
|
};
|
|
|
|
// Now try to return approximately 128 anchor keys.
|
|
// The last one tends to be the largest key.
|
|
virtual Status ApproximateKeyAnchors(const ReadOptions& /*read_options*/,
|
|
std::vector<Anchor>& /*anchors*/) {
|
|
return Status::NotSupported("ApproximateKeyAnchors() not supported.");
|
|
}
|
|
|
|
// Set up the table for Compaction. Might change some parameters with
|
|
// posix_fadvise
|
|
virtual void SetupForCompaction() = 0;
|
|
|
|
virtual std::shared_ptr<const TableProperties> GetTableProperties() const = 0;
|
|
|
|
// Prepare work that can be done before the real Get()
|
|
virtual void Prepare(const Slice& /*target*/) {}
|
|
|
|
// Report an approximation of how much memory has been used.
|
|
virtual size_t ApproximateMemoryUsage() const = 0;
|
|
|
|
// Calls get_context->SaveValue() repeatedly, starting with
|
|
// the entry found after a call to Seek(key), until it returns false.
|
|
// May not make such a call if filter policy says that key is not present.
|
|
//
|
|
// get_context->MarkKeyMayExist needs to be called when it is configured to be
|
|
// memory only and the key is not found in the block cache.
|
|
//
|
|
// readOptions is the options for the read
|
|
// key is the key to search for
|
|
// skip_filters: disables checking the bloom filters even if they exist. This
|
|
// option is effective only for block-based table format.
|
|
virtual Status Get(const ReadOptions& readOptions, const Slice& key,
|
|
GetContext* get_context,
|
|
const SliceTransform* prefix_extractor,
|
|
bool skip_filters = false) = 0;
|
|
|
|
#if USE_COROUTINES
|
|
virtual folly::coro::Task<Status> GetCoroutine(
|
|
const ReadOptions& readOptions, const Slice& key, GetContext* get_context,
|
|
const SliceTransform* prefix_extractor, bool skip_filters = false) {
|
|
co_return Get(readOptions, key, get_context, prefix_extractor,
|
|
skip_filters);
|
|
}
|
|
#endif // USE_COROUTINES
|
|
|
|
// Use bloom filters in the table file, if present, to filter out keys. The
|
|
// mget_range will be updated to skip keys that get a negative result from
|
|
// the filter lookup.
|
|
virtual Status MultiGetFilter(const ReadOptions& /*readOptions*/,
|
|
const SliceTransform* /*prefix_extractor*/,
|
|
MultiGetContext::Range* /*mget_range*/) {
|
|
return Status::NotSupported();
|
|
}
|
|
|
|
virtual void MultiGet(const ReadOptions& readOptions,
|
|
const MultiGetContext::Range* mget_range,
|
|
const SliceTransform* prefix_extractor,
|
|
bool skip_filters = false) {
|
|
for (auto iter = mget_range->begin(); iter != mget_range->end(); ++iter) {
|
|
*iter->s = Get(readOptions, iter->ikey, iter->get_context,
|
|
prefix_extractor, skip_filters);
|
|
}
|
|
}
|
|
|
|
#if USE_COROUTINES
|
|
virtual folly::coro::Task<void> MultiGetCoroutine(
|
|
const ReadOptions& readOptions, const MultiGetContext::Range* mget_range,
|
|
const SliceTransform* prefix_extractor, bool skip_filters = false) {
|
|
MultiGet(readOptions, mget_range, prefix_extractor, skip_filters);
|
|
co_return;
|
|
}
|
|
#endif // USE_COROUTINES
|
|
|
|
// Prefetch data corresponding to a give range of keys
|
|
// Typically this functionality is required for table implementations that
|
|
// persists the data on a non volatile storage medium like disk/SSD
|
|
virtual Status Prefetch(const ReadOptions& /* read_options */,
|
|
const Slice* begin = nullptr,
|
|
const Slice* end = nullptr) {
|
|
(void)begin;
|
|
(void)end;
|
|
// Default implementation is NOOP.
|
|
// The child class should implement functionality when applicable
|
|
return Status::OK();
|
|
}
|
|
|
|
// convert db file to a human readable form
|
|
virtual Status DumpTable(WritableFile* /*out_file*/,
|
|
bool /*show_sequence_number_type*/ = false) {
|
|
return Status::NotSupported("DumpTable() not supported");
|
|
}
|
|
|
|
// check whether there is corruption in this db file
|
|
virtual Status VerifyChecksum(const ReadOptions& /*read_options*/,
|
|
TableReaderCaller /*caller*/,
|
|
bool /*meta_blocks_only*/ = false) {
|
|
return Status::NotSupported("VerifyChecksum() not supported");
|
|
}
|
|
|
|
// Tell the reader that the file should now be obsolete, e.g. as a hint
|
|
// to delete relevant cache entries on destruction. (It might not be safe
|
|
// to "unpin" cache entries until destruction time.) NOTE: must be thread
|
|
// safe because multiple table cache references might all mark this file as
|
|
// obsolete when they are released (the last of which destroys this reader).
|
|
virtual void MarkObsolete(uint32_t /*uncache_aggressiveness*/) {
|
|
// no-op as default
|
|
}
|
|
};
|
|
|
|
} // namespace ROCKSDB_NAMESPACE
|