Skip to content

Commit

Permalink
rules: suppress a dep-rule in special case
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Xiang-Gu committed Sep 7, 2022
1 parent 676d4a9 commit b38ff1c
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 0 deletions.
5 changes: 5 additions & 0 deletions pkg/sql/schemachanger/rel/schema_rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
})

Expand Down
27 changes: 27 additions & 0 deletions pkg/sql/schemachanger/scplan/internal/rules/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
})
12 changes: 12 additions & 0 deletions pkg/sql/schemachanger/scplan/internal/rules/testdata/deprules
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions pkg/sql/schemachanger/scplan/internal/rules/testdata/oprules
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit b38ff1c

Please sign in to comment.