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

descs: don't invalidate kvDescriptors cache #84187

Closed
wants to merge 1 commit into from

Conversation

jasonmchan
Copy link
Contributor

Previously, the collection invalidated its view of descriptors read from
KV after a descriptor was modified. This was necessary because the
collection does not fully buffer writes to descriptors, so the
descriptors stored in KV may change. This is problematic, because users
of the kvDescriptors (virtual tables) must perform a round trip to
refresh their descriptors cache after schema changes in the same
transaction.

To address this, this commit avoids invalidating the cache. Instead,
when serving lookups that rely on the kvDescriptors, we will amend the
cache with the contents of uncommittedDescriptors to ensure that a
transaction sees its own modifications.

Relates to #64673

Release note: None

Previously, the collection invalidated its view of descriptors read from
KV after a descriptor was modified. This was necessary because the
collection does not fully buffer writes to descriptors, so the
descriptors stored in KV may change. This is problematic, because users
of the `kvDescriptors` (virtual tables) must perform a round trip to
refresh their descriptors cache after schema changes in the same
transaction.

To address this, this commit avoids invalidating the cache. Instead,
when serving lookups that rely on the `kvDescriptors`, we will amend the
cache with the contents of `uncommittedDescriptors` to ensure that a
transaction sees its own modifications.

Relates to cockroachdb#64673

Release note: None
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@jasonmchan jasonmchan marked this pull request as ready for review July 11, 2022 18:31
@jasonmchan jasonmchan requested a review from a team July 11, 2022 18:31
@postamar postamar requested a review from ajwerner July 12, 2022 11:20
@jasonmchan
Copy link
Contributor Author

Closing as this isn't the change we want.

@jasonmchan jasonmchan closed this Jul 14, 2022
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.

2 participants