fix dbtest backend #8745
fix dbtest backend #8745
Conversation
|
Important Auto Review SkippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
ziggie1984
force-pushed
the
fix-dbtest-backend
branch
from
cacf03f
to
eb6f6cb
Compare
|
Let's see whether all the unit tests pass using the global kvdb backend tag |
channeldb/db.go
Outdated
| // golang tags (e.g. kvdb_sqlite, kvdb_etcd, kvdb_postgres or default bolt if | ||
| // nothing is explicitly specified). | ||
| // Any necessary schemas migrations due to updates will take place as necessary. | ||
| func Open(dbPath string, modifiers ...OptionModifier) (*DB, error) { |
There was a problem hiding this comment.
I think it'd be better to name this OpenTestDB and make the renames in the tests too as otherwise this method could be mistaken for a generic open.
channeldb/db.go
Outdated
| // to updates will take place as necessary. | ||
| // TODO(bhandras): deprecate this function. | ||
| func Open(dbPath string, modifiers ...OptionModifier) (*DB, error) { | ||
| // OpenBoltDB opens or creates channeldb using the boltDB type and enables |
There was a problem hiding this comment.
Actually I think we can even remove this one, I kept it because in lnwallet/test_utils.go we use the dbModifiers but not for the opening option but rather when we create the final backend ( CreateWithBackend(backend, modifiers...)) tho I find it useful in case we want to use the db opening options in the future to have the function ready for it ? But also open to remove it.
ziggie1984
force-pushed
the
fix-dbtest-backend
branch
2 times, most recently
from
f99f3f0
to
3ce2edd
Compare
ziggie1984
force-pushed
the
fix-dbtest-backend
branch
4 times, most recently
from
ac8f8cc
to
9001910
Compare
ziggie1984
force-pushed
the
fix-dbtest-backend
branch
from
9001910
to
d07d0d2
Compare
| // to updates will take place as necessary. | ||
| // TODO(bhandras): deprecate this function. | ||
| func Open(dbPath string, modifiers ...OptionModifier) (*DB, error) { | ||
| // OpenTestBoltDB opens or creates channeldb using the boltDB type and enables |
There was a problem hiding this comment.
So this function is currently not used, because in all of our testcases we used the default opening options for boltdb:
BoltBackendConfig: kvdb.BoltBackendConfig{
NoFreelistSync: true,
AutoCompact: false,
AutoCompactMinAge: kvdb.DefaultBoltAutoCompactMinAge,
DBTimeout: kvdb.DefaultDBTimeout,
There was a problem hiding this comment.
So I think its best to remove it? @bhandras
ziggie1984
force-pushed
the
fix-dbtest-backend
branch
from
d07d0d2
to
362a7e2
Compare
Previously we would open the kvdb backend only for the dbbolt option which meant that most of the tests where exectued using the bolt db backend although started via a different db backend e.g. sqlite. Now two functions are provided, one opens and creates the db for the bolt specific backend and the other one uses the golang specified db backend. This commit also fixes a bug in TestOpenWithCreate where we would open a different db with a different name and therefore the tests would fail to test its objective. Moreover precautiously parts in the codebase are improved where we would use a different name for the database than the global name "channel.db" which is error prune and therefore improved.
ziggie1984
force-pushed
the
fix-dbtest-backend
branch
from
362a7e2
to
9a59ed6
Compare
ziggie1984
force-pushed
the
fix-dbtest-backend
branch
from
9a59ed6
to
5693599
Compare
|
Short summary of the current PR: The current set of unit tests are not easily portable to all available backends. We are currently running a lot of tests on the boltdb backend, switching over to the global kvdb_architecture flag (e.g. kvdb_etcd ...) needs still work to be done. For example the copy of the db file needs to be different for all backends, we cannot just copy a file like in boltdb in the etcd case. Moreover postgres seems to have problems handling the workload while running embedded (but not sure, this needs to be investigated too). @bhandras proposed taking over this issue because he is already very knowledge in this code area. |
|
@ziggie1984, remember to re-request review from reviewers when ready |
Replaces #8462
This PR fixes a testcase where we would basically test not what the testcase was suppose to test (reopening the same database).
Moreover it enhances a lot of unit-tests which would initially only test the bolt db backend and makes those tests compatible with the global kvdb backend, so those tests are now executed depending on the selected golang kv tag.