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.

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

Some range caching is broken when using the same visible time range across different views #6279

Closed
jleibs opened this issue May 9, 2024 · 0 comments · Fixed by #6292
Closed

Some range caching is broken when using the same visible time range across different views #6279

jleibs opened this issue May 9, 2024 · 0 comments · Fixed by #6292
Assignees
@teh-cmc @jleibs
Labels
🪳 bug Something isn't working ⛃ re_datastore affects the datastore itself
Milestone
0.16

Comments

@jleibs
Copy link
Member

jleibs commented May 9, 2024

Repro:

git checkout jleibs/stocks_timerange
pixi run -e examples py-build
pixi run -e examples blueprint_stocks_timerange

Many views randomly end up missing data:
image

If you save the rrd and then reload, it each time you load a different random set of views will be missing data.
image
@jleibs jleibs added 🪳 bug Something isn't working 👀 needs triage This issue needs to be triaged by the Rerun team labels May 9, 2024
@jleibs jleibs added this to the 0.16 milestone May 9, 2024
@jleibs jleibs added ⛃ re_datastore affects the datastore itself and removed 👀 needs triage This issue needs to be triaged by the Rerun team labels May 9, 2024
@jleibs jleibs mentioned this issue May 11, 2024
Merged
5 tasks
jleibs added a commit that referenced this issue May 13, 2024
@jleibs @teh-cmc
Fix issues with cache invalidation related to parallel queries. (#6292)
95b7910 
### What
- Resolves: #6279

### Issue 1: data invalidation before data use

This failure mode happens where:
  - Thread 1: produces results and returns them
  - Thread 2: invalidates results
- Thread 1: uses the new results which point to a non-overlapping range
and finds nothing of interest.

I worked around this by cloning the data and orphaning the Inner when we
execute an invalidation. This feels a bit heavy-handed for the cases
where we don't need it. Maybe there's an alternative way to keep the
structure locked for read until the results have been consumed?

### Issue 2: not considering pending data

The `time_range` logic wasn't factoring in the possibility of
pending_data. This meant we could have pending gaps in queries that had
not yet been resolved, but wouldn't count them as gaps. Subsequently as
the promises are resolved we would still end up with gaps in the data.

`time_range()` has now been replaced with `pending_time_range()` to do
the proper book-keeping.

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6292?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6292?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/6292)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.

---------

Co-authored-by: Clement Rey <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@teh-cmc teh-cmc

@jleibs jleibs

Labels
🪳 bug Something isn't working ⛃ re_datastore affects the datastore itself
Projects
None yet
Milestone
0.16
Development

Successfully merging a pull request may close this issue.

Fix issues with cache invalidation related to parallel queries. rerun-io/rerun
2 participants
@teh-cmc @jleibs