Skip to content
This repository was archived by the owner on Feb 15, 2024. It is now read-only.

Use lmdb database in picocoin #92

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Use lmdb database in picocoin #92

wants to merge 10 commits into from

Conversation

aido
Copy link
Contributor

@aido aido commented Oct 8, 2016

Hi,

This pull request is the first step in introducing a simple and lightweight key-value database to picocoin:
https://symas.com/products/lightning-memory-mapped-database/

LMDB is written in C, extremely high performance and memory-efficient. It is used in other crypto currency projects such as Monero so seems like a good fit for picocoin.
Along with some other handy features LMDB can have multiple sub-databases. I haven't thought things out fully yet but blkdb could possibly be stored as a sub-database with the blockchain as another sub-database or collection of sub-databases, performance permitting.

This would add another building block to make it easier to use libccoin to build a full node if someone so wishes.

@coveralls
Copy link

Coverage Status

Coverage decreased (-31.6%) to 47.814% when pulling a310547 on aido:liblmdb into 81bb7ab on jgarzik:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-31.6%) to 47.814% when pulling a310547 on aido:liblmdb into 81bb7ab on jgarzik:master.

@coveralls
Copy link

coveralls commented Oct 8, 2016

Coverage Status

Coverage decreased (-39.7%) to 39.763% when pulling ca60a7d on aido:liblmdb into 81bb7ab on jgarzik:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.9%) to 77.565% when pulling f0f32f7 on aido:liblmdb into 81bb7ab on jgarzik:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-1.9%) to 77.565% when pulling f0f32f7 on aido:liblmdb into 81bb7ab on jgarzik:master.

@aido
Copy link
Contributor Author

aido commented Dec 23, 2016

Hi,

Given the preference expressed in PR #96 to using normal autoconf toolage for linking to external libraries rather than importing a tree / adding a git subtree into the codebase I assume this PR in its current form is not a good idea.

@jgarzik
Copy link
Owner

jgarzik commented Dec 23, 2016

@aido It's a balance. As a first step we should link externally. If the library proves consensus critical, or versioning through OS proves problematic, we can look at moving in-tree via git subtree or code import. libevent is stable across most modern OS's. lmdb might(?) be different.

First step would be to link against external lmdb and change the picocoin code to use lmdb.

@aido
Copy link
Contributor Author

aido commented Dec 23, 2016

OK Jeff,

I'll leave this PR open and play around with it a bit to see if LMDB is a good choice.

LMDB allows for the creation of multiple sub-databases so I am thinking of setting up the following sub-databases within a main mainnet/testnet database.

Database Key Value
blockdb block hash block header + list(tx_hashes)
blockheightdb block height block hash
txdb tx hash tx

Experimentation going well here: https://github.com/aido/picocoin/tree/liblmdb_test

@coveralls
Copy link

Coverage Status

Coverage remained the same at 77.643% when pulling 2926183 on aido:liblmdb into 3c3d504 on jgarzik:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 77.643% when pulling 2926183 on aido:liblmdb into 3c3d504 on jgarzik:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 77.643% when pulling 2926183 on aido:liblmdb into 3c3d504 on jgarzik:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 77.643% when pulling e10840d on aido:liblmdb into 3c3d504 on jgarzik:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 77.643% when pulling e10840d on aido:liblmdb into 3c3d504 on jgarzik:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 77.643% when pulling e10840d on aido:liblmdb into 3c3d504 on jgarzik:master.

@jgarzik
Copy link
Owner

jgarzik commented Jan 8, 2017

Looking good so far!

@aido
Copy link
Contributor Author

aido commented Jan 8, 2017

Yes, it;s coming along nicely. LMDB seems to be quite suitable as a DB choice.

I see GitHub has a handy code review feature for each commit to a pull request so feel free to add constructive criticism if it is needed.

@aido
Copy link
Contributor Author

aido commented Jan 9, 2017

Hi @jgarzik,

The latest commit adds a blockheightdb to speed up rebuilding of blkdb on startup.

Some of the OSX builds are failing on Travis. Travis builds fail if no output is received in 10 minutes. The blockfile test requires ~228,000 block headers to be loaded into the blockheightdb (193,000 mainnet + ~35,000 testnet). This is taking slightly more than 10 minutes on OSX due mostly to the fact that the Travis OSX VMs are painfully slow compared to the Linux VMs. There is a Travis workaround for this but it is not ideal and will probably cause the build to fail for other reasons anyway.
The mainnet test headers are in the test/hdr193000.ser file. A smaller file containing maybe 50,000 headers should do the trick.

Also, a couple of things I've noticed while testing, not really related to this PR.
brd seems to initially download 500 blocks, the getblocks limit. It then stops and just listens to the network, If restarted it will then download the next 500 blocks, reading the initial 500 from the blockdb. Is this behaviour by design? If so, without a restart brd will only ever relay blocks 0 to 500.
We now have a blockdb and a blkdb, slightly confusing/clashing names. I propose renaming blkdb to chaindb which is probably a more accurate description of what it is. Your thoughts?

@aido aido force-pushed the liblmdb branch 2 times, most recently from 5510a7a to f07237d Compare January 10, 2017 23:33
@coveralls
Copy link

Coverage Status

Coverage remained the same at 77.746% when pulling b773043 on aido:liblmdb into 3c3d504 on jgarzik:master.

@coveralls
Copy link

coveralls commented Jan 11, 2017

Coverage Status

Coverage remained the same at 77.161% when pulling f07237d on aido:liblmdb into 3c3d504 on jgarzik:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 77.161% when pulling f07237d on aido:liblmdb into 3c3d504 on jgarzik:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 77.161% when pulling f07237d on aido:liblmdb into 3c3d504 on jgarzik:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 77.161% when pulling f07237d on aido:liblmdb into 3c3d504 on jgarzik:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 77.161% when pulling f07237d on aido:liblmdb into 3c3d504 on jgarzik:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 77.746% when pulling fbdddab on aido:liblmdb into 3c3d504 on jgarzik:master.

@aido
Copy link
Contributor Author

aido commented Jan 13, 2017

Hi,

The purpose of this pull request is to introduce LMDB databases to picocoin. So in that regard this PR is now complete (although it may still need some tidying up).

Travis OSX VMs are really slow at best and sometimes really, really slow. When they are really, really slow some Travis builds timeout, fail and need to be restarted. That is why some of the Travis builds of the most recent commit have failed. These will need to be restarted when Travis OSX is not running really, really slow.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 77.683% when pulling fbbb976 on aido:liblmdb into 3c3d504 on jgarzik:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 77.683% when pulling fbbb976 on aido:liblmdb into 3c3d504 on jgarzik:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 77.683% when pulling fbbb976 on aido:liblmdb into 3c3d504 on jgarzik:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.7%) to 79.275% when pulling fb54394 on aido:liblmdb into faa9b83 on jgarzik:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.7%) to 79.275% when pulling fb54394 on aido:liblmdb into faa9b83 on jgarzik:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.7%) to 79.275% when pulling fb54394 on aido:liblmdb into faa9b83 on jgarzik:master.

};

enum {
MAX_DB_SIZE = 67108864 // Maximum database size in bytes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MAX_DB_SIZE should be set to a more realistic valiue e.g. 17179869184

aido added 7 commits October 11, 2017 23:34
This will help speed up regenerating blkdb on startup
Speeds up testing a tad. Mainnet headers reduced from 193000 headers to 50000. Testnet headers reduced from 35141 headers to 25000
…nstead of from file

Also rename blkdb database to chaindb to avoid confusion with LMDB blockdb
@coveralls
Copy link

Coverage Status

Coverage increased (+0.7%) to 79.275% when pulling 4289bc0 on aido:liblmdb into faa9b83 on jgarzik:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.7%) to 79.275% when pulling 4289bc0 on aido:liblmdb into faa9b83 on jgarzik:master.

@coveralls
Copy link

coveralls commented Oct 12, 2017

Coverage Status

Coverage decreased (-0.005%) to 79.729% when pulling 544d903 on aido:liblmdb into 8ae9bad on jgarzik:master.

@jgarzik
Copy link
Owner

jgarzik commented Oct 19, 2018

Taking a fresh look at all the pull requests.

Concept ACK -- lmdb is proving to be superior in other projects, so it makes sense to pull it into picocoin as well.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants