This document analyses the com.sixt.service.envoy-control-plane codebase for concurrency issues in the ServiceDirectory, focusing on the pod scraping path. The TOCTOU gap in ExtractFromPod causes Collect call amplification that directly explains the slow-startup-without-Linkerd observation.
Key correction from earlier version: The original analysis claimed that pod addresses could be lost due to the TOCTOU race. This was disproven by tracing the code and confirmed by tests — updateRecordFromPod (line 388-406) always adds the missing address and triggers notification. The actual impact is resource waste and startup latency, not data loss.
File: internal/directory/service_directory.go:146-165
sd.mu.RLock()
record := sd.serviceInfoCache[getPodOwnerID(pod)] // line 147: read
sd.mu.RUnlock() // line 148: unlock
// ... gap: no lock held ...
if record == nil {
created, err = sd.createRecordFromPod(...) // line 154: acquires write lock internally
}
sd.mu.Lock() // line 163: re-acquire
updated := created || sd.updateRecordFromPod(...) // line 164
sd.mu.Unlock()For two pods (A, B) with the same owner arriving concurrently:
Pod A goroutine: Pod B goroutine:
───────────────── ─────────────────
147: record = cache[owner] → nil 147: record = cache[owner] → nil
148: RUnlock 148: RUnlock
154: createRecordFromPod(podA) 154: createRecordFromPod(podB)
338: Collect(podA_IP:port) [slow] 338: Collect(podB_IP:port) [slow]
... finishes first ... ... finishes second ...
358: Lock()
372: cache[owner] == nil → create it
376: cache[owner] = {Addr: podA_IP}
383: return created=true 358: Lock() [was blocked]
372: cache[owner] != nil → EXISTS
373: return created=false
163: Lock() 163: Lock()
164: created=true || ... 164: created=false || updateRecordFromPod(podB)
→ short-circuit, updated=true 395: record = cache[owner] → EXISTS
165: Unlock() 401: podB_IP not in Addresses → ADD IT
403: return true → updated=true
167: notify ✓ 165: Unlock()
167: notify ✓
Addresses are NOT lost. The updateRecordFromPod at line 164 always runs for the losing goroutine (since created=false, the || does not short-circuit). It looks up the record (which now exists), finds Pod B's IP is missing, adds it, and returns true. Notification fires.
The real issue is that ALL N goroutines independently call Collector.Collect (line 338) — a gRPC reflection network call — even though only 1 result is needed per owner. All pods in a ReplicaSet return identical metadata.
This is the root cause of slow startup without Linkerd:
- Informer fires
OnAdd(isInInitialList=true)for every pod during initial list - Each
OnAddspawns a goroutine →ExtractFromPod(pod, isInitial=true)→IsInInit.Add(1) - Due to the TOCTOU gap, ALL N pods for the same owner call
createRecordFromPod→Collect - Each
Collecthas a retry policy: 5 attempts, 5s→60s exponential backoff,waitForReady: true - Without Linkerd: raw TCP, no connection reuse → slow connects, UNAVAILABLE retries
- With Linkerd: connection pooling + L7 retries → Collect returns fast
- Each goroutine holds
IsInInitelevated →/readystays unhealthy until ALL complete
With 100 services × 3 replicas = 300 pods, that's 300 Collect calls instead of 100.
All pods in a ReplicaSet run the same container image. Collector.Collect returns identical results from any pod in the same RS. The redundant calls are pure waste.
File: internal/directory/service_directory.go:471-499
Note: This is a real bug but is independent of the slow-startup problem (which was observed after disabling REST on STAGE — meaning this code path was not exercised).
sd.mu.RLock()
record := sd.serviceInfoCache[ownerID] // line 472
sd.mu.RUnlock() // line 473
if record == nil {
updated, err = sd.createRecordFromPod(...) // line 477: NETWORK CALL in gap
}
sd.mu.Lock() // line 486
// ... updateRESTServiceInfo ... // line 494
sd.mu.Unlock()When two HTTPRoute events for the same owner arrive concurrently:
- Both see
record == nil, both callcreateRecordFromPod(redundant Collect) - Both then call
updateRESTServiceInfosequentially under the write lock UpdateRESTMethods(withrouteNum=0) replaces the method list from its own matches only- The second writer overwrites the first writer's REST paths
REST endpoints ARE lost in this path (unlike the pod path above). This is confirmed by TestExtractFromHTTPRoute_ConcurrentSameOwner_TOCTOU which fails reliably.
| Test | Result | What it proves |
|---|---|---|
RedundantCollect |
PASS + warning: 10 calls | N×Collect amplification per owner |
InitBlocking |
PASS: peak=N | IsInInit blocked N× → slow startup |
CollectFailureAmplification |
PASS: N failures | Cascading errors when pod not ready |
SubscriberNotification |
PASS | Addresses NOT lost (disproves original claim) |
RaceDetector |
PASS | Shallow-clone pointer safety |
HTTPRoute_TOCTOU |
FAIL | REST endpoint loss (real, separate bug) |
AddRemove |
PASS | Add/remove ordering is safe |
WithPodWatcher |
PASS + warning | Cross-watcher amplification |
Use x/sync/singleflight keyed by ownerID wrapping the Collector.Collect call:
import "golang.org/x/sync/singleflight"
// In ServiceDirectory:
var collectGroup singleflight.Group
// In createRecordFromPod, replace direct Collect call:
result, err, _ := sd.collectGroup.Do(ownerID, func() (interface{}, error) {
return sd.Collector.Collect(ctx, addr)
})This ensures only 1 Collect call per owner regardless of how many pods arrive concurrently. Benefits:
IsInInitduration reduced from N×Collect to 1×Collect- Error amplification eliminated (1 failure, not N)
- Network load reduced by N× during startup
- Startup time matches the with-Linkerd behavior
That's the key issue I think: