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

Prevent flaky roundtrips in rerun rrd compare #8648

Merged
merged 1 commit into from
Jan 13, 2025

Conversation

teh-cmc
Copy link
Member

@teh-cmc teh-cmc commented Jan 10, 2025

Store-side compaction has election heuristics that can yield different results depending on the order in which the chunks arrive.
This can make some cross-language roundtrip tests flaky.

This PR makes sure to disabled store-side compaction when running RRD comparison tests.

Copy link

github-actions bot commented Jan 10, 2025

Web viewer failed to build.

| Result | Commit | Link | Manifest |
| ------ | ------- | ----- |
| ❌ | | https://rerun.io/viewer/pr/8648 | +nightly +main |

Note: This comment is updated whenever you push a commit.

@teh-cmc teh-cmc added 🪳 bug Something isn't working 🔨 testing testing and benchmarks 🏹 arrow concerning arrow exclude from changelog PRs with this won't show up in CHANGELOG.md labels Jan 10, 2025
@teh-cmc teh-cmc marked this pull request as ready for review January 10, 2025 16:20
Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

I'm rather surprised that compaction isn't deterministic. Is the problem that two rrd files with differing message order should still yield the same store from the pov of a comparision, but the fact that data comes in different order changes compaction?

@teh-cmc teh-cmc force-pushed the cmc/eager_partial_4_partial_on_eager branch from a1331e1 to d5bc79c Compare January 13, 2025 08:36
Base automatically changed from cmc/eager_partial_4_partial_on_eager to main January 13, 2025 08:37
@teh-cmc teh-cmc force-pushed the cmc/eager_partial_5_flaky_roundtrips branch from bbe01a7 to a4852e9 Compare January 13, 2025 08:39
@teh-cmc
Copy link
Member Author

teh-cmc commented Jan 13, 2025

I'm rather surprised that compaction isn't deterministic. Is the problem that two rrd files with differing message order should still yield the same store from the pov of a comparision, but the fact that data comes in different order changes compaction?

Store-side compaction is currently done synchronously on the write path: when a Chunk arrives, we find the best possible candidate to merge that Chunk into. The result of this process can vary depending on the order the chunks arrive in.

@teh-cmc teh-cmc merged commit 822a192 into main Jan 13, 2025
26 checks passed
@teh-cmc teh-cmc deleted the cmc/eager_partial_5_flaky_roundtrips branch January 13, 2025 08:42
teh-cmc added a commit that referenced this pull request Jan 13, 2025
…8649)

This adds a partial update example for `Points3D`, in all languages,
using both the legacy and new APIs (if they already exist).

This acts as a before/after comparison, a piece of documentation and a
way to make sure that the legacy and new APIs behave similarly.

* DNM: requires #8648 
* Part of #8583 
* Part of #8581 
* Part of #8582
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏹 arrow concerning arrow 🪳 bug Something isn't working exclude from changelog PRs with this won't show up in CHANGELOG.md 🔨 testing testing and benchmarks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants