Skip to content

arrow datastore: read+write query path MVP #428

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.

Sign up for GitHub

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

Jump to bottom
Merged
teh-cmc merged 63 commits into from
Dec 5, 2022
Merged

arrow datastore: read+write query path MVP #428

teh-cmc merged 63 commits into from
Dec 5, 2022

Conversation

@teh-cmc
Copy link
Member

@teh-cmc teh-cmc commented Dec 1, 2022 •
edited
Loading

An MVP implementation of an Arrow-based datastore that closely follows the design that we've been mulling over the past few weeks.

This is entirely dynamic: insert anything you want, retrieve anything you want!

Consider this step 1 of the usual "1) make it work 2) make it correct 3) make it fast".

teh-cmc added 30 commits December 2, 2022 00:05
@teh-cmc
object path => entity path
a7d245e
@teh-cmc
move utils from lib.rs to dedicated file
86a240d
@teh-cmc
color_rgba -> color_srgba_unmultiplied
b9818fc
@teh-cmc
getting intimate with arrow's datamodel
8e6fc73
@teh-cmc
getting _even more_ intimate with arrow's datamodel
5c12105
@teh-cmc
split it
0b826e9
@teh-cmc
building dem index keys
170c745
@teh-cmc
disgustingly, incorrectly inserting components all the way down
d51d29f
@teh-cmc
timelines need no list
dd6d007
@teh-cmc
similarly clarifying the nested listing situation, on the components …
cf9b660 
…side this time
@teh-cmc
make sure it looks like it should!
5064f0d
@teh-cmc
actual integration tests
32e97b8
@teh-cmc
bootstrapping text-based debugging
8042c7d
@teh-cmc
bootstrapping indices
0820524
@teh-cmc
introducing TypedTimeInt everywhere
21da23c
@teh-cmc
full index sorting
399d4f0
@teh-cmc
auto-inserting empty component lists in starting buckets
af30b16
@teh-cmc
better datagen tools
403ff95
@teh-cmc
bidirectional merges for indices + properly showing NULLs in dataframes
1d670c4
@teh-cmc
finally can show off some more advanced ingestion patterns!
7614743
@teh-cmc
dealing with corrupt validity bitmaps, and the sheer size of my stupi…
feba2ef 
…dity
@teh-cmc
read path taking its first steps: latest_at for indices!
5a0c95a
@teh-cmc
look! it's a read path!
209b315
@teh-cmc
it works!
3d79db0
@teh-cmc
show the resulting dataframe duh
2daec6b
@teh-cmc
clean up pass #1: task log
0900305
@teh-cmc
clean up pass #2: moving everybody where they belong
b288d16
@teh-cmc
clean up pass #3: definitions
ff73f56
@teh-cmc
a minimal solution for missing components
9edf1ab
@teh-cmc
Merge remote-tracking branch 'origin/main' into cmc/arrow_store_conti…
87b631d 
…nued
@teh-cmc teh-cmc marked this pull request as ready for review December 4, 2022 18:15
@teh-cmc
Copy link
Member Author

teh-cmc commented Dec 4, 2022

Unsurprisingly, actually thoroughly testing the thing surfaced a million critical bugs 🙄: added tests and fixes for all of those.

Still need to add the benchmark suite before calling it a complete skeleton, but already I feel much more confident undrafting this.

@teh-cmc teh-cmc force-pushed the cmc/arrow_store_continued branch from 434741f to f424684 Compare December 5, 2022 10:44
@teh-cmc
Copy link
Member Author

teh-cmc commented Dec 5, 2022

  • Added a very simple integration benchmark suite, akin to the one we have for the current data store.
    The APIs and overall models are different enough that it's kinda impossible to port this 1-to-1 right now, so it results in a kinda awkward bench, but at least it's something we can start iterating from.

  • Exploded the DataFrames returned by query().
    Literally just called .explode(), so it's actually possible to do useful stuff with the results (e.g. tests & benchmarks 🙄)

This was linked to issues Dec 5, 2022
re_datastore: basic benchmark suite #434
Closed
re_datastore: do a first pass of ensuring all around correctness #441
Closed
emilk
emilk requested changes Dec 5, 2022
View reviewed changes
Copy link
Member

@emilk emilk left a comment

Choose a reason for hiding this comment

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

Amazing work 👏👏👏

I found some room for improvement though 😆

crates/re_arrow_store/src/store.rs
/// entity: this/that
/// buckets: [
/// IndexBucket {
/// time range: from -∞ (inclusive) to +9223372036.855s (exlusive)
Copy link
Member

@emilk emilk Dec 5, 2022

Choose a reason for hiding this comment

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

this doesn't look like an accurate time range, given the data below

Copy link
Member Author

@teh-cmc teh-cmc Dec 5, 2022

Choose a reason for hiding this comment

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

I purposely want it to look wrong though: this PR completely ignores bucketing and as such it doesn't pretend to get anything right about it.

@teh-cmc teh-cmc requested a review from emilk December 5, 2022 16:19
@teh-cmc
Copy link
Member Author

teh-cmc commented Dec 5, 2022

Alright, that should hopefully cover it all @emilk!

Well, apart from the bucketing stuff; but as I said above, that's the subject of the next 2 PRs, on which I will jump ASAP :)

@teh-cmc teh-cmc merged commit bd69add into main Dec 5, 2022
@teh-cmc teh-cmc deleted the cmc/arrow_store_continued branch December 5, 2022 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@emilk emilk emilk approved these changes

Assignees

No one assigned

Labels

🏹 arrow Apache Arrow ⛃ re_datastore affects the datastore itself

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

re_datastore: do a first pass of ensuring all around correctness re_datastore: basic benchmark suite Out of order Arrow ingestion

4 participants

@teh-cmc @emilk @jondo2010