with nits

Very nice, @irfansharif and I were talking about this just yesterday.

Reviewed 10 of 10 files at r1.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @lunevalex)

pkg/server/config.go, line 502 at r1 (raw file):

			if spec.StickyInMemoryEngineID != "" {
				if cfg.TestingKnobs.Server == nil {
					return Engines{}, errors.Errorf("Could not create a sticky " +

I'd suggest errors.AssertionFailedf so that if an end-user sees this (via cockroach demo), they don't get the mistaken understanding that they can do something about it themselves.

pkg/testutils/testcluster/testcluster.go, line 1102 at r1 (raw file):

func (tc *TestCluster) RestartServer(idx int) error {
	if !tc.serverStopped(idx) {
		return errors.Errorf("Server %d must be stopped before attempting to restart", idx)

nit : small s at the start of the message

pkg/testutils/testcluster/testcluster.go, line 1128 at r1 (raw file):

	for _, specs := range serverArgs.StoreSpecs {
		if specs.StickyInMemoryEngineID == "" {
			return errors.Errorf(" Failed to restart Server %d. Restart can only be used when the servers were started with a sticky engine")

nit: no space and small case f

Thanks! Yea we were discussing how this would be a useful API to have, one that would let us stop leaning on roachtests for large swaths of "integration" tests where we desire real server life cycles.

re: naming, I remember being bitten by this in #54072 when I had tried something like:

tc.Start(t)
tc.StopServer(i)
tc.StartServer(i) // This is not a thing on master; I simply exported tc.startServer.

Which is to say I have a mild preference for overloading Start (for both individual servers and the entire cluster) instead of introducing the Restart symbol. It looks doable. Feel free to ignore though, or defer to a future PR.

Reviewed 1 of 10 files at r1.
Reviewable status: complete! 2 of 0 LGTMs obtained (waiting on @knz and @lunevalex)

pkg/server/sticky_engine.go, line 80 at r1 (raw file):

}

// NewStickyInMemEnginesRegistry creates a new StickyInMemEnginesRegistry

nit: Periods after the sentence.

pkg/server/sticky_engine.go, line 88 at r1 (raw file):

// GetOrCreateStickyInMemEngine implements a method from
// StickyInMemEnginesRegistry.

minor nit: we typically use the "X implements the Y interface" structure for these comments.

pkg/server/testing_knobs.go, line 81 at r1 (raw file):

	OnDecommissionedCallback func(livenesspb.Liveness)
	// StickyEngineRegistry manages the lifecycle of sticky in memory engines,
	// which can be enabled via base.StoreSpec.StickyInMemoryEngineID

nit: Period after the sentence.

pkg/testutils/testcluster/testcluster.go, line 1108 at r1 (raw file):

	if idx == 0 {
		// If it's the first server, then we need to restart the RPC listener by hand.
		// Look at #NewTestCluster method for more details.

nit: Drop the "#" and "method"

pkg/testutils/testcluster/testcluster.go, line 1128 at r1 (raw file):

Lower case "server" as well, we generally use a single sentence with no periods for error messages. This helps them compose better when one wraps the other (same reason for the lower case).

pkg/testutils/testcluster/testcluster_test.go, line 317 at r1 (raw file):

	tc.WaitForValues(t, roachpb.Key("b"), []int64{9, 9, 9})

	require.NoError(t, tc.Restart())

nit: would be nice to first restart a single server in isolation, test that, and only then restart the entire cluster (I can imagine tc.Restart accreting code in addition to simply restarting each server independently).

Hmm, I am not sure thats possible as it was tricky to get this to work, specifically around having the servers being able to connect back to the cluster correctly. startServer also just starts a newly created server, restart recreates the server and places it back into the array, so it actually ends up being a mix of what AddServer does and startServer does.

Reviewable status: complete! 2 of 0 LGTMs obtained (waiting on @irfansharif and @knz)

pkg/server/config.go, line 502 at r1 (raw file):

👍

pkg/testutils/testcluster/testcluster_test.go, line 317 at r1 (raw file):

Good idea, I'll add that to the test