asim: load cluster configuration to be simulated

[lidorcarmel]

Adding a few cluster configurations and a way to load those configs to be
simulated.

Note that this is somewhat different from what I had in mind initially: with
this implementation there is no way to change the configuration mid-run (we
will add that later).

Release note: None

[cockroach-teamcity]

This change is 

[kvoli]

Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @lidorcarmel and @shralex)

---

pkg/kv/kvserver/asim/asim.go, line 208 at r1 (raw file):

		generators: wgs,
		conf:       conf,
		prevState:  NewState(),

Should prevState be set to initialState on this call?

In the first tick, applyWorkload and applyAllocatorAction will both need to consult the previous state in order to apply changes to the nextState?kjkj

Code quote:

		prevState:  NewState(),
		nextState:  NewState(),

---

pkg/kv/kvserver/asim/config_loader.go, line 90 at r1 (raw file):

}

// Zone is a simulated availability zone.

We will need to convert these to roachpb.Locality somewhere along the line. I don't see an issue with trying to keep an independent data model so we can be more flexible but I'm curious what you think and why you picked this method.

Code quote:

// Zone is a simulated availability zone.
type Zone struct {
	Name      string
	NodeCount int
}

---

pkg/kv/kvserver/asim/config_loader.go, line 105 at r1 (raw file):

// TODO(lidor): add cross region network latencies.
type ClusterInfo struct {
	DiskCapacityGB int

Is this per node and we take them all as homogenous? Seems fine just want to check, perhaps we could define a node resources type for configuration in the future, similar to the locality here.

---

pkg/kv/kvserver/asim/config_loader.go, line 114 at r1 (raw file):

	s := NewState()
	s.Cluster = &c
	// TODO(lidor): load locality info to be used by the allocator. Do we need a

I think we need to include the locality information here, I believe the locality can go on the store descriptor specifically.

---

pkg/kv/kvserver/asim/config_loader_test.go, line 30 at r1 (raw file):

		},
		{
			clusterInfo:       asim.MultiRegionConfig,

We should add in the node locality above and assert on the correct locality information in addition to counts.

Code quote:

			clusterInfo:       asim.SingleRegionConfig,
			expectedNodeCount: 15,
		},
		{
			clusterInfo:       asim.MultiRegionConfig,
			expectedNodeCount: 36,
		},
		{
			clusterInfo:       asim.ComplexConfig,
			expectedNodeCount: 28,

[lidorcarmel]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @kvoli and @shralex)

---

pkg/kv/kvserver/asim/asim.go, line 208 at r1 (raw file):

Previously, kvoli (Austen) wrote…

Should prevState be set to initialState on this call?

In the first tick, applyWorkload and applyAllocatorAction will both need to consult the previous state in order to apply changes to the nextState?kjkj

the way things are now, ApplyLoad and ApplyAllocatorAction only care about nextState.
but the naming is not great, fixed a but, PTAL, thanks!

---

pkg/kv/kvserver/asim/config_loader.go, line 90 at r1 (raw file):

Previously, kvoli (Austen) wrote…

We will need to convert these to roachpb.Locality somewhere along the line. I don't see an issue with trying to keep an independent data model so we can be more flexible but I'm curious what you think and why you picked this method.

yes we will - I thought it would be easier to specify a config this way. also, we will need other things like network delay between regions to fit in this structure.

---

pkg/kv/kvserver/asim/config_loader.go, line 105 at r1 (raw file):

Previously, kvoli (Austen) wrote…

Is this per node and we take them all as homogenous? Seems fine just want to check, perhaps we could define a node resources type for configuration in the future, similar to the locality here.

exactly, we should be ok with homogenous disks.

---

pkg/kv/kvserver/asim/config_loader.go, line 114 at r1 (raw file):

Previously, kvoli (Austen) wrote…

I think we need to include the locality information here, I believe the locality can go on the store descriptor specifically.

right, in another pr?

---

pkg/kv/kvserver/asim/config_loader_test.go, line 30 at r1 (raw file):

Previously, kvoli (Austen) wrote…

We should add in the node locality above and assert on the correct locality information in addition to counts.

right, but now the locality info goes nowhere. we will do that once we actually propagate locality to where it should be. does it make sense? I agree that the current test is useless.. it will be improved.

[kvoli]

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @kvoli, @lidorcarmel, and @shralex)

---

pkg/kv/kvserver/asim/asim.go, line 208 at r1 (raw file):

Previously, lidorcarmel (Lidor Carmel) wrote…

the way things are now, ApplyLoad and ApplyAllocatorAction only care about nextState.
but the naming is not great, fixed a but, PTAL, thanks!

Okay, got it - seems good.

---

pkg/kv/kvserver/asim/config_loader.go, line 90 at r1 (raw file):

Previously, lidorcarmel (Lidor Carmel) wrote…

yes we will - I thought it would be easier to specify a config this way. also, we will need other things like network delay between regions to fit in this structure.

Cool, lets go with that then.

---

pkg/kv/kvserver/asim/config_loader.go, line 114 at r1 (raw file):

Previously, lidorcarmel (Lidor Carmel) wrote…

right, in another pr?

Yeah that makes sense, we definitely will want it in the near future as it seems like one of the harder parts of the allocator's logic to reason about from my exp.

[lidorcarmel]

TFTR Austen!

bors r+

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @kvoli, @lidorcarmel, and @shralex)

[craig]

Build failed:

• GitHub CI (Cockroach)

[lidorcarmel]

unrelated failure, retrying.

bors r+

[craig]

Build failed:

• GitHub CI (Cockroach)

[kvoli]

Looks like this was fixed on master. Try rebase off a newer sha.

[lidorcarmel]

bors r+

[craig]

Build failed:

• GitHub CI (Cockroach)

[lidorcarmel]

bors r+

[craig]

Build succeeded:

• GitHub CI (Cockroach)