fix: move SetToServerProtocol outside write lock in getOrNewWorkloadPublisher [fj4WqyCCw3C5ShR1RfB7MoBPTpkRrBFYP1uT35g3MvT] - #15593
Open
waterWang wants to merge 2 commits into
Conversation
…ublisher Move the expensive SetToServerProtocol() call (O(n_servers) label-matching) outside the WorkloadWatcher write lock. The write lock is only needed to protect the publishers map access; the protocol detection can run after the lock is released. During thundering-herd Subscribe() storms, holding the write lock across the full scan blocks all submitPodUpdate() readers (via RWMutex semantics), causing a death spiral where existing proxy streams stop receiving endpoint updates and time out, which triggers reconnections that add more writers. Signed-off-by: waterWang <waterWang@users.noreply.github.com>
Member
|
@waterWang This no longer holds the write lock for the duration of |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Move the expensive SetToServerProtocol() call (O(n_servers) label-matching) outside the WorkloadWatcher write lock.
Problem: getOrNewWorkloadPublisher() acquires ww.mu.Lock() and holds it across the entire function, including the cache-miss path where updatePod() calls SetToServerProtocol() - an O(n_servers) label-matching scan. During a thundering-herd Subscribe() storm, this write lock blocks all submitPodUpdate() readers (via RWMutex semantics), causing a self-reinforcing death spiral.
Fix: Move the pod lookup before the lock (read-only informer cache access), then only the publishers map access is inside the critical section. After releasing the lock, call updatePod() / updateExternalWorkload() outside the lock. Each publisher own wp.mu protects concurrent access.
Closes #15583
Signed-off-by: waterWang waterWang@users.noreply.github.com