server/cluster: RaftCluster.Start leaks goroutines on startup failure

[lance6716]

Bug report

RaftCluster.Start() can leak startup goroutines when it returns an error before running is set to true.

The failure mode is generic and not tied to any one feature: Start() creates components that launch background loops using c.ctx, but if a later startup step fails, Start() returns without canceling that context. Since running is still false, a later RaftCluster.Stop() also returns early and never calls c.cancel().

Current startup path that reproduces it

On current master, this sequence is enough:

1. Start() creates RegionLabeler, which starts RegionLabeler.doGC().
2. Start() creates affinity.Manager, which starts its availability check loop.
3. keyspaceGroupManager.Bootstrap(c.ctx) returns an error.
4. Start() returns early while c.ctx is still live.
5. The package-level goleak check reports leaked goroutines.

Reproduction

I reproduced this locally with a regression test under server/cluster that makes keyspaceGroupManager.Bootstrap fail after the earlier startup components are initialized.

Run:

go test ./server/cluster -run TestStartCancelsContextOnBootstrapFailure -count=1

Observed result: the test body itself passes the expected startup error, but the package fails in goleak with leaked goroutines including:

• labeler.(*RegionLabeler).doGC
• affinity.(*Manager).startAvailabilityCheckLoop.func2

I also saw related partially-initialized background goroutines such as hot cache workers still alive in the same run.

Expected result

If RaftCluster.Start() fails at any point after InitCluster() succeeds, all background work started off c.ctx should be torn down before returning.

Likely root cause

Start() currently relies on Stop() for normal shutdown, but Stop() exits early when running == false. That means any startup failure after background goroutines are created but before running = true has no unified rollback path.

Suggested fix

Add a startup-failure rollback path inside RaftCluster.Start() itself. For example, after InitCluster() succeeds, defer a cleanup that cancels c.ctx when err != nil.

That would make startup failures self-contained and avoid depending on Stop() for a state where running was never set.

---

• LLM model: gpt-5.4
• Thinking level: xhigh
• Client: Codex Desktop 26.305.950 (build 863); codex-cli 0.105.0

[lance6716]

package cluster

import (
	"context"
	stderrors "errors"
	"testing"

	"github.com/pingcap/kvproto/pkg/metapb"
	"github.com/pingcap/kvproto/pkg/pdpb"
	"github.com/stretchr/testify/require"

	"github.com/tikv/pd/pkg/core"
	"github.com/tikv/pd/pkg/id"
	"github.com/tikv/pd/pkg/keyspace"
	"github.com/tikv/pd/pkg/metering"
	"github.com/tikv/pd/pkg/mock/mockid"
	"github.com/tikv/pd/pkg/schedule/hbstream"
	"github.com/tikv/pd/pkg/storage"
	"github.com/tikv/pd/pkg/storage/endpoint"
	"github.com/tikv/pd/pkg/storage/kv"
	"github.com/tikv/pd/pkg/utils/syncutil"
	"github.com/tikv/pd/server/config"
)

var errBootstrapKeyspaceGroup = stderrors.New("bootstrap keyspace group error")

type failingBootstrapKeyspaceGroupStorage struct {
	*endpoint.StorageEndpoint
}

func (*failingBootstrapKeyspaceGroupStorage) SaveKeyspaceGroup(kv.Txn, *endpoint.KeyspaceGroup) error {
	return errBootstrapKeyspaceGroup
}

type startFailureTestServer struct {
	allocator            id.Allocator
	cfg                  *config.Config
	persistOptions       *config.PersistOptions
	storage              storage.Storage
	hbStreams            *hbstream.HeartbeatStreams
	raftCluster          *RaftCluster
	keyspaceGroupManager *keyspace.GroupManager
	keyspaceEnabled      bool
}

func (s *startFailureTestServer) GetAllocator() id.Allocator { return s.allocator }

func (s *startFailureTestServer) GetConfig() *config.Config { return s.cfg }

func (s *startFailureTestServer) GetPersistOptions() *config.PersistOptions { return s.persistOptions }

func (s *startFailureTestServer) GetStorage() storage.Storage { return s.storage }

func (s *startFailureTestServer) GetHBStreams() *hbstream.HeartbeatStreams { return s.hbStreams }

func (s *startFailureTestServer) GetRaftCluster() *RaftCluster { return s.raftCluster }

func (s *startFailureTestServer) GetBasicCluster() *core.BasicCluster {
	return s.raftCluster.BasicCluster
}

func (*startFailureTestServer) GetMembers() ([]*pdpb.Member, error) { return nil, nil }

func (*startFailureTestServer) ReplicateFileToMember(context.Context, *pdpb.Member, string, []byte) error {
	return nil
}

func (*startFailureTestServer) GetKeyspaceManager() *keyspace.Manager { return nil }

func (s *startFailureTestServer) GetKeyspaceGroupManager() *keyspace.GroupManager {
	return s.keyspaceGroupManager
}

func (s *startFailureTestServer) IsKeyspaceGroupEnabled() bool { return s.keyspaceEnabled }

func (*startFailureTestServer) GetMeteringWriter() *metering.Writer { return nil }

func TestStartCancelsContextOnBootstrapFailure(t *testing.T) {
	re := require.New(t)
	ctx := context.Background()
	store := storage.NewStorageWithMemoryBackend()
	re.NoError(store.SaveMeta(&metapb.Cluster{Id: 1, MaxPeerCount: 1}))

	_, opt, err := newTestScheduleConfig()
	re.NoError(err)

	rc := &RaftCluster{
		serverCtx:      ctx,
		BasicCluster:   core.NewBasicCluster(),
		storage:        store,
		storeStateLock: syncutil.NewLockGroup(syncutil.WithRemoveEntryOnUnlock(true)),
	}

	keyspaceStore := &failingBootstrapKeyspaceGroupStorage{StorageEndpoint: endpoint.NewStorageEndpoint(kv.NewMemoryKV(), nil)}
	keyspaceGroupManager := keyspace.NewKeyspaceGroupManager(ctx, keyspaceStore, nil)
	t.Cleanup(func() {
		keyspaceGroupManager.Close()
	})

	server := &startFailureTestServer{
		allocator:            mockid.NewIDAllocator(),
		cfg:                  config.NewConfig(),
		persistOptions:       opt,
		storage:              store,
		raftCluster:          rc,
		keyspaceGroupManager: keyspaceGroupManager,
		keyspaceEnabled:      true,
	}

	err = rc.Start(server, true)
	re.ErrorIs(err, errBootstrapKeyspaceGroup)
	re.NotNil(rc.regionLabeler)
	re.NotNil(rc.affinityManager)
	re.False(rc.IsRunning())
}

This unit test will fail on goroutine leak

[lance6716]

/type bug

[lance6716]

/severity moderate