Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

soroban-rpc: Review and improve memory consumption #554

Closed
2opremio opened this issue Apr 4, 2023 · 2 comments
Closed

soroban-rpc: Review and improve memory consumption #554

2opremio opened this issue Apr 4, 2023 · 2 comments

Comments

@2opremio
Copy link
Contributor

2opremio commented Apr 4, 2023

Soroban-rpc uses some in-memory data structures for recent data (LedgerBucketWindow Go datastructure).

It would be good to know how much memory soroban-rpc needs when running in pubnet (average and peak).

While working on #553 I noticed that, when bootstrapping the in-memory datastructures, we read all the txmeta from the database at once:

https://github.com/stellar/soroban-tools/blob/a210572b64a91b862ec3190bba627ea0503282c2/cmd/soroban-rpc/internal/daemon/daemon.go#L130-L143

This worries me, since, when running on pubnet, the DB contains ~20GB worth of txmeta. Probably resulting in a memory consumption north of 20GB.

Note that this will add on top of the captive-core memory consumption.

We should consider

  • Streaming the txmeta in batches when bootstrapping.
  • Reducing the time window we use for the in-memory data structures.
  • Last resort: move some of the in-memory datastructures to the on-disk DB.

In the context of this issue, we should also re-evaluate the amount of memory effectively used by the in-memory transaction store. Currently, the default is set to 2 hours ( 1440 ledgers ). We might want to revise the default based on this analysis.

@2opremio 2opremio changed the title soroban-rpc: Review and improve memory requirements soroban-rpc: Review and improve memory consumption Apr 4, 2023
@mollykarcher mollykarcher moved this from Backlog to Next Sprint Proposal in Platform Scrum Apr 11, 2023
@mollykarcher mollykarcher moved this from Next Sprint Proposal to Backlog in Platform Scrum Apr 26, 2023
@mollykarcher mollykarcher moved this from Backlog to Current Sprint in Platform Scrum Apr 26, 2023
@mollykarcher mollykarcher moved this from Current Sprint to Backlog in Platform Scrum May 24, 2023
@2opremio 2opremio self-assigned this May 29, 2023
@tsachiherman tsachiherman moved this from Backlog to Next Sprint Proposal in Platform Scrum Jun 5, 2023
@tsachiherman tsachiherman moved this from Next Sprint Proposal to Backlog in Platform Scrum Jun 6, 2023
@mollykarcher mollykarcher added this to the Soroban Pubnet Release milestone Jul 11, 2023
@mollykarcher mollykarcher moved this from Backlog to Next Sprint Proposal in Platform Scrum Sep 27, 2023
@mollykarcher mollykarcher moved this from Next Sprint Proposal to Current Sprint in Platform Scrum Oct 10, 2023
@mollykarcher mollykarcher assigned stellarsaur and unassigned 2opremio Oct 10, 2023
@stellarsaur
Copy link
Contributor

#904 Addresses the main memory consumption issue - now we stream ledgers instead of load them all at once from the database.

The other main areas of memory usage are:

  • Captive-core
  • LedgerBucketWindow

As long as we have a bounded retention window for transactions and events, memory usage shouldn't be an issue. Note that we retain 2 hours of ledgers in the transaction store, and 24 hours of ledgers in the events store. If anything, we could consider reducing the retention window for the events store to something like 12 hours of ledgers, but it probably won't make a substantial difference. Also, we could consider reducing the retention window for the transactions store to something like 1 hour of ledgers, but once again, it probably won't make a substantial difference unless the ledgers were huge.

Finally, preliminary load tests outlined here didn't reveal any issues with memory usage running against testnet and futurenet.

All in all, I would consider this issue solved. We can always reopen if memory usage becomes problematic in the future.

@stellarsaur
Copy link
Contributor

Based on an internal discussion, rather than decreasing the transaction or event ledger windows, we actually may want to increase the transaction ledger window to 24 hours, since it's what users expect.

Closing this but we can reopen it in the future if memory usage becomes an issue.

@github-project-automation github-project-automation bot moved this from Current Sprint to Done in Platform Scrum Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

4 participants