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

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

Abierto
#3,617 1 comentario 0 reacciones 0 asignados Ver en GitHub

Nadie ha tomado este issue todavía.

Lenguaje dominante
Java
Estrellas
944
Forks
242
Merge medio
1 d 4 h
PR fusionados (30 d)
43

Descripción

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?

Guía de contribución

Abrir la guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Línea de trabajo

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.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
java, kubernetes
Área
backend, distributed-systems, performance
Tipo de issue
Error
Dificultad
4/5
Tiempo estimado
3-5 días
Estado de actividad
Activo
Claridad
Bastante claro
Aptitud para principiantes
48/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.