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

Energy metering for persistent memory usage #766

Merged
merged 2 commits into from
Oct 15, 2024

Conversation

coolreader18
Copy link
Collaborator

Description of Changes

As on the tin. I think this is the idea, right? Or am I missing another part of the database that stores user data in-memory.

Expected complexity level and risk

2

@coolreader18 coolreader18 linked an issue Jan 26, 2024 that may be closed by this pull request
@Centril Centril self-requested a review January 26, 2024 20:10
Copy link
Contributor

@Centril Centril left a comment

Choose a reason for hiding this comment

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

Also, could you please integrate this in spacetimedb_table? I'd like to avoid mem-arch getting further away from being merged.

crates/sats/src/array_value.rs Outdated Show resolved Hide resolved
crates/core/src/database_instance_context.rs Outdated Show resolved Hide resolved
crates/core/src/database_instance_context_controller.rs Outdated Show resolved Hide resolved
crates/core/src/database_instance_context_controller.rs Outdated Show resolved Hide resolved
crates/core/src/database_instance_context_controller.rs Outdated Show resolved Hide resolved
crates/core/src/db/datastore/locking_tx_datastore/mod.rs Outdated Show resolved Hide resolved
crates/core/src/db/datastore/locking_tx_datastore/table.rs Outdated Show resolved Hide resolved
crates/core/src/db/relational_db.rs Show resolved Hide resolved
crates/core/src/database_instance_context_controller.rs Outdated Show resolved Hide resolved
crates/core/src/database_instance_context_controller.rs Outdated Show resolved Hide resolved
crates/sats/src/algebraic_value.rs Outdated Show resolved Hide resolved
crates/sats/src/array_value.rs Outdated Show resolved Hide resolved
crates/sats/src/data_key.rs Outdated Show resolved Hide resolved
crates/standalone/src/energy_monitor.rs Outdated Show resolved Hide resolved
@coolreader18 coolreader18 force-pushed the noa/memusage-metering branch 2 times, most recently from 88a3fb9 to 81bffb6 Compare February 2, 2024 17:30
@bfops bfops added energy-tracking Changes related to measuring and tracking resources within SpacetimeDB verify labels labels Feb 8, 2024
@bfops
Copy link
Collaborator

bfops commented Feb 15, 2024

@coolreader18 I just want to confirm for PR shepherding purposes - this change is backwards-compatible right?

@coolreader18
Copy link
Collaborator Author

@bfops yes, it is.

@coolreader18
Copy link
Collaborator Author

coolreader18 commented Feb 16, 2024

Oh, from the user side at least - cloud will need an update (but I have it ready already)

@bfops bfops added release-any To be landed in any release window breaks library compatibility This change breaks the SpacetimeDB library interface and removed verify labels labels Feb 16, 2024
@cloutiertyler cloutiertyler added release-1.0 and removed release-any To be landed in any release window labels Jun 3, 2024
@coolreader18
Copy link
Collaborator Author

coolreader18 commented Oct 8, 2024

The hashbrown update is because 0.15 added an allocation_size() method, which is obviously very helpful. It also changes the default hasher, though, and I didn't want this to affect anything else, so I set our default hasher back to ahash, and all the HashCollectionExt imports are because we can no longer use HashMap::new().

Copy link
Collaborator

@joshua-spacetime joshua-spacetime left a comment

Choose a reason for hiding this comment

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

Thanks @coolreader18! Just a few typos, but otherwise looks good.

You've accounted for nearly all mem usage :)

I think it would have been sufficient to just track the pages, indexes, blob store, and pointer map. That probably could have reduced a lot of the boilerplate. But this is very nice nonetheless.

crates/table/src/indexes.rs Show resolved Hide resolved
crates/table/src/memory_usage.rs Outdated Show resolved Hide resolved
crates/table/src/blob_store.rs Show resolved Hide resolved
@coolreader18 coolreader18 force-pushed the noa/memusage-metering branch 3 times, most recently from 300be74 to fb69349 Compare October 15, 2024 17:00
@coolreader18 coolreader18 added this pull request to the merge queue Oct 15, 2024
Merged via the queue into master with commit c4e637e Oct 15, 2024
7 of 8 checks passed
@coolreader18 coolreader18 deleted the noa/memusage-metering branch October 16, 2024 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaks library compatibility This change breaks the SpacetimeDB library interface energy-tracking Changes related to measuring and tracking resources within SpacetimeDB release-1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Track energy for persistent database memory
5 participants