Closed
Conversation
Member
|
This change is |
fqazi
force-pushed
the
enable-new-schema-changer
branch
2 times, most recently
from
e1a6920 to
2979797
Compare
fqazi
force-pushed
the
enable-new-schema-changer
branch
4 times, most recently
from
403781c to
033ae3c
Compare
fqazi
force-pushed
the
enable-new-schema-changer
branch
7 times, most recently
from
7b93cae to
4c4f7a1
Compare
Release note: None
Previously, the declarative schema changer fetched information about tables in the transaction for ALTER TABLE before checking if the commands were supported. This was problematic because it could change timing/retry behaviour in certain cases, since we would do potentially extra descriptor look ups. To address this, this patch checks if ALTER TABLE commands are supported first to avoid intermittent failures caused by extra round trips. Release note: None
Previously, the drop view errors generated by the declarative schema changer did not have fully resolved names when cross database references were involved. To be consistent with the legacy schema changer we should add fully resolved names when the parent databases differ. To address this, this patch will add fully resolved names on dependents if the parent databases differ. Release note: None
Previously, the declarative schema changer was disabled by default. We are enabling it by default, so some logic test updates are needed. This patch will update logictest expected files. Release note: None more rewriting... <pkg>: <short description - lowercase, no final period> <what was there before: Previously, ...> <why it needed to change: This was inadequate because ...> <what you did about it: To address this, this patch ...> Release note (<category, see below>): <what> <show> <why> disable <pkg>: <short description - lowercase, no final period> <what was there before: Previously, ...> <why it needed to change: This was inadequate because ...> <what you did about it: To address this, this patch ...> Release note (<category, see below>): <what> <show> <why>
Previously, the declarative schema changer did not return the correct error when attempting to modify pg_catalog tables. To address this, this patch will return the correct error when trying to modify pg_catalog tables. Release note: None
Previously, the declarative schema changer was enabled by default, which could cause certain tests designed for the legacy schema changer to fail. To address this, this patch individual disables declarative schema changer on tests designed for legacy schema changer. Release note: None
Previously, declarative schema changer did not clean up type references to multiregion enums when dropping tables. This was inadequate and could lead to validation errors due to dangling references. To address this, this patch will clean up multiregion type references in tables. Release note: None
Previously, the message generated for drop types that were not allowed due to dependencies was had a period. This patch will update the message to match the legacy schema changer. Release note: None
Previously, the drop database logic inside the declarative schema changer always assumed that a public schema would exist. However, due to corruption its possible for his entry to be missing. To address this, this patch will add extra logic to validate if the public schema is valid before proceeding to clean it up. Release note: None
Previously, the declarative schema changer did not generate the correct messages when attempting to drop/alter multiregion enums. To address this patch will update the message generated for multiregion enums to include a hint to indicate that ALTER DATABASE should be used for such modifications. Release note: None
Previously, the declarative schema changer did not support KV tracing. This was inadequate because the legacy schema changer supported this functionality which is needed for supportability of the product. To address this, this patch will add support for KV tracing inside the declarative schema changer. Release note: None
Previously, when dropping a database descriptor, the decalrative schema changer never deleted database descriptors properly. This led to the descriptors being left behind, instead of fully being cleaned up. To address, this patch will delete database descriptors during DROP DATABASE. Release note: None
Previously, the declarative schema changer did not populate the drop time when dropping tables. This could be problematic when looking at crdb_internal tables and other scenarios that detect if a table is dropped based on this time. To address this, this patch starts populating the DropTime inside the declarative schema changer. Release note: None merge into drop time pr <pkg>: <short description - lowercase, no final period> <what was there before: Previously, ...> <why it needed to change: This was inadequate because ...> <what you did about it: To address this, this patch ...> Release note (<category, see below>): <what> <show> <why> remove drop time from test <pkg>: <short description - lowercase, no final period> <what was there before: Previously, ...> <why it needed to change: This was inadequate because ...> <what you did about it: To address this, this patch ...> Release note (<category, see below>): <what> <show> <why>
Previously, the telemetry metrics were not properly tracked for drops inside the declarative schema changer. This was inadequate because we didn't have feature parity with the legacy schema changer. To address this, this patch adds support for tracking telemetry metrics inside the declarative schema changer. Release note: None.
fqazi
force-pushed
the
enable-new-schema-changer
branch
3 times, most recently
from
9df785a to
ed56a4e
Compare
Previously, there the enums logictest with the declarative schema changer code paths enabled could intermittently fail with retry errors. The design of the test is such that we execute SELECT queries and schema changes in the same transaction, so automatic retries are not possible once the result set has been sent to the client. To address this, this patch will workaround the issue by disabling declarative schema changer for this test. Release note: None
Previously, we were right at the timeout boundary for the TenantLogicTest. With the declarative schema changer enabled we are just past the 45 minute mark causing this test to fail. This patch bumps up the test timeout to 50 minutes to allow for breathing room. Release note: None
fqazi
force-pushed
the
enable-new-schema-changer
branch
from
ed56a4e to
f370b8f
Compare
craig bot
pushed a commit
that referenced
this pull request
Feb 24, 2022
76742: dev: don't exit immediately in ./dev ui watch r=irfansharif a=sjbarag ./dev ui watch previously didn't wait for the command context for webpack commands to complete before returning. This caused the 'watch' subcommand to stop immediately, then the 'ui' subcommand, then the entire 'dev' command to stop, exiting the entire process likely before webpack could even start. Wait for the webpack commands to complete (which they never should, since they're meant to run indefinitely) before returning from ./dev ui watch's implementation. Release note: None 76944: *: enable declarative schema changer r=postamar a=postamar This change enables the declarative schema changer by default. This PR originated as #76610 by `@fqazi` which I rebased following the merging of #76776. Only mild alterations were needed, these commits can therefore be considered as having been reviewed. Co-authored-by: Sean Barag <barag@cockroachlabs.com> Co-authored-by: Andrew Werner <awerner32@gmail.com> Co-authored-by: Faizan Qazi <faizan@cockroachlabs.com> Co-authored-by: Marius Posta <marius@cockroachlabs.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
No description provided.