Skip to content

re_datastore: sanity checking cluster keys and their data#650

Merged
teh-cmc merged 147 commits intomainfrom
cmc/datastore/cluster_key_sanity
Jan 2, 2023
Merged

re_datastore: sanity checking cluster keys and their data#650
teh-cmc merged 147 commits intomainfrom
cmc/datastore/cluster_key_sanity

Conversation

@teh-cmc
Copy link
Copy Markdown
Contributor

@teh-cmc teh-cmc commented Dec 27, 2022

Closes #648
Requires #609 (to keep things linear, don't want to have to deal with a multiverse situation...)

Comment on lines +499 to +500
#[cfg(debug_assertions)]
cluster_key: _,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

wat even is this

@teh-cmc teh-cmc marked this pull request as ready for review December 27, 2022 09:23
@teh-cmc teh-cmc linked an issue Dec 27, 2022 that may be closed by this pull request
/// Carrying the cluster key around in debug builds to help with assertions and sanity checks
/// all over the place.
#[cfg(debug_assertions)]
pub(crate) cluster_key: ComponentName,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There was at least once place in the upstream-PR where IndexBuckets knowing the cluster_key would be helpful -- maybe this doesn't need to be debug-gated?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It turned out not be useful in that other PR after all... then again, now that ComponentNames are actually interned under the hood, there's really not that much reason not to store them in each and every bucket.

Base automatically changed from cmc/datastore/range_queries2 to main January 2, 2023 16:28
@teh-cmc teh-cmc merged commit f4c7d43 into main Jan 2, 2023
@teh-cmc teh-cmc deleted the cmc/datastore/cluster_key_sanity branch January 2, 2023 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

re_datastore: sanity_check should check for presence of cluster key

2 participants