Skip to content

Commit

Permalink
Merge #99368 #100313 #100596
Browse files Browse the repository at this point in the history 
99368: workload/schemachanger: basic declarative schema changer support r=fqazi a=fqazi

Previously, the randomized testing only supported, the non-declarative schema changer. This patch adds support for basic single statement transactions for testing the declarative schema changer

Note: This entablement is only focused on very
minimal stability testing. Not implemented errors
are always ignore for now.

Fixes: #77376

Release note: None

100313: bazel: update toolchains to support remote execution r=rail a=rickystewart

We need to add some extra files to `compiler_files` and `linker_files` and update some compiler flags to make sure remote execution works.

Epic: none
Release note: None

100596: testserver: fix version gate for tenant capabilities r=rafiss a=rafiss

This was revealed by mixed version testing, and it can flake sometimes.

Epic: None
Release note: None

Co-authored-by: Faizan Qazi <faizan@cockroachlabs.com>
Co-authored-by: Ricky Stewart <rickybstewart@gmail.com>
Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>
  • Loading branch information
@fqazi @rickystewart @rafiss
4 people committed Apr 4, 2023
4 parents 7b3f394 + 068356a + b08d498 + 5938fca commit 44f3f31
Show file tree
Hide file tree
Showing 6 changed files with 186 additions and 59 deletions.
27 changes: 25 additions & 2 deletions build/toolchains/crosstool-ng/BUILD.tmpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,18 @@ filegroup(
srcs = [
"bin/%{target}-gcc",
"bin/%{target}-g++",
],
"libexec/gcc/%{target}/6.5.0/cc1",
"libexec/gcc/%{target}/6.5.0/cc1plus",
] + glob([
"%{target}/bin/*",
"%{target}/include/c++/6.5.0/**",
"%{target}/sysroot/mingw/include/**",
"%{target}/sysroot/mingw/lib/**",
"%{target}/sysroot/mingw/lib32/**",
"%{target}/sysroot/usr/include/**",
"lib/gcc/%{target}/6.5.0/include/**",
"lib/gcc/%{target}/6.5.0/include-fixed/**",
])
)

filegroup(
Expand All @@ -43,7 +54,19 @@ filegroup(
name = "linker_files",
srcs = [
"bin/%{target}-g++",
],
] + glob([
"%{target}/sysroot/lib/**",
"%{target}/sysroot/lib64/**",
"%{target}/sysroot/usr/%{target}/lib32/**",
"%{target}/sysroot/usr/%{target}/lib/**",
"%{target}/sysroot/usr/lib/**",
"%{target}/sysroot/usr/lib64/**",
]) + glob(
["lib/gcc/%{target}/6.5.0/**"],
exclude=[
"lib/gcc/%{target}/6.5.0/include/**",
"lib/gcc/%{target}/6.5.0/include-fixed/**",
]),
)

filegroup(
Expand Down
16 changes: 8 additions & 8 deletions build/toolchains/crosstool-ng/cc_toolchain_config.bzl.tmpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ def _impl(ctx):
action_name = ACTION_NAMES.cpp_link_static_library,
tools = [tool(path="bin/%{target}-ar")],
),

]

opt_feature = feature(
Expand Down Expand Up @@ -106,7 +105,6 @@ def _impl(ctx):
],
)


default_compile_flags = feature(
name = "default_compile_flags",
enabled = True,
Expand All @@ -117,15 +115,17 @@ def _impl(ctx):
flag_group(
flags = [
"-Wall",
"-I%{repo_path}/%{target}/include/c++/6.5.0",
"-Iexternal/%{repo_name}/%{target}/include/c++/6.5.0",
"-no-canonical-prefixes",
"-fno-canonical-system-headers",
],
),
]),
),
],
)

linker_flags = []
linker_flags = ["-fno-use-linker-plugin"]
if "%{target}" == "x86_64-w64-mingw32":
linker_flags.append("-static")
if "-linux-" in "%{target}":
Expand Down Expand Up @@ -178,11 +178,11 @@ def _impl(ctx):
action_configs = action_configs,
cxx_builtin_include_directories = [
"%sysroot%/usr/include",
"%{repo_path}/%{target}/include/c++/6.5.0",
"%{repo_path}/lib/gcc/%{target}/6.5.0/include",
"%{repo_path}/lib/gcc/%{target}/6.5.0/include-fixed",
"external/%{repo_name}/%{target}/include/c++/6.5.0",
"external/%{repo_name}/lib/gcc/%{target}/6.5.0/include",
"external/%{repo_name}/lib/gcc/%{target}/6.5.0/include-fixed",
],
builtin_sysroot = "%{repo_path}/%{target}/sysroot",
builtin_sysroot = "external/%{repo_name}/%{target}/sysroot",
)

cc_toolchain_config = rule(
Expand Down
4 changes: 1 addition & 3 deletions build/toolchains/crosstool-ng/toolchain.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ def _impl(rctx):
stripPrefix = "x-tools/{}/".format(rctx.attr.target),
)

repo_path = str(rctx.path(""))

rctx.template(
"BUILD",
Label("@com_github_cockroachdb_cockroach//build:toolchains/crosstool-ng/BUILD.tmpl"),
Expand All @@ -26,7 +24,7 @@ def _impl(rctx):
substitutions = {
"%{target}": rctx.attr.target,
"%{host}": rctx.attr.host,
"%{repo_path}": repo_path,
"%{repo_name}": rctx.name,
},
executable = False,
)
Expand Down
2 changes: 1 addition & 1 deletion pkg/server/testserver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1083,7 +1083,7 @@ func (ts *TestServer) StartTenant(
baseCfg.DisableTLSForHTTP = params.DisableTLSForHTTP
baseCfg.EnableDemoLoginEndpoint = params.EnableDemoLoginEndpoint

if st.Version.IsActive(ctx, clusterversion.V23_1TenantCapabilities) {
if ts.ClusterSettings().Version.IsActive(ctx, clusterversion.V23_1TenantCapabilities) {
_, err := ie.Exec(ctx, "testserver-alter-tenant-cap", nil,
"ALTER TENANT [$1] GRANT CAPABILITY can_use_nodelocal_storage", params.TenantID.ToUint64())
if err != nil {
Expand Down
99 changes: 81 additions & 18 deletions pkg/workload/schemachange/operation_generator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ type operationGeneratorParams struct {
enumPct int
rng *rand.Rand
ops *deck
declarativeOps *deck
maxSourceTables int
sequenceOwnedByPct int
fkParentInvalidPct int
Expand Down Expand Up @@ -74,6 +75,9 @@ type operationGenerator struct {

// opGenLog log of statement used to generate the current statement.
opGenLog []interface{}

//useDeclarativeSchemaChanger indices if the declarative schema changer is used.
useDeclarativeSchemaChanger bool
}

// OpGenLogQuery a query with a single value result.
Expand Down Expand Up @@ -131,8 +135,9 @@ func makeOperationGenerator(params *operationGeneratorParams) *operationGenerato
}

// Reset internal state used per operation within a transaction
func (og *operationGenerator) resetOpState() {
func (og *operationGenerator) resetOpState(useDeclarativeSchemaChanger bool) {
og.candidateExpectedCommitErrors.reset()
og.useDeclarativeSchemaChanger = useDeclarativeSchemaChanger
}

// Reset internal state used per transaction
Expand Down Expand Up @@ -282,6 +287,45 @@ var opWeights = []int{
validate: 2, // validate twice more often
}

var opDeclarative = []bool{
addColumn: true,
addConstraint: false,
addForeignKeyConstraint: true,
addRegion: false,
addUniqueConstraint: true,
alterTableLocality: false,
createIndex: true,
createSequence: false,
createTable: false,
createTableAs: false,
createView: false,
createEnum: false,
createSchema: false,
dropColumn: true,
dropColumnDefault: true,
dropColumnNotNull: true,
dropColumnStored: true,
dropConstraint: true,
dropIndex: true,
dropSequence: true,
dropTable: true,
dropView: true,
dropSchema: true,
primaryRegion: false,
renameColumn: false,
renameIndex: false,
renameSequence: false,
renameTable: false,
renameView: false,
setColumnDefault: false,
setColumnNotNull: false,
setColumnType: false,
survive: false,
insertRow: false,
selectStmt: false,
validate: false,
}

// adjustOpWeightsForActiveVersion adjusts the weights for the active cockroach
// version, allowing us to disable certain operations in mixed version scenarios.
func adjustOpWeightsForCockroachVersion(
Expand Down Expand Up @@ -311,10 +355,18 @@ func adjustOpWeightsForCockroachVersion(
// change constructed. Constructing a random schema change may require a few
// stochastic attempts and if verbosity is >= 2 the unsuccessful attempts are
// recorded in `log` to help with debugging of the workload.
func (og *operationGenerator) randOp(ctx context.Context, tx pgx.Tx) (stmt *opStmt, err error) {
func (og *operationGenerator) randOp(
ctx context.Context, tx pgx.Tx, useDeclarativeSchemaChanger bool,
) (stmt *opStmt, err error) {
for {
op := opType(og.params.ops.Int())
og.resetOpState()
var op opType
// The declarative schema changer has a more limited deck of operations.
if useDeclarativeSchemaChanger {
op = opType(og.params.declarativeOps.Int())
} else {
op = opType(og.params.ops.Int())
}
og.resetOpState(useDeclarativeSchemaChanger)
stmt, err = opFuncs[op](og, ctx, tx)
if err != nil {
if errors.Is(err, pgx.ErrNoRows) {
Expand Down Expand Up @@ -2700,13 +2752,14 @@ func makeOpStmt(queryType opStmtType) *opStmt {
// ErrorState wraps schemachange workload errors to have state information for
// the purpose of dumping in our JSON log.
type ErrorState struct {
cause error
ExpectedErrors []string `json:"expectedErrors,omitempty"`
PotentialErrors []string `json:"potentialErrors,omitempty"`
ExpectedCommitErrors []string `json:"expectedCommitErrors,omitempty"`
PotentialCommitErrors []string `json:"potentialCommitErrors,omitempty"`
QueriesForGeneratingErrors []interface{} `json:"queriesForGeneratingErrors,omitempty"`
PreviousStatements []string `json:"previousStatements,omitempty"`
cause error
ExpectedErrors []string `json:"expectedErrors,omitempty"`
PotentialErrors []string `json:"potentialErrors,omitempty"`
ExpectedCommitErrors []string `json:"expectedCommitErrors,omitempty"`
PotentialCommitErrors []string `json:"potentialCommitErrors,omitempty"`
QueriesForGeneratingErrors []interface{} `json:"queriesForGeneratingErrors,omitempty"`
PreviousStatements []string `json:"previousStatements,omitempty"`
UsesDeclarativeSchemaChanger bool `json:"usesDeclarativeSchemaChanger,omitempty"`
}

func (es *ErrorState) Unwrap() error {
Expand All @@ -2724,13 +2777,14 @@ func (og *operationGenerator) WrapWithErrorState(err error, op *opStmt) error {
previousStmts = append(previousStmts, stmt.sql)
}
return &ErrorState{
cause: err,
ExpectedErrors: op.expectedExecErrors.StringSlice(),
PotentialErrors: op.potentialExecErrors.StringSlice(),
ExpectedCommitErrors: og.expectedCommitErrors.StringSlice(),
PotentialCommitErrors: og.potentialCommitErrors.StringSlice(),
QueriesForGeneratingErrors: og.GetOpGenLog(),
PreviousStatements: previousStmts,
cause: err,
ExpectedErrors: op.expectedExecErrors.StringSlice(),
PotentialErrors: op.potentialExecErrors.StringSlice(),
ExpectedCommitErrors: og.expectedCommitErrors.StringSlice(),
PotentialCommitErrors: og.potentialCommitErrors.StringSlice(),
QueriesForGeneratingErrors: og.GetOpGenLog(),
PreviousStatements: previousStmts,
UsesDeclarativeSchemaChanger: og.useDeclarativeSchemaChanger,
}
}

Expand Down Expand Up @@ -2758,6 +2812,15 @@ func (s *opStmt) executeStmt(ctx context.Context, tx pgx.Tx, og *operationGenera
if pgcode.MakeCode(pgErr.Code) == pgcode.SerializationFailure {
return err
}
// TODO(fqazi): For the short term we are going to ignore any not implemented,
// errors in the declarative schema changer. Supported operations have edge
// cases, but later we should mark some of these are fully supported.
if og.useDeclarativeSchemaChanger && pgcode.MakeCode(pgErr.Code) == pgcode.Uncategorized &&
strings.Contains(pgErr.Message, " not implemented in the new schema changer") {
return errors.Mark(errors.Wrap(err, "ROLLBACK; Ignoring declarative schema changer not implemented error."),
errRunInTxnRbkSentinel,
)
}
if !s.expectedExecErrors.contains(pgcode.MakeCode(pgErr.Code)) &&
!s.potentialExecErrors.contains(pgcode.MakeCode(pgErr.Code)) {
return errors.Mark(
Expand Down
Loading

0 comments on commit 44f3f31

Please sign in to comment.