Skip to content

Commit 3f92152

Browse files
afalhambra-hivemqcsviri
authored andcommitted
feat: allow removing the default namespace index of an informer (#3623)
Signed-off-by: Antonio Fernandez Alhambra <antonio.alhambra@hivemq.com>
1 parent e7af8d9 commit 3f92152

22 files changed

Lines changed: 539 additions & 36 deletions

File tree

‎docs/content/en/docs/documentation/eventing.md‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,30 @@ See
348348
also [CaffeineBoundedItemStores](https://github.com/operator-framework/java-operator-sdk/blob/main/caffeine-bounded-cache-support/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeineBoundedItemStores.java)
349349
for more details.
350350

351+
### Removing the Namespace Index
352+
353+
Informers keep an index from namespace to the resources cached for it. JOSDK never reads that index,
354+
only the ones registered explicitly through `IndexerResourceCache.addIndexers(..)`, so it can be
355+
removed to save an entry per cached resource:
356+
357+
```java
358+
@ControllerConfiguration(informer = @Informer(withoutNamespaceIndex = true))
359+
public class MyReconciler implements Reconciler<MyCustomResource> { }
360+
```
361+
362+
The same option is available on `InformerEventSourceConfiguration.Builder` for event sources, and on
363+
`InformerConfiguration.Builder`.
364+
365+
This matters most for informers that cache a large number of resources, and in particular together
366+
with a custom [item store](#bounded-caches-for-informers) that keeps only a reduced form of each
367+
resource: the index is keyed independently of what the store does with the resource itself, so
368+
shrinking what is cached does not shrink the index.
369+
370+
Note that this takes part in the informer pool identity described below: an event source that
371+
removes the index does not share an informer with one that keeps it. If two event sources watch the
372+
same resource type and only one of them sets the option, they end up with two informers, and two
373+
caches of that resource type, which can cost far more memory than the index ever did.
374+
351375
### Sharing Informers Between Controllers (Informer Pool)
352376

353377
{{% alert title="Experimental" color="warning" %}}
@@ -374,7 +398,10 @@ Two event sources share an informer when their effective informer configuration
374398
- the resource type (or the group/version/kind for generic resources),
375399
- the watched namespace,
376400
- the label, field and shard selectors,
377-
- the configured [item store](#bounded-caches-for-informers).
401+
- the configured [item store](#bounded-caches-for-informers),
402+
- whether the [namespace index is removed](#removing-the-namespace-index): it cannot be present for
403+
one event source and absent for another on one shared informer, so event sources that disagree on
404+
it are backed by separate informers.
378405

379406
The `informerListLimit` is intentionally *not* part of this identity: if two otherwise-equivalent
380407
event sources request a different list limit, the existing informer is reused (a warning is logged

‎docs/content/en/docs/documentation/operations/configuration.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,7 @@ josdk.controller.mycontroller.namespaces=team-a,team-b
385385
| `josdk.controller.<name>.informer.shard-selector` | `String` | Shard selector for the primary resource informer (alias for `shard-selector`) |
386386
| `josdk.controller.<name>.informer.list-limit` | `Long` | Page size for paginated informer list requests; omit for no pagination |
387387
| `josdk.controller.<name>.informer.comparable-resource-versions` | `Boolean` | Whether the resource versions of the primary resource can be treated as integers and thus compared |
388+
| `josdk.controller.<name>.informer.without-namespace-index` | `Boolean` | Removes the namespace index the informer maintains; defaults to `false`. See [Removing the Namespace Index](../eventing#removing-the-namespace-index) |
388389

389390
#### Retry
390391

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,18 @@ public ControllerConfigurationOverrider<R> withInformerListLimit(Long informerLi
222222
return this;
223223
}
224224

225+
/**
226+
* Whether to remove the namespace index the underlying informer maintains by default. Note that
227+
* event sources that disagree on this setting do not share an informer.
228+
*
229+
* @param withoutNamespaceIndex true to remove the namespace index, false (the default) to keep it
230+
* @see io.javaoperatorsdk.operator.api.config.informer.Informer#withoutNamespaceIndex()
231+
*/
232+
public ControllerConfigurationOverrider<R> withoutNamespaceIndex(boolean withoutNamespaceIndex) {
233+
config.withoutNamespaceIndex(withoutNamespaceIndex);
234+
return this;
235+
}
236+
225237
public ControllerConfigurationOverrider<R> replacingNamedDependentResourceConfig(
226238
String name, Object dependentResourceConfig) {
227239

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/Informer.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import static io.javaoperatorsdk.operator.api.reconciler.Constants.DEFAULT_COMPARABLE_RESOURCE_VERSION;
3232
import static io.javaoperatorsdk.operator.api.reconciler.Constants.DEFAULT_FOLLOW_CONTROLLER_NAMESPACE_CHANGES;
3333
import static io.javaoperatorsdk.operator.api.reconciler.Constants.DEFAULT_GHOST_RESOURCE_CHECK_INTERVAL_MILLIS;
34+
import static io.javaoperatorsdk.operator.api.reconciler.Constants.DEFAULT_WITHOUT_NAMESPACE_INDEX;
3435
import static io.javaoperatorsdk.operator.api.reconciler.Constants.NO_LONG_VALUE_SET;
3536
import static io.javaoperatorsdk.operator.api.reconciler.Constants.NO_VALUE_SET;
3637

@@ -153,6 +154,23 @@
153154
*/
154155
boolean comparableResourceVersions() default DEFAULT_COMPARABLE_RESOURCE_VERSION;
155156

157+
/**
158+
* Whether to remove the namespace index that the underlying informer maintains by default.
159+
*
160+
* <p>The framework never reads that index, it only reads the indexes registered through {@link
161+
* io.javaoperatorsdk.operator.processing.event.source.IndexerResourceCache#addIndexers}, so
162+
* dropping it saves an entry per cached resource. It is worth setting when the informer caches a
163+
* large number of resources and nothing looks them up by namespace, in particular together with a
164+
* custom {@link #itemStore()} that keeps only a reduced form of each resource.
165+
*
166+
* <p>Note that this makes the informer a distinct one for pooling purposes: event sources that
167+
* disagree on this setting do not share an informer, because the index cannot be present for one
168+
* of them and absent for the other.
169+
*
170+
* @since 5.7.0
171+
*/
172+
boolean withoutNamespaceIndex() default DEFAULT_WITHOUT_NAMESPACE_INDEX;
173+
156174
/**
157175
* @deprecated Ghost resource checking is now triggered by the informer's onList callback. This
158176
* setting is no longer used.

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ public class InformerConfiguration<R extends HasMetadata> {
5858
private Long informerListLimit;
5959
private FieldSelector fieldSelector;
6060
private Boolean comparableResourceVersions;
61+
private Boolean withoutNamespaceIndex;
6162

6263
protected InformerConfiguration(
6364
Class<R> resourceClass,
@@ -75,6 +76,7 @@ protected InformerConfiguration(
7576
Long informerListLimit,
7677
FieldSelector fieldSelector,
7778
Boolean comparableResourceVersions,
79+
Boolean withoutNamespaceIndex,
7880
// TODO for removal in major release
7981
Duration ghostResourceCacheCheckInterval) {
8082
this(resourceClass, resourceGroupVersionKind);
@@ -91,6 +93,7 @@ protected InformerConfiguration(
9193
this.informerListLimit = informerListLimit;
9294
this.fieldSelector = fieldSelector;
9395
this.comparableResourceVersions = comparableResourceVersions;
96+
this.withoutNamespaceIndex = withoutNamespaceIndex;
9497
}
9598

9699
private InformerConfiguration(Class<R> resourceClass, GroupVersionKind resourceGroupVersionKind) {
@@ -140,6 +143,7 @@ public static <R extends HasMetadata> InformerConfiguration<R>.Builder builder(
140143
original.informerListLimit,
141144
original.fieldSelector,
142145
original.comparableResourceVersions,
146+
original.withoutNamespaceIndex,
143147
null)
144148
.builder;
145149
}
@@ -332,6 +336,15 @@ public boolean isComparableResourceVersions() {
332336
return comparableResourceVersions;
333337
}
334338

339+
/**
340+
* Whether the namespace index the underlying informer maintains by default is removed.
341+
*
342+
* @see Informer#withoutNamespaceIndex()
343+
*/
344+
public boolean isWithoutNamespaceIndex() {
345+
return withoutNamespaceIndex;
346+
}
347+
335348
@SuppressWarnings("UnusedReturnValue")
336349
public class Builder {
337350

@@ -349,6 +362,9 @@ public InformerConfiguration<R> buildForController() {
349362
if (comparableResourceVersions == null) {
350363
comparableResourceVersions = DEFAULT_COMPARABLE_RESOURCE_VERSION;
351364
}
365+
if (withoutNamespaceIndex == null) {
366+
withoutNamespaceIndex = DEFAULT_WITHOUT_NAMESPACE_INDEX;
367+
}
352368

353369
return InformerConfiguration.this;
354370
}
@@ -364,6 +380,9 @@ public InformerConfiguration<R> build() {
364380
if (comparableResourceVersions == null) {
365381
comparableResourceVersions = DEFAULT_COMPARABLE_RESOURCE_VERSION;
366382
}
383+
if (withoutNamespaceIndex == null) {
384+
withoutNamespaceIndex = DEFAULT_WITHOUT_NAMESPACE_INDEX;
385+
}
367386

368387
return InformerConfiguration.this;
369388
}
@@ -417,6 +436,7 @@ public InformerConfiguration<R>.Builder initFromAnnotation(
417436
.map(f -> new FieldSelector.Field(f.path(), f.value(), f.negated()))
418437
.toList()));
419438
withComparableResourceVersions(informerConfig.comparableResourceVersions());
439+
withoutNamespaceIndex(informerConfig.withoutNamespaceIndex());
420440
}
421441
return this;
422442
}
@@ -533,6 +553,16 @@ private static boolean isEmpty(FieldSelector fieldSelector) {
533553
|| fieldSelector.getFields().isEmpty();
534554
}
535555

556+
/**
557+
* Whether to remove the namespace index the underlying informer maintains by default.
558+
*
559+
* @see Informer#withoutNamespaceIndex()
560+
*/
561+
public Builder withoutNamespaceIndex(boolean withoutNamespaceIndex) {
562+
InformerConfiguration.this.withoutNamespaceIndex = withoutNamespaceIndex;
563+
return this;
564+
}
565+
536566
public Builder withComparableResourceVersions(boolean comparableResourceVersions) {
537567
InformerConfiguration.this.comparableResourceVersions = comparableResourceVersions;
538568
return this;

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerEventSourceConfiguration.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,11 @@ public Builder<R> withComparableResourceVersion(boolean comparableResourceVersio
292292
return this;
293293
}
294294

295+
public Builder<R> withoutNamespaceIndex(boolean withoutNamespaceIndex) {
296+
config.withoutNamespaceIndex(withoutNamespaceIndex);
297+
return this;
298+
}
299+
295300
@Deprecated(forRemoval = true)
296301
public Builder<R> withGhostResourceCacheCheckInterval(
297302
Duration ghostResourceCacheCheckInterval) {
@@ -317,6 +322,7 @@ public void updateFrom(InformerConfiguration<R> informerConfig) {
317322
.withGenericFilter(informerConfig.getGenericFilter())
318323
.withInformerListLimit(informerConfig.getInformerListLimit())
319324
.withComparableResourceVersions(informerConfig.isComparableResourceVersions())
325+
.withoutNamespaceIndex(informerConfig.isWithoutNamespaceIndex())
320326
.withFieldSelector(informerConfig.getFieldSelector());
321327
}
322328
}

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Constants.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ public final class Constants {
4343
public static final String CONTROLLER_NAME = "controller.name";
4444
public static final boolean DEFAULT_FOLLOW_CONTROLLER_NAMESPACE_CHANGES = true;
4545
public static final boolean DEFAULT_COMPARABLE_RESOURCE_VERSION = true;
46+
public static final boolean DEFAULT_WITHOUT_NAMESPACE_INDEX = false;
4647

4748
@Deprecated(forRemoval = true)
4849
public static final long DEFAULT_GHOST_RESOURCE_CHECK_INTERVAL_MILLIS = 3 * 60 * 1000;

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,8 @@ private InformerClassifier<R> getClassifier(String namespaceIdentifier) {
181181
configuration.getInformerConfig().getResourceGroupVersionKind(),
182182
configuration.getInformerConfig().getFieldSelector(),
183183
configuration.getInformerConfig().getInformerListLimit(),
184-
configuration.getInformerConfig().getItemStore());
184+
configuration.getInformerConfig().getItemStore(),
185+
configuration.getInformerConfig().isWithoutNamespaceIndex());
185186
}
186187

187188
private KubernetesClient getTargetClient() {

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/AbstractInformerPool.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public void setConfigurationService(ConfigurationService configurationService) {
7171
*/
7272
public abstract long numberOfInformersForResource(Class<? extends HasMetadata> resourceClass);
7373

74-
@SuppressWarnings({"rawtypes", "unchecked"})
74+
@SuppressWarnings({"rawtypes", "unchecked", "resource"})
7575
protected SharedIndexInformer createInformer(InformerClassifier<?> classifier) {
7676
var client = classifier.client();
7777

@@ -116,6 +116,11 @@ protected SharedIndexInformer createInformer(InformerClassifier<?> classifier) {
116116

117117
Optional.ofNullable(classifier.itemStore()).ifPresent(informer::itemStore);
118118

119+
if (classifier.withoutNamespaceIndex()) {
120+
// the framework only reads the indexes registered through the resource cache, never this one
121+
informer.removeNamespaceIndex();
122+
}
123+
119124
configurationService
120125
.getInformerStoppedHandler()
121126
.ifPresent(

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/DefaultInformerPool.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,14 @@ public <R extends HasMetadata> SharedIndexInformer<R> getInformer(
4343
synchronized (this) {
4444
var pooled = informers.get(classifier);
4545
if (pooled == null) {
46+
if (informers.keySet().stream()
47+
.anyMatch(existing -> existing.differsOnlyByNamespaceIndex(classifier))) {
48+
log.warn(
49+
"Creating a second informer for classifier {} that differs from an existing one only"
50+
+ " by withoutNamespaceIndex, so the resource type is cached twice. Set the"
51+
+ " option the same way on both to share one informer.",
52+
classifier);
53+
}
4654
informer = createInformer(classifier);
4755
informers.put(classifier, new PooledInformer(informer, new AtomicInteger(1)));
4856
log.debug(

0 commit comments

Comments
 (0)