-
Notifications
You must be signed in to change notification settings - Fork 40
feat: Introduce EphemeralStorage #655
base: main
Are you sure you want to change the base?
Conversation
/// Spawn a future by scheduling it with the local executor. The | ||
/// future will immediately start processing. | ||
pub fn spawn_immediate<F>(future: F) | ||
where |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
may not be necessary, just use spawn
with a wasm32 version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using the async fn spawn()
still alerts to use .await
hm
#[cfg_attr(not(target_arch = "wasm32"), async_trait)] | ||
#[cfg_attr(target_arch = "wasm32", async_trait(?Send))] | ||
pub trait EphemeralStorage: ConditionalSync { | ||
type EphemeralStoreType: KeyValueStore + Disposable; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need better name for EphemeralStoreType
, as the return value is EphemeralStore<Self::EphemeralStoreType>
-- EphemeralStoreInner
?
/// Spawn a future by scheduling it with the local executor. The | ||
/// future will immediately start processing. | ||
pub fn spawn_immediate<F>(future: F) | ||
where |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using the async fn spawn()
still alerts to use .await
hm
key.starts_with(self.partition_key.as_slice()) | ||
} | ||
|
||
pub fn get_key_range(&self) -> (&Vec<u8>, &Vec<u8>) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The symbol for an "end" of the prefix range could be improved possibly with the partition_key
with the last character incremented. Both Rocks and IDB use a partitioned implementation as well as have the same interface for key ranges. Rocks has a more robust solution for generating a key range from a prefix which we could adopt: https://docs.rs/rocksdb/0.21.0/rocksdb/struct.PrefixRange.html
|
a7fe73c
to
e65892e
Compare
The RocksDB build would consistently stack overflow on --- a/rust/noosphere-storage/src/db.rs
+++ b/rust/noosphere-storage/src/db.rs
@@ -40,6 +40,7 @@ where
link_store: S::KeyValueStore,
version_store: S::KeyValueStore,
metadata_store: S::KeyValueStore,
+ storage: S,
}
impl<S> SphereDb<S>
@@ -52,6 +53,7 @@ where
link_store: storage.get_key_value_store(LINK_STORE).await?,
version_store: storage.get_key_value_store(VERSION_STORE).await?,
metadata_store: storage.get_key_value_store(METADATA_STORE).await?,
+ storage: storage.to_owned(),
})
} Changing --- a/rust/noosphere-storage/src/implementation/rocks_db.rs
+++ b/rust/noosphere-storage/src/implementation/rocks_db.rs
@@ -85,13 +85,13 @@ impl Storage for RocksDbStorage {
#[derive(Clone)]
pub struct RocksDbStore {
- name: String,
+ name: Arc<String>,
db: Arc<DbInner>,
}
impl RocksDbStore {
pub fn new(db: Arc<DbInner>, name: String) -> Result<Self> {
- Ok(RocksDbStore { db, name })
+ Ok(RocksDbStore { db, name: Arc::new(name) })
} RocksDB's |
|
5d1ef80
to
4630288
Compare
This is consistently causing a stack overflow on Windows:
|
57d2290
to
869abef
Compare
# threads themselves. | ||
# | ||
# While our main thread isn't under fire here, notating this for future use: | ||
# https://users.rust-lang.org/t/stack-overflow-when-compiling-on-windows-10/50818/8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, this would be applied when running locally outside of CI.
The immediate solutions to achieve this would be to increase the stack size of only the offending test, but the test harness thread itself is overflowing it seems. Alternatively, a Cargo.toml test-only flag such that would apply in both CI and local runs, but no leads yet on how to configure something like that.
Fixed by increasing thread stack size on Windows. Separately, rerunning the failed builds resulted in a successful rocksdb test run |
rebased on #685 |
0dc1563
to
221bdaa
Compare
Test flake analysis
Flake summary for
|
EphemeralStorage
trait, allowing storages to shard off usage for anEphemeralStore
.EphemeralStorage
with tests. Sled uses ephemeral trees, and rocks/indexeddb shard a shared ephemeral store. Ephemeral stores are cleared from the provider onDrop
, and again on startup for shared ephemeral stores in the event cleanups are missed (e.g. force quit)OpenStorage
for unified paths over other trait operatorsNonPersistentStorage
for testsIterableStore
for testing internal details of store ensuring ephemeral storage is cleared (now also implemented for Rocks, with tests)