fix data race in Patroni discovery cache after parallel source resolution

[Syedowais312]

Hi, I think the recent parallel source discovery change introduced a concurrency regression in Patroni discovery.

lastFoundClusterMembers is a package-global map accessed from multiple goroutines after Sources.ResolveDatabases() started resolving sources concurrently.

I traced unsynchronized accesses in:

internal/sources/resolver.go:83
internal/sources/resolver.go:205
internal/sources/resolver.go:328

This can potentially cause concurrent map write panics or race detector failures when multiple continuous-discovery Patroni sources run simultaneously.

I tested a fix locally using an RWMutex-guarded cache helper plus a concurrent regression test.

If this direction looks good, I can open a PR.

[pashagolub]

How did you trace it? Can you reproduce it?

[Syedowais312]

I traced the issue from the real resolver code first: Sources.ResolveDatabases() became concurrent, while the package-global lastFoundClusterMembers cache remained unsynchronized.

To verify that this is a real concurrency issue and not just a theoretical concern, I wrote a minimal focused repro test that exercises the same shared global map access pattern under concurrency:

func TestLastFoundClusterMembersCacheConcurrentAccess(t *testing.T) {
	t.Cleanup(func() {
		lastFoundClusterMembers = make(map[string][]PatroniClusterMember)
	})

	const goroutines = 32
	const iterations = 2000

	var wg sync.WaitGroup
	for g := 0; g < goroutines; g++ {
		g := g
		wg.Add(1)

		go func() {
			defer wg.Done()

			for i := 0; i < iterations; i++ {
				sourceName := fmt.Sprintf("source-%d", (g+i)%8)

				members := []PatroniClusterMember{{
					Name:    fmt.Sprintf("member-%d-%d", g, i),
					Scope:   fmt.Sprintf("scope-%d", g%4),
					ConnURL: fmt.Sprintf("postgres://source-%d/%d", g, i),
					Role:    "primary",
				}}

				lastFoundClusterMembers[sourceName] = members
				_ = lastFoundClusterMembers[sourceName]
			}
		}()
	}

	wg.Wait()
}

Running it under -race reliably reports data races and eventually crashes with:

fatal error: concurrent map writes

Command used:

GOCACHE=/tmp/gocache \
GOMODCACHE=/tmp/gomodcache \
GOTMPDIR=/tmp/gotmp \
go test -race ./internal/sources \
  -run TestLastFoundClusterMembersCacheConcurrentAccess \
  -count=100

So the concurrency issue seems reproducible once the cache is accessed concurrently after the parallel source discovery change.

Please let me know if I’m reasoning about this incorrectly or missing some synchronization guarantee elsewhere in the resolver flow.

[0xgouda]

So, you wrote a test that accesses a global map from several goroutines without locks, and you consider it a reproduction of a race condition :)?

[Syedowais312]

Fair point. The test only proves the cache becomes unsafe once accessed concurrently.

What made me suspect the real resolver path is that Sources.ResolveDatabases() became concurrent in 1677cdb09, while the Patroni path still accesses the package-global lastFoundClusterMembers map without synchronization.

So my reasoning was:

1. source resolution became concurrent
2. multiple Patroni discovery goroutines can now run simultaneously
3. they share the same global cache
4. the cache has unsynchronized reads/writes

I agree this does not yet prove the real resolver flow reaches that state in practice. I’ll try to build a more realistic reproducer using multiple Patroni sources through the actual resolver path?