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

release-20.2: catalogkv: use in-memory descriptors to validate in GetAllDescriptors #57574

Merged
merged 2 commits into from Dec 4, 2020

Conversation

ajwerner
Copy link
Contributor

@ajwerner ajwerner commented Dec 4, 2020

Backport 2/2 commits from #57542.

/cc @cockroachdb/release


In 20.2 we added logic to validate descriptors on the read path for getting
all descriptors and introduced a performance regression. In particular, we'd
now retrieve all databases and cross references for all tables. This stricter
validation has proven to be somewhat handy and has become utilized in some
tests to ensure that descriptors indeed are valid. Eliminating this validation
would thus be problematic. This is fortunately easy to resolve as we already
have all of the descriptors sitting in memory. With this change, we use them.

Fixes #57249.

In the previously added benchmark tracking the number of KV calls, the code
used to perform 32 reads and now perform 1 (the 32 mostly has to do with
resolving the system database when validating system tables).

Release note (bug fix): Fixed performance regression introduced in v20.2 to
reading virtual tables which introspect the schema.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@thoszhang thoszhang left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r1, 2 of 2 files at r2.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained

In 20.2 we added logic to validate descriptors on the read path for getting
all descriptors and introduced a performance regression. In particular, we'd
now retrieve all databases and cross references for all tables. This stricter
validation has proven to be somewhat handy and has become utilized in some
tests to ensure that descriptors indeed are valid. Eliminating this validation
would thus be problematic. This is fortunately easy to resolve as we already
have all of the descriptors sitting in memory. With this change, we use them.

Fixes cockroachdb#57249.

In the previously added benchmark tracking the number of KV calls, the code
used to perform 32 reads and now perform 1 (the 32 mostly has to do with
resolving the system database when validating system tables).

Release note (bug fix): Fixed performance regression introduced in v20.2 to
reading virtual tables which introspect the schema.
@ajwerner ajwerner merged commit 4fced82 into cockroachdb:release-20.2 Dec 4, 2020
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.

None yet

3 participants