Skip to content

Commit

Permalink
Merge bitcoin#28186: kernel: Prune leveldb headers
Browse files Browse the repository at this point in the history
d8f1222 refactor: Correct dbwrapper key naming (TheCharlatan)
be8f159 build: Remove leveldb from BITCOIN_INCLUDES (TheCharlatan)
c95b37d refactor: Move CDBWrapper leveldb members to their own context struct (TheCharlatan)
c534a61 refactor: Split dbwrapper CDBWrapper::EstimateSize implementation (TheCharlatan)
5864488 refactor: Move HandleError to dbwrapper implementation (TheCharlatan)
dede0ee refactor: Split dbwrapper CDBWrapper::Exists implementation (TheCharlatan)
a5c2eb5 refactor: Fix logging.h includes (TheCharlatan)
84058e0 refactor: Split dbwrapper CDBWrapper::Read implementation (TheCharlatan)
e4af240 refactor: Pimpl leveldb::Iterator for CDBIterator (TheCharlatan)
ef941ff refactor: Split dbwrapper CDBIterator::GetValue implementation (TheCharlatan)
b7a1ab5 refactor: Split dbwrapper CDBIterator::GetKey implementation (TheCharlatan)
d743790 refactor: Split dbwrapper CDBIterator::Seek implementation (TheCharlatan)
ea8135d refactor: Pimpl leveldb::batch for CDBBatch (TheCharlatan)
b9870c9 refactor: Split dbwrapper CDBatch::Erase implementation (TheCharlatan)
532ee81 refactor: Split dbwrapper CDBBatch::Write implementation (TheCharlatan)
afc534d refactor: Wrap DestroyDB in dbwrapper helper (TheCharlatan)

Pull request description:

  Leveldb headers are currently included in the `dbwrapper.h` file and thus available to many of Bitcoin Core's source files. However, leveldb-specific functionality should be abstracted by the `dbwrapper` and does not need to be available to the rest of the code. Having leveldb included in a widely-used header such as `dbwrapper.h` bloats the entire project's header tree.

  The `dbwrapper` is a key component of the libbitcoinkernel library. Future users of this library would not want to contend with  having the leveldb headers exposed and potentially polluting their project's namespace.

  For these reasons, the leveldb headers are removed from the `dbwrapper` by moving leveldb-specific code to the implementation file and creating a [pimpl](https://en.cppreference.com/w/cpp/language/pimpl) where leveldb member variables are indispensable. As a final step, the leveldb include flags are removed from the `BITCOIN_INCLUDES` and moved to places where the dbwrapper is compiled.

  ---

  This pull request is part of the [libbitcoinkernel project](bitcoin#27587), and more specifically its stage 1 step 3 "Decouple most non-consensus headers from libbitcoinkernel".

ACKs for top commit:
  stickies-v:
    re-ACK bitcoin@d8f1222
  MarcoFalke:
    ACK d8f1222  🔠

Tree-SHA512: 0f58309be165af0162e648233451cd80fda88726fc10c0da7bfe4ec2ffa9afe63fbf7ffae9493698d3f39653b4ad870c372eee652ecc90ab1c29d86c387070f3
  • Loading branch information
fanquake committed Aug 7, 2023
2 parents 064919e + d8f1222 commit b565485
Show file tree
Hide file tree
Showing 15 changed files with 254 additions and 159 deletions.
4 changes: 2 additions & 2 deletions src/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ check_PROGRAMS =
TESTS =
BENCHMARKS =

BITCOIN_INCLUDES=-I$(builddir) -I$(srcdir)/$(MINISKETCH_INCLUDE_DIR_INT) -I$(srcdir)/secp256k1/include -I$(srcdir)/$(UNIVALUE_INCLUDE_DIR_INT) $(LEVELDB_CPPFLAGS)
BITCOIN_INCLUDES=-I$(builddir) -I$(srcdir)/$(MINISKETCH_INCLUDE_DIR_INT) -I$(srcdir)/secp256k1/include -I$(srcdir)/$(UNIVALUE_INCLUDE_DIR_INT)

LIBBITCOIN_NODE=libbitcoin_node.a
LIBBITCOIN_COMMON=libbitcoin_common.a
Expand Down Expand Up @@ -370,7 +370,7 @@ obj/build.h: FORCE
libbitcoin_util_a-clientversion.$(OBJEXT): obj/build.h

# node #
libbitcoin_node_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(BOOST_CPPFLAGS) $(MINIUPNPC_CPPFLAGS) $(NATPMP_CPPFLAGS) $(EVENT_CFLAGS) $(EVENT_PTHREADS_CFLAGS)
libbitcoin_node_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(LEVELDB_CPPFLAGS) $(BOOST_CPPFLAGS) $(MINIUPNPC_CPPFLAGS) $(NATPMP_CPPFLAGS) $(EVENT_CFLAGS) $(EVENT_PTHREADS_CFLAGS)
libbitcoin_node_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
libbitcoin_node_a_SOURCES = \
addrdb.cpp \
Expand Down
1 change: 1 addition & 0 deletions src/blockencodings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <consensus/validation.h>
#include <crypto/sha256.h>
#include <crypto/siphash.h>
#include <logging.h>
#include <random.h>
#include <streams.h>
#include <txmempool.h>
Expand Down
233 changes: 192 additions & 41 deletions src/dbwrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@

#include <dbwrapper.h>

#include <clientversion.h>
#include <logging.h>
#include <random.h>
#include <tinyformat.h>
#include <serialize.h>
#include <span.h>
#include <streams.h>
#include <util/fs.h>
#include <util/fs_helpers.h>
#include <util/strencodings.h>
Expand All @@ -23,9 +26,31 @@
#include <leveldb/helpers/memenv/memenv.h>
#include <leveldb/iterator.h>
#include <leveldb/options.h>
#include <leveldb/slice.h>
#include <leveldb/status.h>
#include <leveldb/write_batch.h>
#include <memory>
#include <optional>
#include <utility>

static auto CharCast(const std::byte* data) { return reinterpret_cast<const char*>(data); }

bool DestroyDB(const std::string& path_str)
{
return leveldb::DestroyDB(path_str, {}).ok();
}

/** Handle database error by throwing dbwrapper_error exception.
*/
static void HandleError(const leveldb::Status& status)
{
if (status.ok())
return;
const std::string errmsg = "Fatal LevelDB error: " + status.ToString();
LogPrintf("%s\n", errmsg);
LogPrintf("You can use -debug=leveldb to get more complete diagnostic messages\n");
throw dbwrapper_error(errmsg);
}

class CBitcoinLevelDBLogger : public leveldb::Logger {
public:
Expand Down Expand Up @@ -127,24 +152,91 @@ static leveldb::Options GetOptions(size_t nCacheSize)
return options;
}

struct CDBBatch::WriteBatchImpl {
leveldb::WriteBatch batch;
};

CDBBatch::CDBBatch(const CDBWrapper& _parent) : parent(_parent),
m_impl_batch{std::make_unique<CDBBatch::WriteBatchImpl>()},
ssValue(SER_DISK, CLIENT_VERSION){};

CDBBatch::~CDBBatch() = default;

void CDBBatch::Clear()
{
m_impl_batch->batch.Clear();
size_estimate = 0;
}

void CDBBatch::WriteImpl(Span<const std::byte> key, CDataStream& ssValue)
{
leveldb::Slice slKey(CharCast(key.data()), key.size());
ssValue.Xor(dbwrapper_private::GetObfuscateKey(parent));
leveldb::Slice slValue(CharCast(ssValue.data()), ssValue.size());
m_impl_batch->batch.Put(slKey, slValue);
// LevelDB serializes writes as:
// - byte: header
// - varint: key length (1 byte up to 127B, 2 bytes up to 16383B, ...)
// - byte[]: key
// - varint: value length
// - byte[]: value
// The formula below assumes the key and value are both less than 16k.
size_estimate += 3 + (slKey.size() > 127) + slKey.size() + (slValue.size() > 127) + slValue.size();
}

void CDBBatch::EraseImpl(Span<const std::byte> key)
{
leveldb::Slice slKey(CharCast(key.data()), key.size());
m_impl_batch->batch.Delete(slKey);
// LevelDB serializes erases as:
// - byte: header
// - varint: key length
// - byte[]: key
// The formula below assumes the key is less than 16kB.
size_estimate += 2 + (slKey.size() > 127) + slKey.size();
}

struct LevelDBContext {
//! custom environment this database is using (may be nullptr in case of default environment)
leveldb::Env* penv;

//! database options used
leveldb::Options options;

//! options used when reading from the database
leveldb::ReadOptions readoptions;

//! options used when iterating over values of the database
leveldb::ReadOptions iteroptions;

//! options used when writing to the database
leveldb::WriteOptions writeoptions;

//! options used when sync writing to the database
leveldb::WriteOptions syncoptions;

//! the database itself
leveldb::DB* pdb;
};

CDBWrapper::CDBWrapper(const DBParams& params)
: m_name{fs::PathToString(params.path.stem())}, m_path{params.path}, m_is_memory{params.memory_only}
: m_db_context{std::make_unique<LevelDBContext>()}, m_name{fs::PathToString(params.path.stem())}, m_path{params.path}, m_is_memory{params.memory_only}
{
penv = nullptr;
readoptions.verify_checksums = true;
iteroptions.verify_checksums = true;
iteroptions.fill_cache = false;
syncoptions.sync = true;
options = GetOptions(params.cache_bytes);
options.create_if_missing = true;
DBContext().penv = nullptr;
DBContext().readoptions.verify_checksums = true;
DBContext().iteroptions.verify_checksums = true;
DBContext().iteroptions.fill_cache = false;
DBContext().syncoptions.sync = true;
DBContext().options = GetOptions(params.cache_bytes);
DBContext().options.create_if_missing = true;
if (params.memory_only) {
penv = leveldb::NewMemEnv(leveldb::Env::Default());
options.env = penv;
DBContext().penv = leveldb::NewMemEnv(leveldb::Env::Default());
DBContext().options.env = DBContext().penv;
} else {
if (params.wipe_data) {
LogPrintf("Wiping LevelDB in %s\n", fs::PathToString(params.path));
leveldb::Status result = leveldb::DestroyDB(fs::PathToString(params.path), options);
dbwrapper_private::HandleError(result);
leveldb::Status result = leveldb::DestroyDB(fs::PathToString(params.path), DBContext().options);
HandleError(result);
}
TryCreateDirectories(params.path);
LogPrintf("Opening LevelDB in %s\n", fs::PathToString(params.path));
Expand All @@ -153,13 +245,13 @@ CDBWrapper::CDBWrapper(const DBParams& params)
// because on POSIX leveldb passes the byte string directly to ::open(), and
// on Windows it converts from UTF-8 to UTF-16 before calling ::CreateFileW
// (see env_posix.cc and env_windows.cc).
leveldb::Status status = leveldb::DB::Open(options, fs::PathToString(params.path), &pdb);
dbwrapper_private::HandleError(status);
leveldb::Status status = leveldb::DB::Open(DBContext().options, fs::PathToString(params.path), &DBContext().pdb);
HandleError(status);
LogPrintf("Opened LevelDB successfully\n");

if (params.options.force_compact) {
LogPrintf("Starting database compaction of %s\n", fs::PathToString(params.path));
pdb->CompactRange(nullptr, nullptr);
DBContext().pdb->CompactRange(nullptr, nullptr);
LogPrintf("Finished database compaction of %s\n", fs::PathToString(params.path));
}

Expand All @@ -185,16 +277,16 @@ CDBWrapper::CDBWrapper(const DBParams& params)

CDBWrapper::~CDBWrapper()
{
delete pdb;
pdb = nullptr;
delete options.filter_policy;
options.filter_policy = nullptr;
delete options.info_log;
options.info_log = nullptr;
delete options.block_cache;
options.block_cache = nullptr;
delete penv;
options.env = nullptr;
delete DBContext().pdb;
DBContext().pdb = nullptr;
delete DBContext().options.filter_policy;
DBContext().options.filter_policy = nullptr;
delete DBContext().options.info_log;
DBContext().options.info_log = nullptr;
delete DBContext().options.block_cache;
DBContext().options.block_cache = nullptr;
delete DBContext().penv;
DBContext().options.env = nullptr;
}

bool CDBWrapper::WriteBatch(CDBBatch& batch, bool fSync)
Expand All @@ -204,8 +296,8 @@ bool CDBWrapper::WriteBatch(CDBBatch& batch, bool fSync)
if (log_memory) {
mem_before = DynamicMemoryUsage() / 1024.0 / 1024;
}
leveldb::Status status = pdb->Write(fSync ? syncoptions : writeoptions, &batch.batch);
dbwrapper_private::HandleError(status);
leveldb::Status status = DBContext().pdb->Write(fSync ? DBContext().syncoptions : DBContext().writeoptions, &batch.m_impl_batch->batch);
HandleError(status);
if (log_memory) {
double mem_after = DynamicMemoryUsage() / 1024.0 / 1024;
LogPrint(BCLog::LEVELDB, "WriteBatch memory usage: db=%s, before=%.1fMiB, after=%.1fMiB\n",
Expand All @@ -218,7 +310,7 @@ size_t CDBWrapper::DynamicMemoryUsage() const
{
std::string memory;
std::optional<size_t> parsed;
if (!pdb->GetProperty("leveldb.approximate-memory-usage", &memory) || !(parsed = ToIntegral<size_t>(memory))) {
if (!DBContext().pdb->GetProperty("leveldb.approximate-memory-usage", &memory) || !(parsed = ToIntegral<size_t>(memory))) {
LogPrint(BCLog::LEVELDB, "Failed to get approximate-memory-usage property\n");
return 0;
}
Expand All @@ -244,30 +336,89 @@ std::vector<unsigned char> CDBWrapper::CreateObfuscateKey() const
return ret;
}

std::optional<std::string> CDBWrapper::ReadImpl(Span<const std::byte> key) const
{
leveldb::Slice slKey(CharCast(key.data()), key.size());
std::string strValue;
leveldb::Status status = DBContext().pdb->Get(DBContext().readoptions, slKey, &strValue);
if (!status.ok()) {
if (status.IsNotFound())
return std::nullopt;
LogPrintf("LevelDB read failure: %s\n", status.ToString());
HandleError(status);
}
return strValue;
}

bool CDBWrapper::ExistsImpl(Span<const std::byte> key) const
{
leveldb::Slice slKey(CharCast(key.data()), key.size());

std::string strValue;
leveldb::Status status = DBContext().pdb->Get(DBContext().readoptions, slKey, &strValue);
if (!status.ok()) {
if (status.IsNotFound())
return false;
LogPrintf("LevelDB read failure: %s\n", status.ToString());
HandleError(status);
}
return true;
}

size_t CDBWrapper::EstimateSizeImpl(Span<const std::byte> key1, Span<const std::byte> key2) const
{
leveldb::Slice slKey1(CharCast(key1.data()), key1.size());
leveldb::Slice slKey2(CharCast(key2.data()), key2.size());
uint64_t size = 0;
leveldb::Range range(slKey1, slKey2);
DBContext().pdb->GetApproximateSizes(&range, 1, &size);
return size;
}

bool CDBWrapper::IsEmpty()
{
std::unique_ptr<CDBIterator> it(NewIterator());
it->SeekToFirst();
return !(it->Valid());
}

CDBIterator::~CDBIterator() { delete piter; }
bool CDBIterator::Valid() const { return piter->Valid(); }
void CDBIterator::SeekToFirst() { piter->SeekToFirst(); }
void CDBIterator::Next() { piter->Next(); }
struct CDBIterator::IteratorImpl {
const std::unique_ptr<leveldb::Iterator> iter;

namespace dbwrapper_private {
explicit IteratorImpl(leveldb::Iterator* _iter) : iter{_iter} {}
};

void HandleError(const leveldb::Status& status)
CDBIterator::CDBIterator(const CDBWrapper& _parent, std::unique_ptr<IteratorImpl> _piter) : parent(_parent),
m_impl_iter(std::move(_piter)) {}

CDBIterator* CDBWrapper::NewIterator()
{
if (status.ok())
return;
const std::string errmsg = "Fatal LevelDB error: " + status.ToString();
LogPrintf("%s\n", errmsg);
LogPrintf("You can use -debug=leveldb to get more complete diagnostic messages\n");
throw dbwrapper_error(errmsg);
return new CDBIterator{*this, std::make_unique<CDBIterator::IteratorImpl>(DBContext().pdb->NewIterator(DBContext().iteroptions))};
}

void CDBIterator::SeekImpl(Span<const std::byte> key)
{
leveldb::Slice slKey(CharCast(key.data()), key.size());
m_impl_iter->iter->Seek(slKey);
}

Span<const std::byte> CDBIterator::GetKeyImpl() const
{
return MakeByteSpan(m_impl_iter->iter->key());
}

Span<const std::byte> CDBIterator::GetValueImpl() const
{
return MakeByteSpan(m_impl_iter->iter->value());
}

CDBIterator::~CDBIterator() = default;
bool CDBIterator::Valid() const { return m_impl_iter->iter->Valid(); }
void CDBIterator::SeekToFirst() { m_impl_iter->iter->SeekToFirst(); }
void CDBIterator::Next() { m_impl_iter->iter->Next(); }

namespace dbwrapper_private {

const std::vector<unsigned char>& GetObfuscateKey(const CDBWrapper &w)
{
return w.obfuscate_key;
Expand Down
Loading

0 comments on commit b565485

Please sign in to comment.