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

Update all involved allocators before performing queries over links #4415

Closed
wants to merge 1 commit into from

Conversation

tgoyne
Copy link
Member

@tgoyne tgoyne commented Feb 11, 2021

This is an attempt at a more general solution to the same problem as f045da0. There are several places where queries are using the allocator from one Table while accessing another Table. Rather than attempting to validate that all of them are correct, instead update each Table's WrappedAllocator before running the query so that it doesn't matter which one is used.

AFAICT this is a perfectly safe thing to do, but it's possible I've misunderstood something. It appears that not updating each WrappedAllocator immediately is a perf optimization and not something required for correctness (or maybe there just isn't an easy way to do it?), and this should be thread-safe for frozen transactions as it's assigning to an atomic variable with proper sequencing.

This is an attempt at a more general solution to the same problem as f045da0.
There are several places where queries are using the allocator from one Table
while accessing another Table. Rather than attempting to validate that all of
them are correct, instead update each Table's WrappedAllocator before running
the query so that it doesn't matter which one is used.
@jedelbo
Copy link
Contributor

jedelbo commented Feb 11, 2021

I created #4417 as an alternative solution. I will let @finnschiermer rule.

@finnschiermer
Copy link
Contributor

I prefer #4417 since it maintains the relation between Table and it's allocator. I'm afraid deviating from this opens up for wrong assumptions and new errors down the line if we add functionality to the WrappedAllocator - or access the version counters in a new setting.

@tgoyne tgoyne closed this Feb 11, 2021
@tgoyne tgoyne deleted the tg/query-alloc-assert branch February 11, 2021 16:54
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants