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

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

Aperta
#3,617 1 commento 0 reazioni 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

Lingua principale
Java
Stelle
944
Fork
242
Merge medio
1g 4h
PR unite (30g)
43

Descrizione

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?

Guida per i contributori

Apri la guida per i contributori

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Direzione di ricerca

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.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
java, kubernetes
Ambito
backend, distributed-systems, performance
Tipo di issue
Bug
Difficoltà
4/5
Tempo stimato
3-5 giorni
Stato di attività
Attiva
Chiarezza
Abbastanza chiara
Idoneità per principianti
48/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.