feat/Add memiavl based storage #2973

chillyvee · 2024-03-01T13:41:30Z

Description

Add memiavl storage support

Faster state-sync recovery
Reduced disk i/o and space usage

No operational change when memiavl is not enabled.

Note github.com/crypto-org-chain/cronos requires go 1.22

Statesync, Snapshot, Validation works well in Theta Testnet as provided in this branch against v15.0.0-rc3
https://github.com/chillyvee/gaia/tree/cv15.0.0-rc3-memiavl

Quickly test on theta testnet with app.toml as follows:

[memiavl]
  async-commit-buffer = 0
  cache-size = 1000
  enable = true
  snapshot-interval = 61
  snapshot-keep-recent = 3
  zero-copy = false

[state-sync]
  snapshot-interval = 61
  snapshot-keep-recent = 3

The statesync chunk apply time for Theta Testnet is approximately:

70 minutes for goleveldb (as is - on test server)
2 minutes for memiavl (same server)

This work follows a general effort to provide options to increase the speed of statesync and improved i/o utilization.

Related discussion here:
cosmos/iavl#878

Author Checklist

I have...

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

confirmed the correct type prefix in the PR title
confirmed ! in the type prefix if API or client breaking change
confirmed all author checklist items have been addressed
reviewed state machine logic
reviewed API design and naming
reviewed documentation is accurate
reviewed tests and test coverage

yihuang · 2024-03-01T14:03:23Z

go.mod

@@ -221,8 +233,13 @@ replace (
 	// Use special SDK v0.47.x release with support for both ICS and LSM
 	github.com/cosmos/cosmos-sdk => github.com/cosmos/cosmos-sdk v0.47.10-ics-lsm

+	// use memiavl for faster storage
+	github.com/cosmos/cosmos-sdk/store => github.com/crypto-org-chain/cronos/store v0.0.5-0.20240301012710-e9bb006c8d14
+	github.com/cosmos/cosmos-sdk/versiondb => github.com/crypto-org-chain/cronos/versiondb v0.0.5-0.20240301012710-e9bb006c8d14


this one is not needed for memiavl alone.

Thank you @yihuang for the review and comment

Would it be useful for RPC/API providers that need to be able to provide query results with data from different heights?

Or is this only useful for creating statesync snapshots on a verison that memiavl does not snapshot?

Would it be useful for RPC/API providers that need to be able to provide query results with data from different heights?

It do support historical state/proof queries, and the supported range is determined by the memiavl.snapshot-keep-recent config, it support historical query from the oldest available snapshot to latest height.

But I have to say, historical query is not that efficient with memiavl, I'm not sure how it performs under load, we use versiondb aka. state storage to provide state queries, only use memiavl to provide proof queries.

If historical state/proof query support is removed (no go mod replace), will it case any problems for validators or users of API/RPC?

Lets keep versiondb to help the RPC node providers. The store mod replace will be removed.

Or is this only useful for creating statesync snapshots on a verison that memiavl does not snapshot?

You can't create state sync snapshot from versiondb, because it don't have information for the branch nodes, only leaf key value pairs.
and memiavl do support creating state sync snapshots for all the intermediate versions between the oldest snapshot and the latest height.

Thank you for the clarification. For others reviewing, we discussed how to enable snapshots for intermediate versions here: cosmos/iavl#878 (comment)

Thank you for the clarification. For others reviewing, we discussed how to enable snapshots for intermediate versions here: cosmos/iavl#878 (comment)

maybe you can add some comments for the individual parameters here for now.

Comments on parameters added

yihuang · 2024-03-01T14:12:16Z

go.mod

@@ -221,8 +233,13 @@ replace (
 	// Use special SDK v0.47.x release with support for both ICS and LSM
 	github.com/cosmos/cosmos-sdk => github.com/cosmos/cosmos-sdk v0.47.10-ics-lsm

+	// use memiavl for faster storage
+	github.com/cosmos/cosmos-sdk/store => github.com/crypto-org-chain/cronos/store v0.0.5-0.20240301012710-e9bb006c8d14


this replacement is not needed, the github.com/crypto-org-chain/cronos/store is not a replacement for the cosmos store package.

Is it okay to NOT replace cosmos-sdk/store and only use cronos/store for the helper function?

https://github.com/cosmos/gaia/pull/2973/files#diff-0f1d2976054440336a576d47a44a37b80cdf6701dd9113012bce0e3c425819b7R61

memiavlstore "github.com/crypto-org-chain/cronos/store" baseAppOptions = memiavlstore.SetupMemIAVL(logger, homePath, appOpts, false, false, baseAppOptions)

it's not supposed to replace cosmos-sdk/store at all, it's a different package ;D
it implements the same interfaces, and hook into cosmos-sdk to replace the default root multistore.

yihuang · 2024-03-01T15:10:31Z

go.mod

@@ -16,6 +18,7 @@ require (
 	github.com/cosmos/ibc-apps/middleware/packet-forward-middleware/v7 v7.1.2
 	github.com/cosmos/ibc-go/v7 v7.3.2
 	github.com/cosmos/interchain-security/v3 v3.3.0
+	github.com/crypto-org-chain/cronos/store v0.0.0-00010101000000-000000000000


Suggested change

github.com/crypto-org-chain/cronos/store v0.0.0-00010101000000-000000000000

github.com/crypto-org-chain/cronos/store v0.0.5-0.20240301012710-e9bb006c8d14

I think you can pin the version here, and remove the replacement.

Will try again. go mod tidy kept on putting zeros.

will be able to fix after removing the replace

MSalopek

Thank you for your contribution and opening a PR.

I went over the PR and the linked issues/PRs on other repos.

I'm not certain we can just add it - we need to do more extensive testing.

I'm not certain the cosmos-sdk/versiondb needs to be replaced.

I need more information to figure out what this means for future chain upgrades:

Can we upgrade to v16 and add these without issues?
Is this a purely optional feature?
Does this impact upgrade speed and how?

If we add it, does it affect all users?

With this, we would need some docs or guides to communicate to validators how and why use this. Would you be able to provide a more comprehensive guide?

chillyvee · 2024-03-26T03:58:39Z

I'm not certain the cosmos-sdk/versiondb needs to be replaced.

@yihuang - Do you have a chance to comment on this?

I need more information to figure out what this means for future chain upgrades:

memiavl is an alternate key value storage db. On Jackal chain, running memiavl prior to upgrade and after upgrade resulted in a fast upgrade with no problems.

Mixing db drivers before and after upgrade will not work. Similar as switching goleveldb to rocksdb. Must delete local database storage and resync when database drivers change.

Is this a purely optional feature?

Purely optional and opt-in by configuration.

If we add it, does it affect all users?

Default opt-out. PR is configured for Opt-in-only by operators who are interested.

Does this impact upgrade speed and how?

Upgrade speed should be much faster. The one hour downtime for the previous hub upgrade would be greatly reduced. If you prefer an actual tested time, can provision a node an give an actual time.

The statesync chunk apply time for Theta Testnet is approximately:

70 minutes for goleveldb (as is - on test server)
2 minutes for memiavl (same server)

Upgrades changing key value pairs should have similar performance improvements. Statesync is an upgrade from height 0 to current head height.

Can we upgrade to v16 and add these without issues?

Typically no issue seen adding. memiavl needs to be active

With this, we would need some docs or guides to communicate to validators how and why use this. Would you be able to provide a more comprehensive guide?

If this PR is acceptable, we can allocate time to create usage documentation prior to merge.

yihuang · 2024-03-26T04:12:44Z

I'm not certain the cosmos-sdk/versiondb needs to be replaced.
@yihuang - Do you have a chance to comment on this?

Yes, unneeded, there's no this package in the first place.

chillyvee · 2024-03-26T13:57:35Z

updated code to remove version db as suggested by @yihuang

go 1.22 required

go: github.com/crypto-org-chain/cronos/[email protected] requires go >= 1.22; switching to go1.22.1

github-actions · 2024-05-13T00:13:56Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

chillyvee requested review from alexanderbez, zmanian, crodriguezvega, jackzampolin and a team as code owners March 1, 2024 13:41

chillyvee marked this pull request as draft March 1, 2024 13:41

chillyvee force-pushed the cv/memiavl branch from 6fd0456 to 8abc406 Compare March 1, 2024 13:44

chillyvee mentioned this pull request Mar 1, 2024

CV Optimistic Export and Import cosmos/iavl#878

Open

yihuang reviewed Mar 1, 2024

View reviewed changes

chillyvee force-pushed the cv/memiavl branch from afd3091 to 8fd74df Compare March 1, 2024 16:08

chillyvee marked this pull request as ready for review March 1, 2024 16:56

chillyvee force-pushed the cv/memiavl branch 2 times, most recently from 3ca7cd9 to 071abe4 Compare March 7, 2024 17:59

MSalopek reviewed Mar 25, 2024

View reviewed changes

chillyvee and others added 5 commits March 26, 2024 13:47

CV Add memiavl based storage

60ac8fa

CV remove unnecessary store override

db0dd86

CV pinned cronos/store e9bb006c8d14

83edb66

CV document memiavl options

5f8fc73

CV remove unnecessary versiondb

b43866d

chillyvee force-pushed the cv/memiavl branch from 071abe4 to b43866d Compare March 26, 2024 13:48

github-actions bot added the stale label May 13, 2024

github-actions bot closed this May 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat/Add memiavl based storage #2973

feat/Add memiavl based storage #2973

chillyvee commented Mar 1, 2024 •

edited

Loading

yihuang Mar 1, 2024

chillyvee Mar 1, 2024 •

edited

Loading

yihuang Mar 1, 2024 •

edited

Loading

yihuang Mar 1, 2024 •

edited

Loading

chillyvee Mar 1, 2024

chillyvee Mar 1, 2024

yihuang Mar 1, 2024 •

edited

Loading

chillyvee Mar 1, 2024 •

edited

Loading

yihuang Mar 1, 2024

chillyvee Mar 1, 2024

yihuang Mar 1, 2024

chillyvee Mar 1, 2024 •

edited

Loading

yihuang Mar 1, 2024 •

edited

Loading

yihuang Mar 1, 2024

chillyvee Mar 1, 2024

chillyvee Mar 1, 2024

chillyvee Mar 1, 2024

MSalopek left a comment •

edited

Loading

chillyvee commented Mar 26, 2024 •

edited

Loading

yihuang commented Mar 26, 2024

chillyvee commented Mar 26, 2024

github-actions bot commented May 13, 2024

	github.com/crypto-org-chain/cronos/store v0.0.0-00010101000000-000000000000
	github.com/crypto-org-chain/cronos/store v0.0.5-0.20240301012710-e9bb006c8d14

feat/Add memiavl based storage #2973

feat/Add memiavl based storage #2973

Conversation

chillyvee commented Mar 1, 2024 • edited Loading

Description

Author Checklist

Reviewers Checklist

Choose a reason for hiding this comment

chillyvee Mar 1, 2024 • edited Loading

Choose a reason for hiding this comment

yihuang Mar 1, 2024 • edited Loading

Choose a reason for hiding this comment

yihuang Mar 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yihuang Mar 1, 2024 • edited Loading

Choose a reason for hiding this comment

chillyvee Mar 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chillyvee Mar 1, 2024 • edited Loading

Choose a reason for hiding this comment

yihuang Mar 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MSalopek left a comment • edited Loading

Choose a reason for hiding this comment

chillyvee commented Mar 26, 2024 • edited Loading

yihuang commented Mar 26, 2024

chillyvee commented Mar 26, 2024

github-actions bot commented May 13, 2024

chillyvee commented Mar 1, 2024 •

edited

Loading

chillyvee Mar 1, 2024 •

edited

Loading

yihuang Mar 1, 2024 •

edited

Loading

yihuang Mar 1, 2024 •

edited

Loading

yihuang Mar 1, 2024 •

edited

Loading

chillyvee Mar 1, 2024 •

edited

Loading

chillyvee Mar 1, 2024 •

edited

Loading

yihuang Mar 1, 2024 •

edited

Loading

MSalopek left a comment •

edited

Loading

chillyvee commented Mar 26, 2024 •

edited

Loading