Skip to content

Commit

Permalink
asim: extract history into its own package
Browse files Browse the repository at this point in the history
Previously, the history struct (capturing store metrics at each tick and
simulator’s current state) resided within the asim package.

Another PR 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: cockroachdb#109316
Part of: cockroachdb#106192
Release note: none
  • Loading branch information
wenyihu6 committed Aug 28, 2023
1 parent 9eb73a8 commit 9b80425
Show file tree
Hide file tree
Showing 10 changed files with 66 additions and 29 deletions.
1 change: 1 addition & 0 deletions pkg/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -1305,6 +1305,7 @@ GO_TARGETS = [
"//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
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
12 changes: 12 additions & 0 deletions pkg/kv/kvserver/asim/history/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")

go_library(
name = "history",
srcs = ["history.go"],
importpath = "github.com/cockroachdb/cockroach/pkg/kv/kvserver/asim/history",
visibility = ["//visibility:public"],
deps = [
"//pkg/kv/kvserver/asim/metrics",
"//pkg/kv/kvserver/asim/state",
],
)
31 changes: 31 additions & 0 deletions pkg/kv/kvserver/asim/history/history.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright 2022 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package history

import (
"context"

"github.com/cockroachdb/cockroach/pkg/kv/kvserver/asim/metrics"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/asim/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)
}
4 changes: 2 additions & 2 deletions pkg/kv/kvserver/asim/tests/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ go_test(
embed = [":tests"],
deps = [
"//pkg/kv/kvserver/allocator/allocatorimpl",
"//pkg/kv/kvserver/asim",
"//pkg/kv/kvserver/asim/config",
"//pkg/kv/kvserver/asim/event",
"//pkg/kv/kvserver/asim/gen",
"//pkg/kv/kvserver/asim/history",
"//pkg/kv/kvserver/asim/metrics",
"//pkg/kv/kvserver/asim/state",
"//pkg/kv/kvserver/liveness/livenesspb",
Expand All @@ -41,10 +41,10 @@ go_library(
importpath = "github.com/cockroachdb/cockroach/pkg/kv/kvserver/asim/tests",
visibility = ["//visibility:public"],
deps = [
"//pkg/kv/kvserver/asim",
"//pkg/kv/kvserver/asim/config",
"//pkg/kv/kvserver/asim/event",
"//pkg/kv/kvserver/asim/gen",
"//pkg/kv/kvserver/asim/history",
"//pkg/kv/kvserver/asim/metrics",
"//pkg/kv/kvserver/asim/state",
"//pkg/roachpb",
Expand Down
14 changes: 8 additions & 6 deletions pkg/kv/kvserver/asim/tests/assert.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
"math"
"strings"

"github.com/cockroachdb/cockroach/pkg/kv/kvserver/asim"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/asim/history"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/asim/metrics"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/spanconfig/spanconfigtestutils"
Expand Down Expand Up @@ -83,7 +83,7 @@ type SimulationAssertion interface {
// Assert looks at a simulation run history and returns true if the
// assertion holds and false if not. When the assertion does not hold, the
// reason is also returned.
Assert(context.Context, asim.History) (holds bool, reason string)
Assert(context.Context, history.History) (holds bool, reason string)
// String returns the string representation of the assertion.
String() string
}
Expand All @@ -107,7 +107,7 @@ type steadyStateAssertion struct {
// assertion tick. If violated, holds is returned as false along with the
// reason.
func (sa steadyStateAssertion) Assert(
ctx context.Context, h asim.History,
ctx context.Context, h history.History,
) (holds bool, reason string) {
m := h.Recorded
ticks := len(m)
Expand Down Expand Up @@ -193,7 +193,9 @@ type balanceAssertion struct {
// stat's maximum/mean (over all stores) in the cluster meets the threshold
// constraint at each assertion tick. If violated, holds is returned as false
// along with the reason.
func (ba balanceAssertion) Assert(ctx context.Context, h asim.History) (holds bool, reason string) {
func (ba balanceAssertion) Assert(
ctx context.Context, h history.History,
) (holds bool, reason string) {
m := h.Recorded
ticks := len(m)
if ba.ticks > ticks {
Expand Down Expand Up @@ -255,7 +257,7 @@ type storeStatAssertion struct {
// assertion holds and false if not. When the assertion does not hold, the
// reason is also returned.
func (sa storeStatAssertion) Assert(
ctx context.Context, h asim.History,
ctx context.Context, h history.History,
) (holds bool, reason string) {
m := h.Recorded
ticks := len(m)
Expand Down Expand Up @@ -313,7 +315,7 @@ const conformanceAssertionSentinel = -1
// assertion holds and false if not. When the assertion does not hold, the
// reason is also returned.
func (ca conformanceAssertion) Assert(
ctx context.Context, h asim.History,
ctx context.Context, h history.History,
) (holds bool, reason string) {
report := h.S.Report()
buf := strings.Builder{}
Expand Down
4 changes: 2 additions & 2 deletions pkg/kv/kvserver/asim/tests/datadriven_simulation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ import (
"time"

"github.com/cockroachdb/cockroach/pkg/kv/kvserver/allocator/allocatorimpl"
"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/event"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/asim/gen"
"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/liveness/livenesspb"
Expand Down Expand Up @@ -171,7 +171,7 @@ func TestDataDriven(t *testing.T) {
settingsGen := gen.StaticSettings{Settings: config.DefaultSimulationSettings()}
eventGen := gen.StaticEvents{DelayedEvents: event.DelayedEventList{}}
assertions := []SimulationAssertion{}
runs := []asim.History{}
runs := []history.History{}
datadriven.RunTest(t, path, func(t *testing.T, d *datadriven.TestData) string {
switch d.Cmd {
case "gen_load":
Expand Down
4 changes: 2 additions & 2 deletions pkg/kv/kvserver/asim/tests/rand_framework.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ import (
"strings"
"time"

"github.com/cockroachdb/cockroach/pkg/kv/kvserver/asim"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/asim/gen"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/asim/history"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/asim/state"
)

Expand Down Expand Up @@ -167,7 +167,7 @@ func loadClusterInfo(configName string) gen.LoadedCluster {
// checkAssertions checks the given history and assertions, returning (bool,
// reason) indicating any failures and reasons if any assertions fail.
func checkAssertions(
ctx context.Context, history asim.History, assertions []SimulationAssertion,
ctx context.Context, history history.History, assertions []SimulationAssertion,
) (bool, string) {
assertionFailures := []string{}
failureExists := false
Expand Down

0 comments on commit 9b80425

Please sign in to comment.