From b38ff1c77c9b1e76609601b8c9c2835e71e25cbd Mon Sep 17 00:00:00 2001 From: Xiang Gu Date: Wed, 31 Aug 2022 13:05:02 -0400 Subject: [PATCH] rules: suppress a dep-rule in special case Release justification: fixed a release blocker bug. Previously, in a mixed version state (v22.1 and v22.2), if a old node drops a rowLevelTTL table and the new shcema changer job was adopted by a new node, then there is a dep rule that cannot satisfied and hence causes forward incompatibility. This PR fixes this by suppressing this dep rule if this particular case happens (namely, it suppress the rule if `from` is Table element, `to` is a RowLevelTTL element, and there is no `PUBLIC` status of the table element anywhere in the graph). Fixes: #86672 Release note: None --- pkg/sql/schemachanger/rel/schema_rules.go | 5 ++++ .../scplan/internal/rules/dep_drop_object.go | 1 + .../scplan/internal/rules/helpers.go | 27 +++++++++++++++++++ .../scplan/internal/rules/testdata/deprules | 12 +++++++++ .../scplan/internal/rules/testdata/oprules | 11 ++++++++ 5 files changed, 56 insertions(+) diff --git a/pkg/sql/schemachanger/rel/schema_rules.go b/pkg/sql/schemachanger/rel/schema_rules.go index a1ad2b2dde9f..cd2edb1b96de 100644 --- a/pkg/sql/schemachanger/rel/schema_rules.go +++ b/pkg/sql/schemachanger/rel/schema_rules.go @@ -95,6 +95,11 @@ func (sc *Schema) Def3(name string, a, b, c Var, def func(a, b, c Var) Clauses) return sc.rule(name, regular, def, a, b, c).(Rule3) } +// DefNotJoin3 defines a not-join rule with three bound variable arguments. +func (sc *Schema) DefNotJoin3(name string, a, b, c Var, def func(a, b, c Var) Clauses) Rule3 { + return sc.rule(name, notJoin, def, a, b, c).(Rule3) +} + // Def4 defines a Rule4. func (sc *Schema) Def4(name string, a, b, c, d Var, def func(a, b, c, d Var) Clauses) Rule4 { return sc.rule(name, regular, def, a, b, c, d).(Rule4) diff --git a/pkg/sql/schemachanger/scplan/internal/rules/dep_drop_object.go b/pkg/sql/schemachanger/scplan/internal/rules/dep_drop_object.go index 71218ce6c5cb..26d36428a2a6 100644 --- a/pkg/sql/schemachanger/scplan/internal/rules/dep_drop_object.go +++ b/pkg/sql/schemachanger/scplan/internal/rules/dep_drop_object.go @@ -67,6 +67,7 @@ func init() { to.typeFilter(isSimpleDependent), joinOnDescID(from, to, "desc-id"), statusesToAbsent(from, scpb.Status_DROPPED, to, scpb.Status_ABSENT), + fromHasPublicStatusIfFromIsTableAndToIsRowLevelTTL(from.target, from.el, to.el), } }) diff --git a/pkg/sql/schemachanger/scplan/internal/rules/helpers.go b/pkg/sql/schemachanger/scplan/internal/rules/helpers.go index 7bb690c66237..84c51b1bd7e9 100644 --- a/pkg/sql/schemachanger/scplan/internal/rules/helpers.go +++ b/pkg/sql/schemachanger/scplan/internal/rules/helpers.go @@ -486,3 +486,30 @@ var descriptorIsNotBeingDropped = screl.Schema.DefNotJoin1( } }, ) + +// fromHasPublicStatusIfFromIsTableAndToIsRowLevelTTL creates +// a clause which leads to the outer clause failing to unify +// if the passed element `from` is a Table, `to` is a RowLevelTTl, +// and there does not exist a node with the same target as +// `fromTarget` in PUBLIC status. +// It is used to suppress rule "descriptor drop right before dependent element removal" +// for the special case where we drop a rowLevelTTL table in mixed +// version state for forward compatibility (issue #86672). +var fromHasPublicStatusIfFromIsTableAndToIsRowLevelTTL = screl.Schema.DefNotJoin3( + "fromHasPublicStatusIfFromIsTableAndToIsRowLevelTTL", + "fromTarget", "fromEl", "toEl", func(fromTarget, fromEl, toEl rel.Var) rel.Clauses { + n := rel.Var("n") + return rel.Clauses{ + fromEl.Type((*scpb.Table)(nil)), + toEl.Type((*scpb.RowLevelTTL)(nil)), + n.Type((*screl.Node)(nil)), + n.AttrEqVar(screl.Target, fromTarget), + screl.Schema.DefNotJoin1("nodeHasNoPublicStatus", "n", func(n rel.Var) rel.Clauses { + public := rel.Var("public") + return rel.Clauses{ + public.Eq(scpb.Status_PUBLIC), + n.AttrEqVar(screl.CurrentStatus, public), + } + })(n), + } + }) diff --git a/pkg/sql/schemachanger/scplan/internal/rules/testdata/deprules b/pkg/sql/schemachanger/scplan/internal/rules/testdata/deprules index 5ca9da05c0f8..76b3c18abd65 100644 --- a/pkg/sql/schemachanger/scplan/internal/rules/testdata/deprules +++ b/pkg/sql/schemachanger/scplan/internal/rules/testdata/deprules @@ -41,6 +41,10 @@ joinTargetNode($element, $target, $node): - joinTarget($element, $target) - $node[Type] = '*screl.Node' - $node[Target] = $target +node does not have a PUBLIC status($n): + not-join: + - $public = PUBLIC + - $n[CurrentStatus] = $public nodeNotExistsWithStatusIn_BACKFILLED_BACKFILL_ONLY($sharedTarget): not-join: - $n[Type] = '*screl.Node' @@ -83,6 +87,13 @@ nodeNotExistsWithStatusIn_VALIDATED_TRANSIENT_WRITE_ONLY_MERGE_ONLY_TRANSIENT_ME - $n[CurrentStatus] IN [VALIDATED, TRANSIENT_WRITE_ONLY, MERGE_ONLY, TRANSIENT_MERGE_ONLY, MERGED, TRANSIENT_MERGED] sourceIndexIsSet($index): - $index[SourceIndexID] != 0 +suppress rule if dropping rowLevelTTL tables in mixed version state($fromTarget, $fromEl, $toEl): + not-join: + - $fromEl[Type] = '*scpb.Table' + - $toEl[Type] = '*scpb.RowLevelTTL' + - $n[Type] = '*screl.Node' + - $n[Target] = $fromTarget + - node does not have a PUBLIC status($n) toAbsent($target1, $target2): - $target1[TargetStatus] = ABSENT - $target2[TargetStatus] = ABSENT @@ -1664,6 +1675,7 @@ deprules - toAbsent($descriptor-target, $dependent-target) - $descriptor-node[CurrentStatus] = DROPPED - $dependent-node[CurrentStatus] = ABSENT + - suppress rule if dropping rowLevelTTL tables in mixed version state($descriptor-target, $descriptor, $dependent) - joinTargetNode($descriptor, $descriptor-target, $descriptor-node) - joinTargetNode($dependent, $dependent-target, $dependent-node) - name: descriptor drop right before removing dependent with attr ref diff --git a/pkg/sql/schemachanger/scplan/internal/rules/testdata/oprules b/pkg/sql/schemachanger/scplan/internal/rules/testdata/oprules index 9131c112a2e2..8c2ebbd829eb 100644 --- a/pkg/sql/schemachanger/scplan/internal/rules/testdata/oprules +++ b/pkg/sql/schemachanger/scplan/internal/rules/testdata/oprules @@ -41,6 +41,10 @@ joinTargetNode($element, $target, $node): - joinTarget($element, $target) - $node[Type] = '*screl.Node' - $node[Target] = $target +node does not have a PUBLIC status($n): + not-join: + - $public = PUBLIC + - $n[CurrentStatus] = $public nodeNotExistsWithStatusIn_BACKFILLED_BACKFILL_ONLY($sharedTarget): not-join: - $n[Type] = '*screl.Node' @@ -83,6 +87,13 @@ nodeNotExistsWithStatusIn_VALIDATED_TRANSIENT_WRITE_ONLY_MERGE_ONLY_TRANSIENT_ME - $n[CurrentStatus] IN [VALIDATED, TRANSIENT_WRITE_ONLY, MERGE_ONLY, TRANSIENT_MERGE_ONLY, MERGED, TRANSIENT_MERGED] sourceIndexIsSet($index): - $index[SourceIndexID] != 0 +suppress rule if dropping rowLevelTTL tables in mixed version state($fromTarget, $fromEl, $toEl): + not-join: + - $fromEl[Type] = '*scpb.Table' + - $toEl[Type] = '*scpb.RowLevelTTL' + - $n[Type] = '*screl.Node' + - $n[Target] = $fromTarget + - node does not have a PUBLIC status($n) toAbsent($target1, $target2): - $target1[TargetStatus] = ABSENT - $target2[TargetStatus] = ABSENT