operator-framework / operator-framework/java-operator-sdk

Informer start under the workflow executor's monitor serializes the first reconciliation

Offen
#3,617 1 Kommentar 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen

Dieses Issue hat noch niemand übernommen.

Vorherrschende Sprache
Java
Sterne
944
Forks
242
Ø Merge
1 T. 4 Std.
Gemergte PRs (30 T.)
43

Beschreibung

We hit an unexpected slowdown on the first reconciliation of a primary and traced it back to how activation-conditioned dependents get their informers started.

WorkflowReconcileExecutor.handleReconcile is synchronized (WorkflowReconcileExecutor.java:54) and calls registerOrDeregisterEventSourceBasedOnActivation at line 95. For a dependent with an activation condition, that ends up in EventSourceManager.dynamicallyRegisterEventSource -> eventSource.start() (EventSourceManager.java:256) -> InformerManager.start (:79), which blocks until the informer's initial LIST has synced. Since the executor's monitor is held the whole time, all the other dependents queue up behind it, so the first reconciliation ends up paying the sum of every activation-conditioned informer's sync time instead of overlapping them.

What made us fairly confident this isn't intended: EventSourceManager already moves eventSource.start() outside its own synchronized block (lines 252-256, with the "lock striping" comment). The intent is clearly that a blocking start shouldn't sit under a lock, it's just that the workflow executor happens to hold a different one across the same call.

Here's what we saw. JFR from one first reconciliation:

pool-10-thread-2  parked 978 ms
    InformerManager.start() line: 79
    EventSourceManager.dynamicallyRegisterEventSource(EventSource) line: 256
    AbstractWorkflowExecutor.registerOrDeregisterEventSourceBasedOnActivation(...) line: 197
    WorkflowReconcileExecutor.handleReconcile(DependentResourceNode) line: 95

pool-10-thread-5  blocked 967 ms on WorkflowReconcileExecutor monitor, previousOwner = pool-10-thread-2
pool-10-thread-4  blocked 966 ms on WorkflowReconcileExecutor monitor, previousOwner = pool-10-thread-5

That reconciliation took 2879 ms; with the same informers syncing quickly it was 136 ms. In our case the syncs are slow because those informers use informerListLimit, which stops the API server from answering the LIST from its watch cache, but anything that makes an initial LIST slow would do the same, so we suspect others could run into this without ever touching that setting.

Two pontential ideas here:

  1. Register activation-conditioned event sources eagerly at controller start when the condition doesn't need a primary to evaluate (ours are just configuration checks). The sync then happens at startup, in parallel, covered by cacheSyncTimeout, and never during a reconciliation. This is the one we'd prefer, since it takes the cost off the reconciliation path completely.
  2. Start dynamically registered event sources outside the executor's monitor. The registration bookkeeping needs the lock, the blocking start() doesn't. This lets the syncs overlap, though the first reconciliation still waits for the slowest one.

Happy to put up a PR for whichever shape you prefer.

On option 1: we had a look and there's no notion of a primary-independent activation condition today, Condition.isMet always takes the primary. Though CRDPresentActivationCondition ignores it completely and caches globally, so it seems like the common case. Would you be open to something explicit for it, like a marker interface or a requiresPrimary() default, so those can be registered eagerly at startup?

Beitragsleitfaden

Beitragsleitfaden öffnen

Erste Schritte

  1. Lies das ganze Issue und danach den Beitragsleitfaden des Projekts.
  2. Schreib ins Issue, dass du es übernimmst — das erspart doppelte Arbeit.
  3. Forke das Repository und arbeite in einem Branch.
  4. Öffne einen Pull Request, der die Issue-Nummer nennt.

Rechercherichtung

Start with WorkflowReconcileExecutor.handleReconcile at line 54 and line 95, then trace EventSourceManager.dynamicallyRegisterEventSource through eventSource.start() at line 256 and InformerManager.start at line 79. Compare this path with EventSourceManager's lock-striping section around lines 252-256. Done means activation-conditioned informer startup no longer serializes unrelated dependents, with the chosen eager or out-of-monitoring approach validated against the first-reconciliation behavior described here.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
java, kubernetes
Bereich
backend, distributed-systems, performance
Issue-Typ
Bug
Schwierigkeit
4/5
Geschätzter Aufwand
3-5 Tage
Aktivitätsstatus
Aktiv
Klarheit
Größtenteils klar
Anfängerfreundlichkeit
48/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.