Update columnar, timely, differential #33938
Conversation
antiguru
force-pushed
the
update_c_td_dd
branch
from
76e19c9 to
af8b4ed
Compare
| if let Some(frontier) = &ts_rewrite { | ||
| h.write_usize(frontier.len()); | ||
| frontier.iter().for_each(|t| t.encode().hash(&mut h)); | ||
| } |
There was a problem hiding this comment.
@bkirwi Do you think this change is correct? We don't require Timestamp to be Hash, so instead of requiring it, I'm pivoting to hasing the codec.
There was a problem hiding this comment.
Yeah, this is fine! Codec64 guarantees that it's equally unique, and since this is just a debug assert helper we don't have to worry too much about the (probably very small number of) extra cycles.
antiguru
force-pushed
the
update_c_td_dd
branch
from
26058c6 to
2f8d051
Compare
def-
left a comment
def-
left a comment
There was a problem hiding this comment.
test change lgtm, thanks for triggering nightly
teskje
left a comment
teskje
left a comment
There was a problem hiding this comment.
Made it through the compute parts, but not the storage parts yet.
antiguru
force-pushed
the
update_c_td_dd
branch
from
2f8d051 to
95c516c
Compare
src/storage/src/render/sinks.rs
Outdated
| use crate::storage_state::StorageState; | ||
|
|
||
| /// _Renders_ complete _differential_ [`Collection`]s | ||
| /// _Renders_ complete _differential_ Collections |
There was a problem hiding this comment.
Nit: I'd lower-case these (here and in src/storage/src/render/sources.rs) to make it more obvious that this refers to the concept, not a specific type.
| if let Some(capability) = &self.capability | ||
| && cap.time() != capability.time() | ||
| { | ||
| self.flush(); | ||
| self.capability = None; | ||
| } | ||
| if self.capability.is_none() { | ||
| self.capability = Some(cap.clone()); | ||
| } |
There was a problem hiding this comment.
This leaves the opportunity to "upgrade" the capability, right? Also you can re-activate an output after it has been ceased. Seems safer to never drop the existing capability and downgrade it instead.
There was a problem hiding this comment.
We can't downgrade the capability if we're in partially-ordered time though, so I'd rather drop it. It shouldn't make a performance difference as a downgrade is a retract and insert under the hood.
Signed-off-by: Moritz Hoffmann <[email protected]>
Signed-off-by: Moritz Hoffmann <[email protected]>
antiguru
force-pushed
the
update_c_td_dd
branch
from
95c516c to
1405be4
Compare
4 participants
Update Columnar, Timely and Differential.
TODO:
The async operator builder in its current form will emit single-record containers.✔️ Also removed unsafe code.