Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
109337: asim: extract history, assertion into their own packages r=kvoli a=wenyihu6

**asim: extract history into its own package**

Previously, the history struct (capturing store metrics at each tick and
simulator’s current state) resided within the asim package.

Another PR(#109316) is introducing assertion registration which enables
scheduled assertion checks to run as delayed events. Since these assertion
events use simulation’s history to conduct assertion checks, the event package
would have to depend on the asim package. However, the asim package would also
have to depend on the event package’s event executor for event ticking, creating
a circular dependency.

To address this issue, this patch moves the history component out of the asim
package to its own package.

Note that this commit does not change any existing behavior, and the main
purpose is to make future commits cleaner.

See also: #109316
Part of: #106192
Release note: none

----

**asim: extract assertions into its own package**

Previously, simulation assertions resided within the tests package.

Another PR(#109316) is introducing assertion registration which enables
scheduled assertion checks to run as delayed events. Since these assertion
events require the assertion structs to conduct assertion checks, the event
package would have to depend on the test package. However, the test package
would also have to depend on the event package’s exported struct to initialize
structures for event generation, creating a circular dependency.

To address this issue, this patch moves the assertion component out of the test
package to its own package.

Note that this commit does not change any existing behavior, and the main
purpose is to make future commits cleaner.

See also: #109316
Part of: #106192
Release note: none

----

**asim: refactor liveness parsing to be handled by scanArg**

Previously, the test code directly parsed the string to derive a
`livenessStatus`. To make this cleaner, this patch delegates string parsing and
creation of `livenessStatus` to `scanArg`.

Note that this commit does not change any existing behavior, and the main
purpose is to make future commits cleaner.

Epic: none
Release note: none

109611: sql,schemachanger: disallow dropping indexes when referenced in UDF/View r=Xiang-Gu a=Xiang-Gu

This PR disallows schema changes (in both legacy and declarative schema changers) that would drop indexes referenced explicitly via index hinting in UDF or view. They include `DROP INDEX`, `ADD COLUMN`, `DROP COLUMN`, and `ALTER PRIMARY KEY`.

Fixes #108974
Epic: None
Release note: None

Co-authored-by: wenyihu6 <[email protected]>
Co-authored-by: Xiang Gu <[email protected]>
  • Loading branch information
3 people committed Aug 29, 2023
3 parents 3e9b7de + 430511c + dcb2acf commit c2c39fa
Show file tree
Hide file tree
Showing 28 changed files with 426 additions and 246 deletions.
2 changes: 2 additions & 0 deletions pkg/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -1300,11 +1300,13 @@ GO_TARGETS = [
"//pkg/kv/kvserver/allocator:allocator",
"//pkg/kv/kvserver/apply:apply",
"//pkg/kv/kvserver/apply:apply_test",
"//pkg/kv/kvserver/asim/assertion:assertion",
"//pkg/kv/kvserver/asim/config:config",
"//pkg/kv/kvserver/asim/event:event",
"//pkg/kv/kvserver/asim/gen:gen",
"//pkg/kv/kvserver/asim/gossip:gossip",
"//pkg/kv/kvserver/asim/gossip:gossip_test",
"//pkg/kv/kvserver/asim/history:history",
"//pkg/kv/kvserver/asim/metrics:metrics",
"//pkg/kv/kvserver/asim/metrics:metrics_test",
"//pkg/kv/kvserver/asim/op:op",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,13 +129,13 @@ exec-sql
ALTER TABLE sc1.tbl1 RENAME TO tbl1_new
----
pq: cannot rename relation "sc1.tbl1" because function "f1" depends on it
HINT: you can drop f1 instead.
HINT: consider dropping "f1" first.

exec-sql
ALTER TABLE sc1.tbl1 SET SCHEMA sc2;
----
pq: cannot set schema on relation "tbl1" because function "f1" depends on it
HINT: you can drop f1 instead.
HINT: consider dropping "f1" first.

exec-sql
DROP TYPE sc1.enum1
Expand Down Expand Up @@ -279,13 +279,13 @@ exec-sql
ALTER TABLE sc1.tbl1 RENAME TO tbl1_new
----
pq: cannot rename relation "sc1.tbl1" because function "f1" depends on it
HINT: you can drop f1 instead.
HINT: consider dropping "f1" first.

exec-sql
ALTER TABLE sc1.tbl1 SET SCHEMA sc2;
----
pq: cannot set schema on relation "tbl1" because function "f1" depends on it
HINT: you can drop f1 instead.
HINT: consider dropping "f1" first.

exec-sql
DROP TYPE sc1.enum1
Expand Down
2 changes: 2 additions & 0 deletions pkg/kv/kvserver/asim/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ go_library(
"//pkg/kv/kvserver/asim/config",
"//pkg/kv/kvserver/asim/event",
"//pkg/kv/kvserver/asim/gossip",
"//pkg/kv/kvserver/asim/history",
"//pkg/kv/kvserver/asim/metrics",
"//pkg/kv/kvserver/asim/op",
"//pkg/kv/kvserver/asim/queue",
Expand All @@ -26,6 +27,7 @@ go_test(
deps = [
":asim",
"//pkg/kv/kvserver/asim/config",
"//pkg/kv/kvserver/asim/history",
"//pkg/kv/kvserver/asim/metrics",
"//pkg/kv/kvserver/asim/state",
"//pkg/kv/kvserver/asim/workload",
Expand Down
20 changes: 4 additions & 16 deletions pkg/kv/kvserver/asim/asim.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/asim/config"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/asim/event"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/asim/gossip"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/asim/history"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/asim/metrics"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/asim/op"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/asim/queue"
Expand Down Expand Up @@ -60,7 +61,7 @@ type Simulator struct {
settings *config.SimulationSettings

metrics *metrics.Tracker
history History
history history.History
}

func (s *Simulator) Curr() time.Time {
Expand All @@ -71,19 +72,6 @@ func (s *Simulator) State() state.State {
return s.state
}

// History contains recorded information that summarizes a simulation run.
// Currently it only contains the store metrics of the run.
// TODO(kvoli): Add a range log like structure to the history.
type History struct {
Recorded [][]metrics.StoreMetrics
S state.State
}

// Listen implements the metrics.StoreMetricListener interface.
func (h *History) Listen(ctx context.Context, sms []metrics.StoreMetrics) {
h.Recorded = append(h.Recorded, sms)
}

// NewSimulator constructs a valid Simulator.
func NewSimulator(
duration time.Duration,
Expand Down Expand Up @@ -118,7 +106,7 @@ func NewSimulator(
shuffler: state.NewShuffler(settings.Seed),
// TODO(kvoli): Keeping the state around is a bit hacky, find a better
// method of reporting the ranges.
history: History{Recorded: [][]metrics.StoreMetrics{}, S: initialState},
history: history.History{Recorded: [][]metrics.StoreMetrics{}, S: initialState},
events: events,
settings: settings,
}
Expand Down Expand Up @@ -190,7 +178,7 @@ func (s *Simulator) GetNextTickTime() (done bool, tick time.Time) {

// History returns the current recorded history of a simulation run. Calling
// this on a Simulator that has not begun will return an empty history.
func (s *Simulator) History() History {
func (s *Simulator) History() history.History {
return s.history
}

Expand Down
3 changes: 2 additions & 1 deletion pkg/kv/kvserver/asim/asim_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (

"github.com/cockroachdb/cockroach/pkg/kv/kvserver/asim"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/asim/config"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/asim/history"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/asim/metrics"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/asim/state"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/asim/workload"
Expand Down Expand Up @@ -57,7 +58,7 @@ func TestAsimDeterministic(t *testing.T) {
// be larger than 3 keys per range.
keyspace := 3 * ranges
// Track the run to compare against for determinism.
var refRun asim.History
var refRun history.History

for run := 0; run < runs; run++ {
rwg := make([]workload.Generator, 1)
Expand Down
16 changes: 16 additions & 0 deletions pkg/kv/kvserver/asim/assertion/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")

go_library(
name = "assertion",
srcs = ["assert.go"],
importpath = "github.com/cockroachdb/cockroach/pkg/kv/kvserver/asim/assertion",
visibility = ["//visibility:public"],
deps = [
"//pkg/kv/kvserver/asim/history",
"//pkg/kv/kvserver/asim/metrics",
"//pkg/roachpb",
"//pkg/spanconfig/spanconfigtestutils",
"//pkg/util/log",
"@com_github_montanaflynn_stats//:stats",
],
)
Loading

0 comments on commit c2c39fa

Please sign in to comment.