[WIP] Informer pools#3325
Draft
csviri wants to merge 18 commits into
Draft
Conversation
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
Open
csviri
marked this pull request as ready for review
April 29, 2026 13:28
Contributor
There was a problem hiding this comment.
Pull request overview
Introduces initial scaffolding for “event source pooling” (starting with informers) under processing.event.source.pool, likely to enable sharing/reuse of informers across components/controllers.
Changes:
- Added
EventSourcePoolinterface andAbstractEventSourcePoolbase type. - Added
InformerClassifierrecord to key pooled informers by selector/namespace/resource type. - Added initial (currently incomplete)
InformerPoolimplementation and a minor whitespace cleanup inInformerManager.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/pool/InformerPool.java |
Adds a new pool for SharedIndexInformer<?> instances (currently stubbed/incomplete). |
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/pool/InformerClassifier.java |
Adds a classifier record intended as the cache key for pooled informers. |
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/pool/EventSourcePool.java |
Introduces a generic pool interface for event sources. |
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/pool/AbstractEventSourcePool.java |
Adds a base class placeholder for pool implementations. |
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java |
Removes an extraneous whitespace line in createEventSource. |
Comment on lines
+24
to
+33
| var actual = informers.get(classifier); | ||
| if (actual == null) { | ||
| actual = null; // create Informer | ||
| } | ||
| incrementCounter(actual); | ||
| return null; | ||
| } | ||
|
|
||
| private synchronized void incrementCounter(SharedIndexInformer<?> actual) { | ||
| counters.compute(actual, (k, v) -> new AtomicInteger(v == null ? 0 : v.incrementAndGet())); |
| } | ||
|
|
||
| private synchronized void incrementCounter(SharedIndexInformer<?> actual) { | ||
| counters.compute(actual, (k, v) -> new AtomicInteger(v == null ? 0 : v.incrementAndGet())); |
openshift-ci
Bot
added
the
needs-rebase
csviri
marked this pull request as draft
May 4, 2026 12:59
csviri
force-pushed
the
next
branch
2 times, most recently
from
May 11, 2026 11:49
d5157bd to
0297680
Compare
openshift-ci
Bot
removed
the
needs-rebase
csviri
force-pushed
the
next
branch
3 times, most recently
from
July 2, 2026 06:51
e45bf4c to
1e91a47
Compare
openshift-ci
Bot
added
the
needs-rebase
openshift-ci
Bot
removed
the
needs-rebase
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| return (SharedIndexInformer<R>) informer; | ||
| } | ||
|
|
||
| public synchronized <R extends HasMetadata> void releaseInformer( |
| return configurationService; | ||
| } | ||
|
|
||
| public void setConfigurationService(ConfigurationService configurationService) { |
| return client; | ||
| } | ||
|
|
||
| public void setClient(KubernetesClient client) { |
| private final Map<InformerClassifier<?>, AtomicInteger> counters = new HashMap<>(); | ||
|
|
||
| @SuppressWarnings("unchecked") | ||
| public <R extends HasMetadata> SharedIndexInformer<R> getInformer( |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 25 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java:66
- InformerWrapper.start() is now a no-op while InformerManager.start() still calls iw.start() to ensure informers are started, and stop() still calls informer.stop(). With shared informers, stopping from a single wrapper can stop the informer for other controllers, and the no-op start makes the lifecycle expectations unclear.
public void start() {}
@Override
public void stop() throws OperatorException {
informer.stop();
Comment on lines
+152
to
+156
| var informer = | ||
| Optional.ofNullable(informerConfig.getInformerListLimit()) | ||
| .map(filteredBySelectorClient::withLimit) | ||
| .orElse(filteredBySelectorClient) | ||
| .runnableInformer(0); | ||
| Optional.ofNullable(informerConfig.getItemStore()).ifPresent(informer::itemStore); | ||
| var source = | ||
| informerPool.getInformer(configuration.getInformerConfig().getName(), classifier); | ||
| source = | ||
| new InformerWrapper<>( | ||
| informer, controllerConfiguration.getConfigurationService(), namespaceIdentifier); | ||
| informer, namespaceIdentifier, controllerConfiguration.getConfigurationService()); |
Comment on lines
+48
to
+72
| SharedIndexInformer<R> informer; | ||
| synchronized (this) { | ||
| informer = (SharedIndexInformer<R>) informers.get(classifier); | ||
| if (informer == null) { | ||
| informer = createInformer(classifier); | ||
| informers.put(classifier, informer); | ||
| counters.put(classifier, new AtomicInteger(1)); | ||
| } else { | ||
| informers.keySet().stream() | ||
| .filter(existing -> existing.differsOnlyByInformerListLimit(classifier)) | ||
| .findFirst() | ||
| .ifPresent( | ||
| existing -> | ||
| log.warn( | ||
| "Reusing informer for classifier {} that differs only by informerListLimit" | ||
| + " (existing: {}, requested: {}). The existing informerListLimit is" | ||
| + " kept.", | ||
| classifier, | ||
| existing.informerListLimit(), | ||
| classifier.informerListLimit())); | ||
| counters.get(classifier).incrementAndGet(); | ||
| } | ||
| } | ||
| start(informer, classifier); | ||
| return informer; |
Comment on lines
+80
to
+89
| var counter = counters.get(classifier); | ||
| informer = (SharedIndexInformer<R>) informers.get(classifier); | ||
| if (counter != null && counter.decrementAndGet() == 0) { | ||
| counters.remove(classifier); | ||
| informers.remove(classifier); | ||
| // todo check if we can remove event handled if informer stopped | ||
| informer.stop(); | ||
| } else { | ||
| log.warn("No informer found in the pool."); | ||
| } |
Comment on lines
+75
to
+97
| FilterWatchListDeletable filteredClient; | ||
| if (WATCH_ALL_NAMESPACES.equals(classifier.namespaceIdentifier())) { | ||
| filteredClient = | ||
| clientWithResource | ||
| .inAnyNamespace() | ||
| .withLabelSelector(classifier.labelSelector()) | ||
| .withShardSelector(classifier.shardSelector()); | ||
| } else { | ||
| filteredClient = | ||
| clientWithResource | ||
| .inNamespace(classifier.namespaceIdentifier()) | ||
| .withLabelSelector(classifier.labelSelector()) | ||
| .withShardSelector(classifier.shardSelector()); | ||
| } | ||
|
|
||
| if (classifier.labelSelector() != null) { | ||
| filteredClient = | ||
| (FilterWatchListDeletable) filteredClient.withLabelSelector(classifier.labelSelector()); | ||
| } | ||
| if (classifier.shardSelector() != null) { | ||
| filteredClient = | ||
| (FilterWatchListDeletable) filteredClient.withShardSelector(classifier.shardSelector()); | ||
| } |
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/DefaultInformerPool.java:85
- releaseInformer() logs "No informer found" for the common case where an informer exists but the ref-count is still > 0. It also decrements the counter inside an extra synchronized block (method is already synchronized), and can attempt to stop a null informer if the maps get out of sync. This will spam logs and makes it harder to reason about pool state.
if (counter != null && counter.decrementAndGet() == 0) {
Comment on lines
+25
to
+36
| import io.fabric8.kubernetes.api.model.HasMetadata; | ||
| import io.fabric8.kubernetes.client.KubernetesClient; | ||
| import io.fabric8.kubernetes.client.informers.SharedIndexInformer; | ||
|
|
||
| @SuppressWarnings({"unchecked", "rawtypes"}) | ||
| public class AlwaysCreateInformerPool extends AbstractInformerPool { | ||
|
|
||
| private static final Logger log = LoggerFactory.getLogger(AlwaysCreateInformerPool.class); | ||
|
|
||
| private final Map<ClassifierWithName, SharedIndexInformer> informers = new ConcurrentHashMap(); | ||
|
|
||
| @Override |
Comment on lines
+194
to
+196
| informerPool.releaseInformer( | ||
| configuration.getInformerConfig().getName(), wrapper.getClassifier()); | ||
| log.debug("Stopping informer for namespace: {} -> {}", ns, wrapper); |
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
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.
Will put here early impl so we can discuss on the community meeting. Also will expand design description in the issue.
Some key design points: