sql: use leased database descriptors, implement db schema changes in job #52975
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
thoszhang
force-pushed
the
db-descs
branch
2 times, most recently
from
22a2e7f to
18d7987
Compare
thoszhang
force-pushed
the
db-descs
branch
2 times, most recently
from
a15e3ff to
ad1554e
Compare
rohany
suggested changes
Aug 19, 2020
thoszhang
force-pushed
the
db-descs
branch
2 times, most recently
from
7470508 to
71f782c
Compare
thoszhang
requested review from
miretskiy and
ajwerner
and removed request for
a team and
miretskiy
|
Still need to add the mixed-version logic tests but this is ready for a look. |
thoszhang
force-pushed
the
db-descs
branch
5 times, most recently
from
06e6f61 to
e622663
Compare
rohany
approved these changes
Aug 19, 2020
ajwerner
approved these changes
Aug 19, 2020
There was a problem hiding this comment.
What's up with the tenant tests?
Reviewed 5 of 10 files at r1, 2 of 55 files at r3, 65 of 65 files at r4.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @lucy-zhang and @rohany)
pkg/sql/catalog/descs/collection.go, line 418 at r4 (raw file):
// func (tc *Collection) GetDatabaseVersion() { // // }
?
thoszhang
commented
Aug 19, 2020
There was a problem hiding this comment.
What's up with the tenant tests?
I'm unskipping the 3node-tenant configuration for these tests, which will close #52567. I need to write a commit message.
Reviewable status:
pkg/sql/catalog/descs/collection.go, line 418 at r4 (raw file):
Previously, ajwerner wrote…
// func (tc *Collection) GetDatabaseVersion() { // // }?
Oops.
thoszhang
force-pushed
the
db-descs
branch
3 times, most recently
from
e9da118 to
9415d79
Compare
thoszhang
force-pushed
the
db-descs
branch
3 times, most recently
from
bfe5202 to
c47a3ea
Compare
This commit introduces the use of leased database descriptors, and implements database schema changes in the schema change job. The main changes: * The descriptor collection now provides access to database descriptors, and name resolution uses leased database descriptors where appropriate. * The resolver API now allows resolving mutable database descriptors, which we now fetch and use consistently in schema changes. * The former incoherent database cache (`database.Cache`) has been replaced, and database caching no longer relies on gossip. However: * The deprecated database cache and `uncommittedDatabases` set are still in use in the 20.1 mixed-version state, and the use of leased descriptors is version-gated. The cache itself is no longer exposed to users of the descriptor collection, and when reading descriptors, the choice of caching mechanism should be mostly transparent to the caller. * The schema change job now handles database schema changes (rename, drop, grant/revoke), which involves draining names, waiting for old leases to drain, etc. Release note (sql change, bug fix): Renaming and dropping databases now satisfy the same transactional guarantees as online schema changes on tables: Changes will become visible if and only if the transaction commits, and a successful result indicates changes having propagated to the entire cluster. This fixes some long-standing problems with inconsistent database state being visible after renames and drops. (As a side effect of coherent caching for databases, database names can no longer be recycled in multiple schema changes in the same transaction.)
|
bors r+ |
|
Build succeeded: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit introduces the use of leased database descriptors, and
implements database schema changes in the schema change job.
The main changes:
and name resolution uses leased database descriptors where
appropriate.
which we now fetch and use consistently in schema changes.
database.Cache) has beenreplaced, and database caching no longer relies on gossip. However:
uncommittedDatabasesset arestill in use in the 20.1 mixed-version state, and the use of leased
descriptors is version-gated. The cache itself is no longer exposed to
users of the descriptor collection, and when reading descriptors, the
choice of caching mechanism should be mostly transparent to the
caller.
drop, grant/revoke), which involves draining names, waiting for old
leases to drain, etc.
Closes #41942.
Closes #52567.
Release note (sql change, bug fix): Renaming and dropping databases now
satisfy the same transactional guarantees as online schema changes on
tables: Changes will become visible if and only if the transaction
commits, and a successful result indicates changes having propagated to
the entire cluster. This fixes some long-standing problems with
inconsistent database state being visible after renames and drops. (As a
side effect of coherent caching for databases, database names can no
longer be recycled in multiple schema changes in the same transaction.)