Update leveldb subtree to latest upstream #31671

pull fanquake wants to merge 3 commits into bitcoin:master from fanquake:update_leveldb_subtree changing 8 files +8 −63
  2. Squashed 'src/leveldb/' changes from 688561cba8..04b5790928 
    04b5790928 Merge bitcoin-core/leveldb-subtree#46: Fix invalid pointer arithmetic in Hash (#1222)
59669817c5 Merge bitcoin-core/leveldb-subtree#40: cherry-pick: Remove leveldb::port::kLittleEndian.
73013d1a37 Merge bitcoin-core/leveldb-subtree#45: [jumbo] Add begin()/end() to Slice.
a8844b23ab Fix invalid pointer arithmetic in Hash (#1222)
be4dfc94b3 [jumbo] Add begin()/end() to Slice.
2e3c0131d3 Remove leveldb::port::kLittleEndian.

git-subtree-dir: src/leveldb
git-subtree-split: 04b57909285c7335c1908d53bcde9b90fe0439be
    d336b7ab85
  3. Update leveldb subtree to latest upstream 9ec64253ab
  4. build: remove LEVELDB_IS_BIG_ENDIAN 910a11fa66
  5. DrahtBot commented at 11:13 am on January 16, 2025: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31671.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK hebasto, l0rinc, theuni
    Concept ACK kevkevinpal

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

  6. l0rinc commented at 11:51 am on January 16, 2025: contributor

    Could you share more about how these changes were selected. I noticed there have been many (useless) refactorings in LevelDB since the last stable version we’re using. While I completely agree that cherry-picking every update may not align with our goals (especially for many of the high-risk, low-reward changes), a few targeted updates addressing critical issues might still be worth considering:

    • fb644cb: Fixes #1081 - [BUG] LevelDB data loss after a crash when deployed on GlusterFS.
    • eb31d19: Improves TEST_CompactRange by allowing compactions to finish.
    • 8a68093: Ensures VersionEdit::Clear() also clears compact_pointers_.

    Have these been considered? These seem relatively low-risk and possibly high-reward - and could help improve reliability or edge-case behavior.

  7. kevkevinpal commented at 12:40 pm on January 16, 2025: contributor

    Concept ACK 910a11f

    I think it makes sense to update to the leveldb latest upstream, I also agree with @l0rinc there might be some other changes that might be worth considering.

    I also just validated that this change only includes these 3 changes https://github.com/bitcoin-core/leveldb-subtree/pull/40, https://github.com/bitcoin-core/leveldb-subtree/pull/45, https://github.com/bitcoin-core/leveldb-subtree/pull/46

    Minus the removal of LEVELDB_IS_BIG_ENDIAN in cmake/leveldb.cmake

  8. fanquake commented at 2:46 pm on January 17, 2025: member

    Could you share more about how these changes were selected. might still be worth considering:

    They were PR’d, with some rationale, to https://github.com/bitcoin-core/leveldb-subtree/. This PR is just updating to the current state of the subtree, but if you think there are other worthwhile changes that should be looked at, I’d suggest opening a PR upstream for discussion (that would have to be the case anyways, before they could be part of this PR).

  9. fanquake commented at 3:27 pm on January 17, 2025: member

    Guix Build (aarch64):

     0139048cf62397ec98d6863fc1706a3c7c2afd02307873ec9a9bd707c531283a6  guix-build-910a11fa6630/output/aarch64-linux-gnu/SHA256SUMS.part
 17d2de717c990cbc3f7da29291e8de257e610a5a79b6a351f00b9c9cad4e8ed65  guix-build-910a11fa6630/output/aarch64-linux-gnu/bitcoin-910a11fa6630-aarch64-linux-gnu-debug.tar.gz
 2c9b45a7c734d6cba6f736cbc53cea1dd207600133eaa2e82f58fa4dc96bd21c9  guix-build-910a11fa6630/output/aarch64-linux-gnu/bitcoin-910a11fa6630-aarch64-linux-gnu.tar.gz
 38c11bbb0fda0e16bbf7b3f9f1aed1708f8271e827dabe0365be8a8667fe58d0f  guix-build-910a11fa6630/output/arm-linux-gnueabihf/SHA256SUMS.part
 4fee2d0ea352b712dfa76164b01703974e3244e4ac6f896de47d23e4fa3d2589c  guix-build-910a11fa6630/output/arm-linux-gnueabihf/bitcoin-910a11fa6630-arm-linux-gnueabihf-debug.tar.gz
 571d11b361378211413c2ff4376031e86d06ce3ee480c3766a3661c0551c1a41a  guix-build-910a11fa6630/output/arm-linux-gnueabihf/bitcoin-910a11fa6630-arm-linux-gnueabihf.tar.gz
 61daf51607a74af072e4bb1a7c2cc1d27015cfc342e2ffab6554d66ffcb11d4f9  guix-build-910a11fa6630/output/arm64-apple-darwin/SHA256SUMS.part
 75e14455f3c05f5426fe67db83c1d63441934f4a3b1dee7b4746a8862debd16d2  guix-build-910a11fa6630/output/arm64-apple-darwin/bitcoin-910a11fa6630-arm64-apple-darwin-unsigned.tar.gz
 8cb9e41c27371de5c6a673c9182ae934a8d4dfe4fcb1b2989029e551a678cb20c  guix-build-910a11fa6630/output/arm64-apple-darwin/bitcoin-910a11fa6630-arm64-apple-darwin-unsigned.zip
 9b794699a47207330c205e6200f2b056cab042f3fab6ba7c0a7dd6a09305bfc48  guix-build-910a11fa6630/output/arm64-apple-darwin/bitcoin-910a11fa6630-arm64-apple-darwin.tar.gz
10c854c3d153df11f44fbd916fa8054cd39ed5f1e41d7ed67852a66d44641677f2  guix-build-910a11fa6630/output/dist-archive/bitcoin-910a11fa6630.tar.gz
11748f88b9cedae4da3838f763c045646726d066f02b32b9d0655e5464bbb5c25c  guix-build-910a11fa6630/output/powerpc64-linux-gnu/SHA256SUMS.part
123e62689fd321e1f923367781c3ee9afb7843010b57b59217691b9145b554b572  guix-build-910a11fa6630/output/powerpc64-linux-gnu/bitcoin-910a11fa6630-powerpc64-linux-gnu-debug.tar.gz
1334635ecd7a77bff15bbd092e1cc17a2dfa2a6c8620ec07407c91ccfdf4d4a6cc  guix-build-910a11fa6630/output/powerpc64-linux-gnu/bitcoin-910a11fa6630-powerpc64-linux-gnu.tar.gz
14bda7f58f6364d41b9615bd9f9f9a703d0b7200611dde2d43b605fae2eeaf0e52  guix-build-910a11fa6630/output/riscv64-linux-gnu/SHA256SUMS.part
151c7601d2596480f2147687d567893b503b9b046071b491adac94687d8bc48ca0  guix-build-910a11fa6630/output/riscv64-linux-gnu/bitcoin-910a11fa6630-riscv64-linux-gnu-debug.tar.gz
167a4dc52ce7bcaca40ca5ec64248caa878e5ce2afda1f7fc262ff969a79cc9174  guix-build-910a11fa6630/output/riscv64-linux-gnu/bitcoin-910a11fa6630-riscv64-linux-gnu.tar.gz
17d545b852fd1237d84a79bf339561e199b404faa6060785820e96f9cef599215e  guix-build-910a11fa6630/output/x86_64-apple-darwin/SHA256SUMS.part
1846204fb509b43da8feb1d271e219ca92ff08c7ca396518ec988548a3412356f1  guix-build-910a11fa6630/output/x86_64-apple-darwin/bitcoin-910a11fa6630-x86_64-apple-darwin-unsigned.tar.gz
19e3d92f97e00e4849ce44070d4f07e162c1703bc5ac32fdccf24829d29c27a8b1  guix-build-910a11fa6630/output/x86_64-apple-darwin/bitcoin-910a11fa6630-x86_64-apple-darwin-unsigned.zip
20b1dd7a8e50167787cb6e2f4ea193572f065da5d54db7885ad5b38139d5014ee1  guix-build-910a11fa6630/output/x86_64-apple-darwin/bitcoin-910a11fa6630-x86_64-apple-darwin.tar.gz
2174ec6cd5b0ba17ef072b3af4149ef210a1129a70112069e6383ac8d93cf3b363  guix-build-910a11fa6630/output/x86_64-linux-gnu/SHA256SUMS.part
222ded19b149b7c5ffc06302d9f3a60905374928fd6f083f90d3a1a4f011073862  guix-build-910a11fa6630/output/x86_64-linux-gnu/bitcoin-910a11fa6630-x86_64-linux-gnu-debug.tar.gz
233fe7b25ef52d2054b5e2ba14dc8c9e14f487ed1f420a8780123bf6f60278927b  guix-build-910a11fa6630/output/x86_64-linux-gnu/bitcoin-910a11fa6630-x86_64-linux-gnu.tar.gz
2489bb522b8fad6b8bcea84f115a6ab33654b4f801e6f94aeb2a4161a6b3a6d17b  guix-build-910a11fa6630/output/x86_64-w64-mingw32/SHA256SUMS.part
2558f5b6c655a5448b1003b0e1e6720f948d56b4854ecbf172826ab4b71002852d  guix-build-910a11fa6630/output/x86_64-w64-mingw32/bitcoin-910a11fa6630-win64-debug.zip
26ba63ef4e8ce6ac71a1851c87aab3ac4d408be2472f1d0505b9c845ccf2ea1b80  guix-build-910a11fa6630/output/x86_64-w64-mingw32/bitcoin-910a11fa6630-win64-setup-unsigned.exe
279bcec129fb2971cbd7c3e6e8dae46379c8a6309ca7faaf9a6415f2aeed06d3bb  guix-build-910a11fa6630/output/x86_64-w64-mingw32/bitcoin-910a11fa6630-win64-unsigned.tar.gz
28140ed309a4d45de507d0e5831ef04710114f5822e9277b43c8378c7dd63e9244  guix-build-910a11fa6630/output/x86_64-w64-mingw32/bitcoin-910a11fa6630-win64.zip
  10. hebasto approved
  11. hebasto commented at 9:38 am on January 21, 2025: member

    ACK 910a11fa66305f90b0f3a8aa9d2055b58a2d8d80, I’ve performed a subtree update locally and got the same changes.

    I also verified that the LEVELDB_IS_BIG_ENDIAN macro is no longer used in the code.

  12. l0rinc commented at 12:15 pm on January 21, 2025: contributor

    Reapplying the original leveldb PRs and comparing it against this PR leaves:

     0diff --git a/cmake/leveldb.cmake b/cmake/leveldb.cmake
 1--- a/cmake/leveldb.cmake	(revision 846a4dfc107d6593d0133bb8ae63edd824f7cb74)
 2+++ b/cmake/leveldb.cmake	(date 1737461205681)
 3@@ -60,6 +60,7 @@
 4     HAVE_FULLFSYNC=$<BOOL:${HAVE_FULLFSYNC}>
 5     HAVE_O_CLOEXEC=$<BOOL:${HAVE_O_CLOEXEC}>
 6     FALLTHROUGH_INTENDED=[[fallthrough]]
 7+    LEVELDB_IS_BIG_ENDIAN=$<STREQUAL:${CMAKE_CXX_BYTE_ORDER},BIG_ENDIAN>
 8     $<$<NOT:$<BOOL:${WIN32}>>:LEVELDB_PLATFORM_POSIX>
 9     $<$<BOOL:${WIN32}>:LEVELDB_PLATFORM_WINDOWS>
10     $<$<BOOL:${WIN32}>:_UNICODE;UNICODE>

    Which was indeed unused after the LevelDB changes.

    ACK 910a11fa66305f90b0f3a8aa9d2055b58a2d8d80

    Edit: running a full reindex-chainstate to make sure it passes, let’s not merge it yet

  13. theuni approved
  14. theuni commented at 4:41 pm on January 21, 2025: member
    utACK 910a11fa66305f90b0f3a8aa9d2055b58a2d8d80
  15. fanquake merged this on Jan 21, 2025
  16. fanquake closed this on Jan 21, 2025
  17. fanquake deleted the branch on Jan 21, 2025
  18. l0rinc commented at 2:57 pm on January 22, 2025: contributor

    Since LevelDB changes are consensus-critical, I ran -reindex-chainstate multiple times up to 880000 to check for any changes in performance or correctness.

    I’m happy to report that no issues were found.

     0hyperfine \
 1--runs 2 \
 2--parameter-list COMMIT 712cab3a8f8ad76db959337ddc35cb4c34cac388,910a11fa66305f90b0f3a8aa9d2055b58a2d8d80 \
 3--prepare 'rm -f /mnt/my_storage/BitcoinData/debug.log && git checkout {COMMIT} && git clean -fxd && git reset --hard && cmake -B build -DCMAKE_BUILD_TYPE=Release -DBUILD_UTIL=OFF -DBUILD_TX=OFF -DBUILD_TESTS=OFF -DENABLE_WALLET=OFF -DINSTALL_MAN=OFF && cmake --build build -j$(nproc)' \
 4--cleanup 'mv /mnt/my_storage/BitcoinData/debug.log /mnt/my_storage/logs/debug-{COMMIT}.log' \
 5'COMMIT={COMMIT} ./build/src/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=880000 -dbcache=30000 -reindex-chainstate -connect=0'
 6
 7Benchmark 1: COMMIT=712cab3a8f8ad76db959337ddc35cb4c34cac388 ./build/src/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=880000 -dbcache=30000 -reindex-chainstate -connect=0
 8  Time (mean ± σ):     23745.543 s ± 172.006 s    [User: 37663.092 s, System: 696.830 s]
 9  Range (min … max):   23623.917 s … 23867.170 s    2 runs
10 
11Benchmark 2: COMMIT=910a11fa66305f90b0f3a8aa9d2055b58a2d8d80 ./build/src/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=880000 -dbcache=30000 -reindex-chainstate -connect=0
12  Time (mean ± σ):     23847.875 s ± 249.798 s    [User: 37649.007 s, System: 692.565 s]
13  Range (min … max):   23671.241 s … 24024.509 s    2 runs
14 
15Summary
16  COMMIT=712cab3a8f8ad76db959337ddc35cb4c34cac388 ./build/src/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=880000 -dbcache=30000 -reindex-chainstate -connect=0 ran
17    1.00 ± 0.01 times faster than COMMIT=910a11fa66305f90b0f3a8aa9d2055b58a2d8d80 ./build/src/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=880000 -dbcache=30000 -reindex-chainstate -connect=0


fanquake DrahtBot l0rinc kevkevinpal hebasto theuni

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2025-02-23 00:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me