Merge bitcoin/bitcoin#35568: txospenderindex: disable bloom filters to optimize disk usage

6d0ea4cf5b doc: add release notes (Andrew Toth)
a2b1c86903 txospenderindex: disable bloom filters to optimize disk usage (Andrew Toth)

Pull request description:

  LevelDB bloom filters are only consulted on `Get` point reads. This can be verified in https://github.com/bitcoin/bitcoin/blob/master/src/leveldb/table/table.cc#L224-L228. `InternalGet` is the only place that consults the filter, and it is only reached via a `Get` or `Exists` point read. The filters are never consulted for iterator seeks with an iterator created via `NewIterator`.
  txospenderindex only reads via iterator seeks, so building them is wasted effort and space.

  For a db as large as txospenderindex, this results in measurable performance and disk usage.
  On master, a full sync took 4h37m, and the resulting db was 85.0 GiB.
  On this branch, a full sync took 3h57m, and the resulting db was 80.9 GiB.
  So this is a sync speedup of 39 minutes (1.17x), and a disk space reduction of 4.2 GiB.

ACKs for top commit:
  l0rinc:
    ACK 6d0ea4cf5b
  sedited:
    Re-ACK 6d0ea4cf5b
  fjahr:
    Code review ACK 6d0ea4cf5b

Tree-SHA512: fb88b9f9a16ff31562d388e3fd9fd9590c7864dbe6093cd9430ecbce9cdc3f2a8d3fc612aade743d26ad4c6eca1e5dc9b3f1ca28d75caea1209e5c784895405d

doc/release-notes-35634.md

@@ -1,9 +1,8 @@
 Indexes
 -------
 
-- The transaction output spender index (`-txospenderindex`) now stores one byte
-  less per spender entry for newly indexed blocks. Existing indexes remain
-  compatible and no action is required. To apply the same saving to entries
-  indexed before upgrading, stop the node, delete the
+- The transaction output spender index (`-txospenderindex`) now uses less disk
+  space. Existing indexes remain compatible and no action is required. To reclaim
+  the space for data indexed before upgrading, stop the node, delete the
   `<datadir>/indexes/txospenderindex/` directory, and restart with
-  `-txospenderindex` enabled to rebuild it.
+  `-txospenderindex` enabled to rebuild it. (#35634, #35568)

src/dbwrapper.cpp

@@ -136,12 +136,12 @@ static void SetMaxOpenFiles(leveldb::Options *options) {
              options->max_open_files, default_open_files);
 }
 
-static leveldb::Options GetOptions(size_t nCacheSize)
+static leveldb::Options GetOptions(size_t nCacheSize, bool bloom_filter)
 {
     leveldb::Options options;
     options.block_cache = leveldb::NewLRUCache(nCacheSize / 2);
     options.write_buffer_size = nCacheSize / 4; // up to two write buffers may be held in memory simultaneously
-    options.filter_policy = leveldb::NewBloomFilterPolicy(10);
+    options.filter_policy = bloom_filter ? leveldb::NewBloomFilterPolicy(10) : nullptr;
     options.compression = leveldb::kNoCompression;
     options.info_log = new CBitcoinLevelDBLogger();
     if (leveldb::kMajorVersion > 1 || (leveldb::kMajorVersion == 1 && leveldb::kMinorVersion >= 16)) {
@@ -225,7 +225,7 @@ CDBWrapper::CDBWrapper(const DBParams& params)
     DBContext().iteroptions.verify_checksums = true;
     DBContext().iteroptions.fill_cache = false;
     DBContext().syncoptions.sync = true;
-    DBContext().options = GetOptions(params.cache_bytes);
+    DBContext().options = GetOptions(params.cache_bytes, params.bloom_filter);
     DBContext().options.create_if_missing = true;
     DBContext().options.max_file_size = params.max_file_size;
     assert(!(params.testing_env && params.memory_only));

src/dbwrapper.h

@@ -50,6 +50,8 @@ struct DBParams {
     //! If true, store data obfuscated via simple XOR. If false, XOR with a
     //! zero'd byte array.
     bool obfuscate = false;
+    //! If true, build a LevelDB bloom filter to accelerate point lookups.
+    bool bloom_filter = true;
     //! Passed-through options.
     DBOptions options{};
     //! If non-null, use this as the leveldb::Env instead of the default.

src/index/base.cpp

@@ -65,13 +65,14 @@ CBlockLocator GetLocator(interfaces::Chain& chain, const uint256& block_hash)
     return locator;
 }
 
-BaseIndex::DB::DB(const fs::path& path, size_t n_cache_size, bool f_memory, bool f_wipe, bool f_obfuscate) :
+BaseIndex::DB::DB(const fs::path& path, size_t n_cache_size, bool f_memory, bool f_wipe, bool f_obfuscate, bool f_bloom) :
     CDBWrapper{DBParams{
         .path = path,
         .cache_bytes = n_cache_size,
         .memory_only = f_memory,
         .wipe_data = f_wipe,
         .obfuscate = f_obfuscate,
+        .bloom_filter = f_bloom,
         .options = [] { DBOptions options; node::ReadDatabaseArgs(gArgs, options); return options; }()}}
 {}
 

src/index/base.h

@@ -65,7 +65,7 @@ protected:
     {
     public:
         DB(const fs::path& path, size_t n_cache_size,
-           bool f_memory = false, bool f_wipe = false, bool f_obfuscate = false);
+           bool f_memory = false, bool f_wipe = false, bool f_obfuscate = false, bool f_bloom = true);
 
         /// Read block locator of the chain that the index is in sync with.
         /// Note, the returned locator will be empty if no record exists.

src/index/txospenderindex.cpp

@@ -62,7 +62,7 @@ struct DBKey {
 };
 
 TxoSpenderIndex::TxoSpenderIndex(std::unique_ptr<interfaces::Chain> chain, size_t n_cache_size, bool f_memory, bool f_wipe)
-    : BaseIndex(std::move(chain), "txospenderindex", "txospenderidx"), m_db{std::make_unique<DB>(gArgs.GetDataDirNet() / "indexes" / "txospenderindex" / "db", n_cache_size, f_memory, f_wipe)}
+    : BaseIndex(std::move(chain), "txospenderindex", "txospenderidx"), m_db{std::make_unique<DB>(gArgs.GetDataDirNet() / "indexes" / "txospenderindex" / "db", n_cache_size, f_memory, f_wipe, /*f_obfuscate=*/false, /*f_bloom=*/false)}
 {
     if (!m_db->Read("siphash_key", m_siphash_key)) {
         FastRandomContext rng(false);

src/test/fuzz/dbwrapper.cpp

@@ -188,6 +188,7 @@ DBParams ConsumeDBParams(FuzzedDataProvider& provider, leveldb::Env* testing_env
         .path = "dbwrapper_fuzz",
         .cache_bytes = provider.ConsumeIntegralInRange<size_t>(64 << 10, 1_MiB),
         .obfuscate = obfuscate,
+        .bloom_filter = provider.ConsumeBool(),
         .options = options,
         .testing_env = testing_env,
         .max_file_size = provider.ConsumeBool()