Comments (11)
To add to this now IAVL has a dependency on Tendermint - for tendermint/libs/common and tendermint/crypto/tmhash. This is quite painful for upgrades since I dep now wants me to have compatible tendermint and IAVL versions.
It looks like this dependency is mostly for errors.Wrap and some straight-forward hash conventions that I feel could easily be copied. Is it really worth the dependency?
from iavl.
Personally I would like to see IAVL remain a separate library, but it is the worst of all worlds to have it in a separate repo but have dependencies on something has a heavy as Tendermint, so I suppose if that is the way it goes it would make sense to roll it into the cosmos-sdk.
It was a nightmare having half a dozen dependency libraries I agree, and particularly since in reality only Tendermint depended on them - they weren't mature and moved too quickly for that to make sense. So stability is one factor, but another one is if you genuinely have multiple independent things depending on a library. For the db libs you currently have at least: Tendermint, Burrow, IAVL, and the cosmos-sdk.
As @jaekwon says above:
The pain is partially due to us packaging many dependencies together. As packages (like db) mature I think we should move the out, and db is ripe.
I'm inclined to agree. I don't think the DB is necessarily 'very stable' - everything that depends on it is in principle alpha software! However I do think it is relatively more stable and it has real dependencies (and could attract more as a key-value db abstraction layer if maintained separately from Tendermint which I think would be for the good).
from iavl.
I'm in strong agreement.
Would you be open to make a PR to remove the dependencies?
Only thing is that it should be made very clear that the expect dbs come from tmlibs/db, but it shouldn't be a problem to replicate the parts of the interface we need and the MemDB itself within this repo.
from iavl.
<open debate>
The DB interfaces are stable and useful. We should also consider using more of it and decoupling it from tmlibs. The pain is partially due to us packaging many dependencies together. As packages (like db) mature I think we should move the out, and db is ripe. (it benefited by being developed in tandem with the SDK and Tendermint, so I'm not knocking tmlibs in any way).
I'd extend Dave's blog post with useful entangled interface sets like our tmlibs/db or https://golang.org/pkg/crypto/ . They are most useful/ergonomic when entangled (for as long as they are well designed and consistent), and they don't suffer from the smells that he's talking about. He mentioned recursive interfaces but I think entangled interfaces are the more general exceptional case.
And this is also due to the design constraints of Golang where isometric interfaces aren't identical. If they could be used interchangeably then this wouldn't be an issue. https://play.golang.org/p/mUBkkbQPz8_s
I'm more concerned about us making this proposed change without fully considering the reasons/heuristics, than the proposed change itself. It's easy enough to pull the DB.Iterator method out and define a new func Iterator2 ...
in iavl where we can just pass in the db.Iterator
method as a function. I just don't think it's necessary in this case, (and arguably it's more awkward to separate an iterator like this from the context of a db connection) and we ought to be spinning db out of tmlibs anyways, which would solve the problem.
Well designed interfaces like the ones in DB are hard earned... Once they've matured IMO we benefit by using them more, not less. I'm sure we'll find better examples too in the future.
from iavl.
I'm also on a mission to replace all instances of pkg/errors in all of our codebases, starting with IAVL, to this system: tendermint/tmlibs#220 , so eventually that will have to spin out of tmlibs as well. That can wait though, we can use pkg/errors for now.
from iavl.
The DB interfaces are stable and useful. We should also consider using more of it and decoupling it from tmlibs.
I agree with this. Extracting from tmlilb substantially solves the issue.
In terms of enhanced (de)composition of the db interface. As you say it's an annoyance of Go's type system that we can't do this as it. It seems like we could pull out NewBatch
NewReverseIterator
and NewIterator
out of the main interface and provide them as standalone functions/interfaces. There is no coupling between dependent interfaces. We get the same hard-won interface and there is nothing to stop you from forming a superset interface.
I have regularly thought it would have been useful to implement an interception layer for IAVL just for the subset of DB
it actually uses but implementing the parts it doesn't use felt onerous. Examples I recall are:
- Implementing a content-addressed layer against existing implementation
- Adding a sketch-based cache layer
- Providing a 'pseudo batch' to IAVL so I can coordinate other state updates with IAVL's commit
I think asking for a minimal interface is a good heuristic in go anyway, but to give some context.
I can see how defining the minimal interface at the consumer (e.g. IAVL) could lead to fragmentation though - and there is an advantage to centralising the component interfaces (in a db lib). There's less going on but here is the kind of approach I have taken: https://github.com/hyperledger/burrow/blob/develop/account/state/state.go
We could have the same interface as currently exists as a composition, but have get-set, get-set-sync, batch, iterator, etc as separate component interfaces. IAVL could compose the ones it needs. Could try and make this more concrete with a PR in IAVL as a prototype if it would help.
from iavl.
Having had a play with this - I agree with @jaekwon that there isn't really a nice way to extract the Iterator
methods so that we could benefit from isomorphic interfaces. Is anyone from Tendermint in a position to extract db lib (let's not call it go-db :) )? We also do make use of the goleveldb backend for benchmarking which is not something we would want to duplicate (a version of memdb - maybe...) so I think a separate lib is the right answer here. I think the db lib will be a useful integration point for building other things (similar to the way IAVL is) so I'd love to see it spun out as 'sqlx' style layer for key-value stores in Go.
I'm up for making a PR that removes the other more trivial Tendermint dependencies and we could see how that looks in the meantime?
from iavl.
Spinning of this db stuff is a fairly large design choice, we have chosen to adopt the monorepo style for better utilization of developer time. This would mean breaking off from it. We were actually discussing moving IAVL to the Cosmos-SDK as well.
I'm not opposed to this iff the DB stuff is very stable.
from iavl.
Probably would be good to see db
move into its own repo eventually, but I don't think it's ready yet. Some upcoming changes:
- possible issues with ReverseIterator (cosmos/cosmos-sdk#2243)
- experimenting with backends to improve performance (tendermint/tendermint#2337, tendermint/tendermint#2370).
Meanwhile, it looks like IAVL is taking on some more dependence on Tendermint for the general merkle proof stuff - not quite sure how to resolve that, and where the general merkle proof stuff should live, but if it's going to be in Tendermint and the IAVL needs it, then maybe IAVL should move to Tendermint too.
Ideally, nothing needs to move, and the IAVL just needs to implement some interfaces and maybe duplicate a bit of code, but otherwise doesn't have dependencies on Tendermint at all. So hopefully we can work towards that.
from iavl.
Update: the only thing that is being used from tendermint/tendermint
now is crypto, hopefully in the near future we can remove this as well.
from iavl.
for the time being, we will keep tendermint as a dep because of tendermint/crypto
, if you want it to be entirely removed, please reopen this issue
from iavl.
Related Issues (20)
- iaviewer doesn't work HOT 2
- Inner node proofs should require either the left or right node be empty HOT 3
- Proof: Remove RangeProofs in favour of only ics23 HOT 2
- Improve docs to clarify definition of an inner node's key
- Modify the key format of node in DB to a sequential integer ID instead of the hash value. HOT 4
- The immutableTree clone() func is missing skipFastStorageUpgrade field --> wrong Block.Header.AppHash. error in the blockchain HOT 3
- move internal api to internal
- fastnode causing excessive disk thrashing HOT 5
- Problem: possible app-mismatch from `enableFastStorageAndCommitIfNotEnabled` HOT 1
- fastnode: DeserializeNode doesn't check that decoded varint is in bounds length of buffer HOT 1
- iaviewer cannot show versions
- `skipFastStorageUpgrade` a little confusing to followup HOT 2
- IsFastCacheEnabled is activated performantly after skipFastStorageUpgrade is false HOT 1
- iavl: *MutableTree doesn't provide a way to release the database; noticed inside iavl/cmd/iaviewer.ReadTree HOT 1
- Refactor One Large Commit Into Smaller Batches HOT 7
- Optimise iavl storage for archive mode HOT 6
- Osmosis did it perfectly, the bool kills it
- prune versions without bookkeepings HOT 1
- iavl start up time is slow
- LoadVersionForOverwriting doesn't commit HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from iavl.