Feat/lazy load deployment #2519

joske · 2024-07-12T13:51:25Z

Motivation

We've seen that RSS size is correlated with deployments loaded (and staying) in memory. This PR keeps MAX_STACKS programs in memory (chosen arbitrarily at 1000).

Test Plan

Tested by running a local devnet, deploying programs and executing them. Then restarting the network and trying to execute these programs. The Stacks were lazily loaded from storage, execution worked. Also tested executing programs that are not deployed.

raychu86 · 2024-07-16T01:23:49Z

Would love some unit tests for this as well! An easy way to do this would be to use a #[cfg(any(test, feature = "test"))] and set MAX_STACKS to be a small value for testing purposes only.

raychu86 · 2024-07-16T01:28:47Z

synthesizer/process/src/lib.rs

+        // try to lazy load the stack
+        #[cfg(feature = "rocks")]
+        let store = ConsensusStore::<N, ConsensusDB<N>>::open(storage_mode);
+        #[cfg(not(feature = "rocks"))]
+        let store = ConsensusStore::<N, ConsensusMemory<N>>::open(storage_mode);


It may be easier to either have C: ConsensusStorage as a generic or to pass in storage somehow.

It might break some APIs, but it feels cleaner than reloading the storage it at each instance.

yes, this was the original idea. but adding the generic cascaded everywhere... Will look into that.

This cascades too much. The perf hit to open storage is about 2s (on a 15GB ledger). That's not negligible, but hopefully we don't need to keep so many programs in memory.

found a way to avoid opening storage every time 🎉

kpandl · 2024-09-04T11:49:20Z

When running the snarkOS ./devnet.sh script (with the commits of this PR attached to snarkVM) with e.g. 10 validators (and no clients), the nodes fail to load the ledger and the network doesn't start.

Typically, the logs look similar to this:
9 validators log:

DEBUG snarkvm_synthesizer_process: Opening storage
thread 'main' panicked at cli/src/commands/start.rs:176:84:
Failed to parse the node: Failed to load ledger (run 'snarkos clean' and try again)

IO error: While lock file: /Users/user/dev/snarkOS/.ledger-0-0/LOCK: Resource temporarily unavailable

1 validator logs:

INFO snarkvm_ledger: Loading the ledger from storage...
thread 'main' panicked at cli/src/commands/start.rs:176:84:
Failed to parse the node: Failed to load ledger (run 'snarkos clean' and try again)
Mismatching network ID or storage mode in the database

joske · 2024-09-04T12:29:40Z

lock file: /Users/user/dev/snarkOS/.ledger-0-0/LOCK: Resource temporarily unavailable This is a bit strange. Do all nodes have this?

kpandl · 2024-09-04T12:32:45Z

Yes, all nodes had it. On aws, validator 0 worked, all other validators got stuck on this:

2024-09-03T14:50:51.110986Z DEBUG Opening storage
2024-09-03T14:50:51.172513Z DEBUG Opened storage
2024-09-03T14:51:08.816863Z INFO Loading the ledger from storage...

(the --no-dev-txs flag was set)

joske · 2024-09-04T12:43:28Z

did you put --dev 0 on all by accident?

vicsn · 2024-09-09T13:43:55Z

synthesizer/process/src/lib.rs

    }
 }

 impl<N: Network> Process<N> {
    /// Initializes a new process.
    #[inline]
    pub fn load() -> Result<Self> {
+        // Assumption: this is only called in test code.
+        Process::load_from_storage(Some(aleo_std::StorageMode::Development(0)))


Noting this may be the source of our troubles, and we may want to enclose the function in #[cfg(any(test, feature = "test"))] if possible.

Yes this code needs to be updated.

marking as draft again.

4aa125b looks good.

If enclosing fn load() in something like #[cfg(any(test, feature = "test"))] is too difficult (perhaps because its called both by tests and benchmarks?), another safer approach could be to get rid of fn load() alltogether. Should take you just one search and replace to call Process::load_from_storage(Some(aleo_std::StorageMode::Development(0))) directly, and that keeps the aleo_std::StorageMode::Development(0) logic contained to within tests and benchmarks. That makes a future developer less likely to make mistakes. UPDATE: See new comment below.

I realise also that the current approach can lead to deadlocks on the storage when running tests, because all the tests will try to access the same database... Looks like we also didn't run CI on this PR yet (you can ask @ljedrz for how he has been running CI with a separate -ci branch).

New proposal: what about we let fn load use store: None. Just like load_from_storage , it should load the credits.aleo program because the tests need it. To avoid adding too many lines of code, you can still abstract similar logic to a shared function.

We can not use None as this breaks many tests.

Clear. Then we should rename load to load_testing_only.

vicsn · 2024-09-26T11:21:43Z

synthesizer/process/src/tests/test_process.rs

@@ -0,0 +1,92 @@
+// Copyright (C) 2019-2023 Aleo Systems Inc.


This license needs to be updated, see e.g. https://github.com/AleoNet/snarkVM/blob/mainnet-staging/fields/src/fp12_2over3over2.rs#L1

vicsn · 2024-10-01T12:09:34Z

synthesizer/process/Cargo.toml

@@ -93,9 +96,12 @@ package = "snarkvm-utilities"
 path = "../../utilities"
 version = "=0.16.19"

+[dependencies.tracing]


Hate to nit, but these are ordered alphabetically, should be moved after [dependencies.serde_json]

vicsn · 2024-10-01T12:17:54Z

synthesizer/process/Cargo.toml

@@ -78,6 +78,9 @@ package = "snarkvm-ledger-store"
 path = "../../ledger/store"
 version = "=0.16.19"

+[dependencies.lru]


Same nit, all the non-Aleo dependencies are below

vicsn · 2024-10-07T15:54:33Z

Closed by #2553

joske force-pushed the feat/lazy-load-deployment branch 2 times, most recently from a334232 to 61872f3 Compare July 15, 2024 07:44

raychu86 reviewed Jul 16, 2024

View reviewed changes

joske force-pushed the feat/lazy-load-deployment branch 3 times, most recently from 2e79b48 to ab3fb4b Compare July 30, 2024 14:48

joske marked this pull request as ready for review July 30, 2024 14:58

joske requested a review from raychu86 July 30, 2024 14:59

joske added 5 commits July 31, 2024 09:14

feat: lazy load deployments

80ec42d

chore: update lock

48c1541

feat: add tests for cache

27890a8

chore: bump MAX_STACKS to 1000

1a85d9a

fix: avoid lazy loading cost

dc9340b

joske force-pushed the feat/lazy-load-deployment branch from ab3fb4b to dc9340b Compare July 31, 2024 07:14

vicsn requested a review from niklaslong July 31, 2024 10:45

vicsn reviewed Sep 9, 2024

View reviewed changes

joske marked this pull request as draft September 9, 2024 19:02

fix: avoid loading the ledger storage in the EpochProgram case

4aa125b

vicsn reviewed Sep 26, 2024

View reviewed changes

vicsn reviewed Oct 1, 2024

View reviewed changes

vicsn marked this pull request as ready for review October 1, 2024 12:54

vicsn mentioned this pull request Oct 7, 2024

Feat/lazy load deployment #2553

Merged

vicsn closed this Oct 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/lazy load deployment #2519

Feat/lazy load deployment #2519

joske commented Jul 12, 2024 •

edited

Loading

raychu86 commented Jul 16, 2024

raychu86 Jul 16, 2024

joske Jul 16, 2024

joske Jul 30, 2024

joske Jul 30, 2024

kpandl commented Sep 4, 2024

joske commented Sep 4, 2024

kpandl commented Sep 4, 2024

joske commented Sep 4, 2024

vicsn Sep 9, 2024

joske Sep 9, 2024

joske Sep 9, 2024

vicsn Sep 10, 2024 •

edited

Loading

vicsn Sep 10, 2024

joske Sep 11, 2024

vicsn Oct 3, 2024

vicsn Sep 26, 2024

vicsn Oct 1, 2024

vicsn Oct 1, 2024

vicsn commented Oct 7, 2024

		@@ -0,0 +1,92 @@
		// Copyright (C) 2019-2023 Aleo Systems Inc.

Feat/lazy load deployment #2519

Feat/lazy load deployment #2519

Conversation

joske commented Jul 12, 2024 • edited Loading

Motivation

Test Plan

raychu86 commented Jul 16, 2024

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

kpandl commented Sep 4, 2024

joske commented Sep 4, 2024

kpandl commented Sep 4, 2024

joske commented Sep 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vicsn Sep 10, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vicsn commented Oct 7, 2024

joske commented Jul 12, 2024 •

edited

Loading

vicsn Sep 10, 2024 •

edited

Loading