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

gnovm store: SetCachePackage stores packages on memory. Possible OOM errors. #812

Open
ajnavarro opened this issue May 9, 2023 · 7 comments
Assignees
Labels
📦 🤖 gnovm Issues or PRs gnovm related 🌟 improvement performance improvements, refactors ...

Comments

@ajnavarro
Copy link
Contributor

ajnavarro commented May 9, 2023

Description

After trying to reduce the memory usage moving from maps to an LRU cache here (#780), I realized that packages set on cacheObjects are not expected to be evicted, causing problems if we use an LRU cache.

Possible Solution

We can store these packages on persistent storage using a special key like /tmp/[UUID]/oid:[OID] where [UUID] is a unique ID created per storage session (every time you initialise a store or purge it)

When purging cache calling ClearObjectCache we generate a new UUID and we remove all keys under the previous UUID /tmp/[UUID]

On every server restart, we remove all keys under /tmp

WDYT?

@thehowl
Copy link
Member

thehowl commented May 12, 2023

So your proposal is to have a LRU cache which additionally has some disk-backed data for cache misses?

Could another solution not be of requiring "temporary" / "throwaway" packages added with SetCachePackage to be finalised?

@ajnavarro
Copy link
Contributor Author

@thehowl Yes, that might be another solution. However, we will need more research about the actual codebase to check where this method is used to ensure we can modify the behavior.

@moul
Copy link
Member

moul commented Oct 15, 2024

Do we agree that this issue is indeed a bug, but we might consider postponing it until after the launch? In practice, triggering this DDoS could be costly, and it may be easier to ensure our servers have adequate memory for the launch.

Alternatively, do you believe this DDoS is likely to occur and should be resolved before the launch?

@ajnavarro
Copy link
Contributor Author

@moul It might not be a problem for the launch, but if we start to have several hundreds of packages, a malicious realm can import all of them causing nodes to OOM.

@Kouteki
Copy link
Contributor

Kouteki commented Oct 16, 2024

I'll put it down as a fast follow to resolve immediately after launch.

@zivkovicmilos
Copy link
Member

@Kouteki @moul
I think this might be a big problem, as @ajnavarro says, we can deploy a Realm with many imports, and start spamming Query or Call on the RPC to take up resources. It might be a DDoS vector that we can avoid

@thehowl
Copy link
Member

thehowl commented Oct 21, 2024

more than the many imports problem, the packages are just stored in memory and always reloaded at startup. having a realm with many imports does not pose a problem, it's more a problem of adding many packages with many ast nodes, and spamming msg_addpackage until the nodes start running out of memory.

we need to have a cache of the preprocessed packages, so that only a limited amount of preprocessed data is kept in the cache at many times. this is a bit complex though to achieve effectively, as it does mean creating a list of packages to be loaded ahead of each gnovm execution, which considers direct imports as well as "indirect" (ie. values from other realms).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 🤖 gnovm Issues or PRs gnovm related 🌟 improvement performance improvements, refactors ...
Projects
Status: Triage
Development

No branches or pull requests

5 participants