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.

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

Fix wrong RowId order of logged data #4658

Merged
merged 5 commits into from
Jan 3, 2024
Merged

Fix wrong RowId order of logged data #4658

merged 5 commits into from
Jan 3, 2024

Conversation

emilk
Copy link
Member

@emilk emilk commented Jan 3, 2024

What

Rerun is designed to be able to handle out-of-order ingestion, but with a performance hit.

Usually that performance hit is very small, but when logging to the exact same timestamp many times, we hit a corner-case of the data store where we get a huge bucket (see #4415). If the data arrives out-of-order, this means an expensive resort of the huge bucket on each ingestion.

Usually such out-of-order log rows should only happen in multi-threaded applications, but due to a bug it happened almost always. This PR fixes this bug, and adds regression test for it. This PR thus alleviates the problem in #4415, but does not fix it for actual out-of-order multi-threaded applications.

I introduced the bug in #4502 after the 0.11 release. EDIT: no, it was pre-existing!

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I have tested the web demo (if applicable):
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG

@emilk emilk added 🪳 bug Something isn't working 📉 performance Optimization, memory use, etc include in changelog exclude from changelog PRs with this won't show up in CHANGELOG.md and removed include in changelog labels Jan 3, 2024
@emilk emilk changed the title Emilk/fix row id order Fix wrong RowId order of logged data Jan 3, 2024
@emilk emilk added include in changelog and removed exclude from changelog PRs with this won't show up in CHANGELOG.md labels Jan 3, 2024
@emilk emilk added this to the 0.12 milestone Jan 3, 2024
crates/re_sdk/src/recording_stream.rs Outdated Show resolved Hide resolved
crates/rerun/tests/rerun_tests.rs Outdated Show resolved Hide resolved
@emilk emilk merged commit 18aac95 into main Jan 3, 2024
40 checks passed
@emilk emilk deleted the emilk/fix-row-id-order branch January 3, 2024 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🪳 bug Something isn't working include in changelog 📉 performance Optimization, memory use, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants