LevelDB Fixes and Enhancements #6904

pull ghost wants to merge 8 commits into bitcoin:master from jgarzik:2015_sqlite changing 7 files +267 −129
  1. ghost commented at 3:07 PM on October 29, 2015: none

    No description provided.

  2. dbwrapper: switch from leveldb to sqlite for bitcoind backend database 791e70fba7
  3. dbwrapper: sqlite bug fixes 65f95b4234
  4. dbwrapper: adjust db page size, cache size parameters 03765433af
  5. dbwrapper: sqlite db init bug fixes, better errors 4d2e72900d
  6. dbwrapper: improve sqlite comments, error diagnostics cbd5412a67
  7. dbwrapper: enable write-ahead journalling (credit: wumpus) 0fdb5cebed
  8. dbwrapper: enable sqlite WAL auto-checkpointing 0172e8d37b
  9. dbwrapper: sqlite WAL auto checkpoint at 10,000 pages 159934199f
  10. mcelrath commented at 3:16 PM on October 29, 2015: none

    @jgarzik please advise since you didn't originate this request.

  11. jonasschnelli commented at 3:44 PM on October 29, 2015: contributor

    Would be nice to have a bench utility to compare this PR agains the leveldb version.

  12. jgarzik commented at 4:07 PM on October 29, 2015: contributor

    I intentionally avoided creating a pull request because we don't yet know if sqlite has adequate performance for our needs.

    Based on preliminary @jmcorgan input, it sounds like sqlite is too slow.

    Recommendation: Close pull request, but request @jmcorgan paste his test results here for issue tracking purposes.

  13. unknown closed this on Oct 29, 2015
  14. unknown reopened this on Oct 29, 2015
  15. unknown closed this on Oct 29, 2015
  16. jmcorgan commented at 4:23 PM on October 29, 2015: contributor

    My tests were entirely informal and subjective. We'd actually have to define a benchmark that could be part of an A/B test in order for the results to be interpreted.

  17. jgarzik commented at 5:07 PM on October 29, 2015: contributor

    @jmcorgan Informal is still informative :) "It took Y hours to reindex with sqlite"

  18. dcousens commented at 8:02 PM on October 29, 2015: contributor

    @LordOfTheCoin was this PR in working order such that the above could easily be tested?

  19. ghost commented at 10:07 PM on October 29, 2015: none

    Why not use http://www.aerospike.com ? Its insanely fast and opens up options for the future, ie. multi-threads in multiple processes.

  20. ghost commented at 5:26 AM on October 30, 2015: none

    Tried on Win 10, Compiled and Ran, Didn't Thoroughly Test Though

  21. dcousens commented at 5:45 AM on October 30, 2015: contributor

    @jgarzik I'll see if I can provide that information soon. Is there any specific metrics you'd prefer?

  22. jgarzik commented at 6:31 AM on October 30, 2015: contributor

    @dcousens The first metric is initial sync time, which is sometimes "approximated" with a reindex. A/B testing for current master leveldb versus sqlite.

  23. sipa commented at 6:44 AM on October 30, 2015: member

    Also, running with -debug=bench will give a breakdown of time per activity.

  24. ghost commented at 9:10 AM on October 30, 2015: none

    BTW, AeroSpike doesn't have a c++ implementation

  25. ghost commented at 10:37 AM on October 30, 2015: none

    I think the client library doesnt exist in C++, but should easily support it.

    Question is what our aim here is? Speed? Future in-ram processing ideas?

  26. jgarzik commented at 1:00 PM on October 30, 2015: contributor

    @LordOfTheCoin C++ works just fine with C libraries. @cryptomarc speed

  27. ghost commented at 2:07 PM on October 30, 2015: none

    To my understanding Aerospike pretty much kills in all the benchmark tests done vs other No-SQL db's. I will try to run a couple of tests myself and post them here.

  28. sipa commented at 4:09 PM on October 30, 2015: member

    Aerospike is AGPL...

  29. ghost commented at 12:45 PM on October 31, 2015: none

    Its' AGPL, compatable with GPL, but with MIT?

  30. sipa commented at 3:14 PM on October 31, 2015: member

    Using an AGPL library in Bitcoin Core would require Bitcoin Core itself to be licensed under AGPL or compatible terms.

    Even if we would want to do that, it would be nearly impossible to accomplish. Every contributor in the past would need to give consent about this, as they hold copyright over the code they wrote.

  31. ghost commented at 4:10 PM on October 31, 2015: none

    There are about ~ 6000 Contributors, so I get it....

  32. ghost commented at 5:23 PM on October 31, 2015: none

    Its a shame, and something I missed out when looking at Aerospike. Shame because it seems to be much on par with some interesting functionality that we could utilize in the future for Bitcoin.

  33. mcelrath commented at 5:26 PM on October 31, 2015: none

    I really don't think this database should be or needs to be distributed with the core source (as leveldb currently is). There are no licensing issues when linking against an external library. This should be an external dependency, whose correctness can be verified using UTXO commitment hashes as I describe here: http://lists.linuxfoundation.org/pipermail/bitcoin-dev/2015-October/011638.html

  34. sipa commented at 5:32 PM on October 31, 2015: member

    Linking against a library is usually considered a derivative work, in particular when the library's .h file are under the same license.

    Including the source code of our database engine inside the source tree or not is an artifact of using git subtrees. Even if we didn't, we would still need to specify an exact version to prevent bugfixes that break consensus by users who link against different versions. Using subtree or not is not relevant in this discussion.

    UTXO commitments do not fix this, as they need a different trust model (miners not committing to incorrect data).

  35. mcelrath commented at 5:45 PM on October 31, 2015: none

    That's just not true regarding derivative works. Bitcoin links against e.g. boost and is not a derivative of boost. Its header files are distributed separately. It's easy to specify specific versions of external dependencies.

    Verifying correctness of database operation is not a trust model change. I'm thinking of it mostly in an advisory capacity...to notify node operators that something has gone horribly wrong. Note that the UTXO commitment scheme I describe doesn't actually require those hashes to be committed to in a block. Nodes can locally compute them simply to verify their own correctness of operation (and not communicate them at all). So one has potentially 3 sources of these hashes: computed during block ingest, computed by database query, and committed in the block. Nodes that detect such an error could e.g. refuse to mine blocks (at the discretion of the operator -- obviously blindly trusting the hash in the block IS a trust model change).

  36. sipa commented at 6:37 PM on October 31, 2015: member

    @mcelrath In my understanding, Bitcoin binaries are a derivative work of the Boost headers, and if linked statically, a derivative work of the Boost C++ code too. If we were to make Bitcoin Core depend on an AGPL library, we could not distribute binaries without also providing the Bitcoin Core source code under AGPL-compatible licensing. If the AGPL library would be an optional dependency, things are potentially different - we could ship Bitcoin Core binaries that do not need the library for correct operation, but load them at runtime if available in that case.

  37. DrahtBot locked this on Sep 8, 2021
Contributors
ghostmcelrathjonasschnellijgarzikjmcorgandcousenssipa
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: 2026-04-17 15:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me