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

Add size limit for storing in the DB #71

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

michelemin
Copy link
Collaborator

@michelemin michelemin commented Jan 7, 2025

This PR adds an optional size limit that modules must respect when storing data in the permanent storage (e.g., DB).
If a module tries to store more than it is allowed, the insertion will fail.

Modules can also be given (by default or through overrides) Unlimited storage in the DB, in which case we will still count how many bytes they are storing, but we will not fail the insert.

When counting what is stored, we take into account the byte length of the key and the values.

Breaking changes

The configuration needs a new section that looks like

[loading.storage_size]
default = "Unlimited"
[loading.storage_size.log_type]
[loading.storage_size.module_overrides]
"test_db.wasm" = { Limited = 50 }

@@ -104,6 +104,10 @@ pub struct Env {
pub api: Arc<Api>,
// A handle to the storage system if one is configured
pub storage: Option<Arc<Storage>>,
// Number of bytes the module is currently saving in persistent storage
pub storage_current: Arc<RwLock<u64>>,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mutex might be faster here. The critical section for these kinds of checks are going to be super short. RWLock isn't a problem but worth looking into.

// Note: we _substract_ the size of existing data. If we were to insert the new data, the old data would be overwritten.
let used_storage = match env_data.storage_current.read() {
Ok(data) => *data,
Err(_) => panic!(),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should never panic. This needs to be surfaced and handled in some way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants