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

ClearEntity Archetype & migration #3023

Closed
16 tasks done
Tracked by #2795
Wumpf opened this issue Aug 17, 2023 · 2 comments
Closed
16 tasks done
Tracked by #2795

ClearEntity Archetype & migration #3023

Wumpf opened this issue Aug 17, 2023 · 2 comments
Labels
🌊 C++ API C/C++ API specific 🍏 primitives Relating to Rerun primitives 🐍 Python API Python logging API 🦀 Rust API Rust logging API

Comments

@Wumpf
Copy link
Member

Wumpf commented Aug 17, 2023

In the new APIs style we don't have a way yet to clear entities. We decided to emulate this behavior with a special kind of archetype, getting rid of EntityPathOpMsg in the process.

  • Python support
    • Extensions
    • Unit test (constructors, serialization)
    • Doc example
  • Rust support
    • Extensions
    • Unit test (constructors, rust-to-rust roundtrip)
    • Doc example
  • C++ support
    • Extensions
    • Unit test (constructors, serialization)
    • Doc example
  • Cross-language roundtrip test
  • Remove old components
    • Clean up re_components
    • Port viewer to use new types and queries
@Wumpf Wumpf added 🐍 Python API Python logging API 🦀 Rust API Rust logging API 🍏 primitives Relating to Rerun primitives 🌊 C++ API C/C++ API specific labels Aug 17, 2023
@Wumpf Wumpf changed the title ClearEntity Archetype ClearEntity Archetype & migration Aug 17, 2023
@Wumpf
Copy link
Member Author

Wumpf commented Aug 17, 2023

internal slack thread on the topic https://rerunio.slack.com/archives/C033K5VS2KD/p1689179014868759

@emilk
Copy link
Member

emilk commented Aug 28, 2023

I guess it needs flag to inform if it is recursive or not

@teh-cmc teh-cmc self-assigned this Sep 8, 2023
teh-cmc added a commit that referenced this issue Sep 12, 2023
The clear APIs insert multiple rows worth of data (1 per component) that
all re-use the same `RowId` (the `RowId` that was used to do the
log_clear() call itself).
This leads to a flaky ordering of these rows internally, as there is no
way to tie-break between them.
It is also less performant than it should be as all of this data could
and should live on the same row.

The problem is accentuated when doing recursive clears, since in that
case the row ID is not only shared between multiple rows of the same
entity, but also multiple rows across multiple entities: even more
ordering flakiness.

This flaky ordering then leaks through the public APIs (e.g. range
queries) and, importantly, through roundtrip tests.
This is how I first encoutered this issue: #3023 is blocked by this
because the roundtrip tests for recursive clears are flaky. This PR
fixes that.

Unfortunately we don't have a test suite in place for clear APIs
(AFAIK), and I'm not even sure what the expected behavior is in all
cases, so I cannot write that test suite for now. I've opened an issue
for this:
- #3287 

In the meantime, the roundtrip tests introduced in #3023 will act as a
regression test at the very least.


### What

### 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 [demo.rerun.io](https://demo.rerun.io/pr/3288) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/3288)
- [Docs
preview](https://rerun.io/preview/7faa657db390c43112be6366b8c125f1a30a512c/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/7faa657db390c43112be6366b8c125f1a30a512c/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://ref.rerun.io/dev/bench/)
- [Wasm size tracking](https://ref.rerun.io/dev/sizes/)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌊 C++ API C/C++ API specific 🍏 primitives Relating to Rerun primitives 🐍 Python API Python logging API 🦀 Rust API Rust logging API
Projects
None yet
Development

No branches or pull requests

4 participants