tendermint / tm-db

Common database interface for various database backends for Tendermint Core and Cosmos SDK
Apache License 2.0
89 stars 136 forks source link

add state sync test #255

Closed faddat closed 2 years ago

faddat commented 2 years ago

Gaia Speed Run and compatiblity test

Instead of assuming that a given database works, why not state sync gaia, and run her for an hour or two?

Reasoning:

@caatshark's pebbledb code passes every test in this repository, and every test in iavl, and performs great on benchmarks.

When running live nodes, it explodes with funny type errors, every single time.

This test will require a runner that is larger/more capable than what github provides for free.

Our options are:

If we use one of the self-hosted options, I could host that machine, we're building one for that purpose right now, in fact.

I'd prefer to use gitlab, that can be set up to mirror this repository and afaik github actions runners still cannot run more than one job at once, whereas we'd need minimum six.

further thoughts

It seems to me that this worked well! I didn't know if it would. Since it did, I'd like to suggest that we start testing changes in the SDK and tendermint this way, too.

I cannot count the number of times I've found gotchas -- in production -- instead of in tests.

I'm hoping that by catching the gotchas in CI, we can all see the gotchas, and then figure out what tests to write so we catch the gotchas with tests next time.

faddat commented 2 years ago

@creachadair on second thought I have to disagree somewhat.

The issue with state sync lives in this repo

creachadair commented 2 years ago

@creachadair on second thought I have to disagree somewhat.

The issue with state sync lives in this repo

I'm not sure what you mean, can you say more? Generally if a package X depends on a package Y, then the integration tests for X & Y should live with X, not with Y. Otherwise we wind up creating a circular dependency. And it's the maintainers of X who need to make upgrade decisions.

In this case, the circular dependency isn't of the "won't compile" variety, but I don't think it makes sense for an infrastructure library to depend on the repositories that use it.

faddat commented 2 years ago

ah, not really -- I think I'll close this, but I'll also show you what it feeds into when that's complete.

Thanks very much for your guidance in working on tm-db, you've been really helpful @creachadair

(possibly hard to explain, easier to show)

:)