-
Notifications
You must be signed in to change notification settings - Fork 373
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
DataStore
changelog 2: introduce StoreEvent
s
#4203
Conversation
05a71ef
to
88af0d0
Compare
092b7e0
to
d0fa7a8
Compare
88af0d0
to
b0b1603
Compare
d0fa7a8
to
009dcfa
Compare
b0b1603
to
b00c552
Compare
009dcfa
to
70571cd
Compare
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.
nicely documented <3
a bit of a pitty how a few things in store_gc increased in code length, but not seeing any obvious shortucts
|
||
/// Is it an addition or a deletion? | ||
/// | ||
/// Reminder: ⚠ Do not confuse _a deletion_ and _a clear_ ⚠. |
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 store's internals are opaque and don't necessarily reflect the query model (e.g. there | ||
/// might be data in the store that cannot by reached by any query). | ||
/// | ||
/// A [`StoreDiff`] answers a logical question: "does there exist a query path which can return | ||
/// data from that row?". |
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.
makes sense, but what has the reachability of rows to do with whether this is an addition or deletion?
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.
That's the point I'm trying to make: an event of kind deletion only tells you that, from this point on, no query can return data from that row.
But that doesn't mean that the data is actually gone, i.e. don't make assumptions of e.g. the size in bytes of the store based on these events. They are in "query-model space" and are not an accurate representation of what happens in storage space.
/// Since insertions and deletions both work on a row-level basis, this is guaranteed to be the | ||
/// same set of values for both the insertion and deletion events (if any). | ||
pub cells: IntMap<ComponentName, DataCell>, | ||
} |
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.
love how you clarify all the logical invariants here, will make it much easier for new comers to implement StoreViews!
@@ -71,6 +71,7 @@ polars-ops = { workspace = true, optional = true, features = [ | |||
|
|||
|
|||
[dev-dependencies] | |||
re_log_types = { workspace = true, features = ["testing"] } |
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.
at the very least in this pr this doesn't seem to happen. Why is it needed now?
(also, makes me sad!)
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.
Probably needed at some point, dont remember... in any case it can never hurt and will probably save someone some pain in the future so I'll leave it there 🤷
if event.is_timeless() { | ||
let per_component = this.timeless.entry(event.entity_path.hash()).or_default(); | ||
for component_name in event.cells.keys() { | ||
*per_component.entry(*component_name).or_default() += |
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.
shouldn't the delta be always negative? Or asked differently, isn't the right thing to do to first check whether the event is a deletion event? Seems strange not to do that
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.
Yes, this looks weird because this is in a temporary state. In future PRs this whole Deleted
thing actually becomes CompactedStoreEvents
, and works for both additions and deletions.
b00c552
to
1f9b9c6
Compare
…4202) The upcoming `StoreView` works in global scope: by registering a view you subscribe to changes to _all_ `DataStore`s, including those that are yet to be created. This is very powerful as it allows views & triggers implementers to build cross-recording indices as well as be notified as soon as new recordings come in and go out. But it means that `StoreEvent`s must indicate which `DataStore` they originate from, which isn't possible today since the stores themselves don't know who they are to begin with. This trivial PR plumbs the `StoreId` all the way through so `DataStore`s know about their own ID. Also made `StoreGeneration` account for the GC counter while I was at it. --- Requires: - #4215 `DataStore` changelog PR series: - #4202 - #4203 - #4205 - #4206 - #4208 - #4209
e0e5d6d
to
93d6739
Compare
Introducing the `StoreView` trait and registration system, allowing anybody to subscribe to `DataStore` changes, even from external code. `StoreView`s global scope: by registering a view you subscribe to changes to _all_ `DataStore`s, including those that are yet to be created. This is very powerful as it allows views & triggers implementers to build cross-recording indices as well as be notified as soon as new recordings come in and go out. ```rust /// A [`StoreView`] subscribes to atomic changes in one or more [`DataStore`]s through [`StoreEvent`]s. /// /// [`StoreView`]s can be used to build both secondary indices and trigger systems. pub trait StoreView: std::any::Any + Send + Sync { /// Arbitrary name for the view. /// /// Does not need to be unique. fn name(&self) -> String; /// Workaround for downcasting support, simply return `self`: /// ```ignore /// fn as_any(&self) -> &dyn std::any::Any { /// self /// } /// ``` fn as_any(&self) -> &dyn std::any::Any; /// Workaround for downcasting support, simply return `self`: /// ```ignore /// fn as_any_mut(&mut self) -> &mut dyn std::any::Any { /// self /// } /// ``` fn as_any_mut(&mut self) -> &mut dyn std::any::Any; /// The core of this trait: get notified of changes happening in one or more [`DataStore`]s. /// /// This will be called automatically by the [`DataStore`] itself if the view has been /// registered: [`DataStore::register_view`]. /// Or you might want to feed it [`StoreEvent`]s manually, depending on your use case. /// /// ## Example /// /// ```ignore /// fn on_events(&mut self, events: &[StoreEvent]) { /// use re_arrow_store::StoreDiffKind; /// for event in events { /// match event.kind { /// StoreDiffKind::Addition => println!("Row added: {}", event.row_id), /// StoreDiffKind::Deletion => println!("Row removed: {}", event.row_id), /// } /// } /// } /// ``` fn on_events(&mut self, events: &[StoreEvent]); } ``` --- `DataStore` changelog PR series: - #4202 - #4203 - #4205 - #4206 - #4208 - #4209
…4206) Standalone example of how to implement and register custom `StoreView`s, even from external code. <picture> <img src="https://static.rerun.io/custom_store_view/f7258673486f91d944180bd4a83307bce09b741e/full.png" alt=""> <source media="(max-width: 480px)" srcset="https://static.rerun.io/custom_store_view/f7258673486f91d944180bd4a83307bce09b741e/480w.png"> <source media="(max-width: 768px)" srcset="https://static.rerun.io/custom_store_view/f7258673486f91d944180bd4a83307bce09b741e/768w.png"> <source media="(max-width: 1024px)" srcset="https://static.rerun.io/custom_store_view/f7258673486f91d944180bd4a83307bce09b741e/1024w.png"> <source media="(max-width: 1200px)" srcset="https://static.rerun.io/custom_store_view/f7258673486f91d944180bd4a83307bce09b741e/1200w.png"> </picture> --- `DataStore` changelog PR series: - #4202 - #4203 - #4205 - #4206 - #4208 - #4209
This is mostly preliminary work for #4209, which makes this PR a bit weird. Basically just trying to offload complexity from #4209. `TimesPerTimeline` as well as `TimeHistogramPerTimeline` are now living on their own and are maintained as `StoreView`s, i.e. they react to changes to the `DataStore` rather than constructing alternate truths. This is the first step towards turning the `EntityTree` giga-structure into an event-driven view in the next PR. --- `DataStore` changelog PR series: - #4202 - #4203 - #4205 - #4206 - #4208 - #4209
Turns the `EntityTree` giga-datastructure into a `StoreView`, meaning it now reacts to `StoreEvent`s rather than creating alternate truths. This introduces the notion of cascading side-effects, and more specifically `ClearCascade`s. When the `EntityTree` reacts to changes in the store, this might cause cascading effects (e.g. pending clears), that in turn need to write back to the store, which in turn sends more events to react to! The cycle is guaranteed finite because "clears don't get cleared"! Cascading side-effects have an interesting requirement: they need to log their cascaded data using a `RowId` _similar_ to the one used in the original event that caused the cascade (so they get GC'd at roughly the same pace). "Similar" in this cases means that their `TUID` shares the same timestamp and that the new `RowId` is strictly greater than the old one. `PathOp` has finally been annihilated. According to our new "Clears" & "Time Histograms" test suites, this behaves exactly like the `main` branch. --- `DataStore` changelog PR series: - #4202 - #4203 - #4205 - #4206 - #4208 - #4209
Introduces
StoreEvent
, an event that describes the atomic unit of change in the RerunDataStore
: a row has been added to or removed from the store.StoreEvent
s are fired on both the insertion and garbage collection paths, enabling listeners to build arbitrary, always up-to-date views & trigger systems.DataStore
changelog PR series:DataStore
changelog 1: letDataStore
s know about theirStoreId
#4202DataStore
changelog 2: introduceStoreEvent
s #4203DataStore
changelog 3: introduceStoreView
s #4205DataStore
changelog 4: add standalone "Custom StoreView" example #4206DataStore
changelog 5: event-driven time histograms #4208DataStore
changelog 6: event-drivenEntityTree
#4209Checklist