Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion docs/content/en/docs/documentation/eventing.md
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,30 @@ See
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)
for more details.

### Removing the Namespace Index

Informers keep an index from namespace to the resources cached for it. JOSDK never reads that index,
only the ones registered explicitly through `IndexerResourceCache.addIndexers(..)`, so it can be
removed to save an entry per cached resource:

```java
@ControllerConfiguration(informer = @Informer(withoutNamespaceIndex = true))
public class MyReconciler implements Reconciler<MyCustomResource> { }
```

The same option is available on `InformerEventSourceConfiguration.Builder` for event sources, and on
`InformerConfiguration.Builder`.

This matters most for informers that cache a large number of resources, and in particular together
with a custom [item store](#bounded-caches-for-informers) that keeps only a reduced form of each
resource: the index is keyed independently of what the store does with the resource itself, so
shrinking what is cached does not shrink the index.

Note that this takes part in the informer pool identity described below: an event source that
removes the index does not share an informer with one that keeps it. If two event sources watch the
same resource type and only one of them sets the option, they end up with two informers, and two
caches of that resource type, which can cost far more memory than the index ever did.

### Sharing Informers Between Controllers (Informer Pool)

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

The `informerListLimit` is intentionally *not* part of this identity: if two otherwise-equivalent
event sources request a different list limit, the existing informer is reused (a warning is logged
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,7 @@ All controller-level keys are prefixed with `josdk.controller.<controller-name>.
| `josdk.controller.<name>.informer.label-selector` | `String` | Label selector for the primary resource informer (alias for `label-selector`) |
| `josdk.controller.<name>.informer.shard-selector` | `String` | Shard selector for the primary resource informer (alias for `shard-selector`) |
| `josdk.controller.<name>.informer.list-limit` | `Long` | Page size for paginated informer list requests; omit for no pagination |
| `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) |

#### Retry

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,18 @@ public ControllerConfigurationOverrider<R> withInformerListLimit(Long informerLi
return this;
}

/**
* Whether to remove the namespace index the underlying informer maintains by default. Note that
* event sources that disagree on this setting do not share an informer.
*
* @param withoutNamespaceIndex true to remove the namespace index, false (the default) to keep it
* @see io.javaoperatorsdk.operator.api.config.informer.Informer#withoutNamespaceIndex()
*/
public ControllerConfigurationOverrider<R> withoutNamespaceIndex(boolean withoutNamespaceIndex) {
config.withoutNamespaceIndex(withoutNamespaceIndex);
return this;
}

public ControllerConfigurationOverrider<R> replacingNamedDependentResourceConfig(
String name, Object dependentResourceConfig) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import static io.javaoperatorsdk.operator.api.reconciler.Constants.DEFAULT_COMPARABLE_RESOURCE_VERSION;
import static io.javaoperatorsdk.operator.api.reconciler.Constants.DEFAULT_FOLLOW_CONTROLLER_NAMESPACE_CHANGES;
import static io.javaoperatorsdk.operator.api.reconciler.Constants.DEFAULT_GHOST_RESOURCE_CHECK_INTERVAL_MILLIS;
import static io.javaoperatorsdk.operator.api.reconciler.Constants.DEFAULT_WITHOUT_NAMESPACE_INDEX;
import static io.javaoperatorsdk.operator.api.reconciler.Constants.NO_LONG_VALUE_SET;
import static io.javaoperatorsdk.operator.api.reconciler.Constants.NO_VALUE_SET;

Expand Down Expand Up @@ -153,6 +154,23 @@
*/
boolean comparableResourceVersions() default DEFAULT_COMPARABLE_RESOURCE_VERSION;

/**
* Whether to remove the namespace index that the underlying informer maintains by default.
*
* <p>The framework never reads that index, it only reads the indexes registered through {@link
* io.javaoperatorsdk.operator.processing.event.source.IndexerResourceCache#addIndexers}, so
* dropping it saves an entry per cached resource. It is worth setting when the informer caches a
* large number of resources and nothing looks them up by namespace, in particular together with a
* custom {@link #itemStore()} that keeps only a reduced form of each resource.
*
* <p>Note that this makes the informer a distinct one for pooling purposes: event sources that
* disagree on this setting do not share an informer, because the index cannot be present for one
* of them and absent for the other.
*
* @since 5.7.0
*/
boolean withoutNamespaceIndex() default DEFAULT_WITHOUT_NAMESPACE_INDEX;

/**
* @deprecated Ghost resource checking is now triggered by the informer's onList callback. This
* setting is no longer used.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ public class InformerConfiguration<R extends HasMetadata> {
private Long informerListLimit;
private FieldSelector fieldSelector;
private Boolean comparableResourceVersions;
private Boolean withoutNamespaceIndex;

protected InformerConfiguration(
Class<R> resourceClass,
Expand All @@ -75,6 +76,7 @@ protected InformerConfiguration(
Long informerListLimit,
FieldSelector fieldSelector,
Boolean comparableResourceVersions,
Boolean withoutNamespaceIndex,
Comment thread
csviri marked this conversation as resolved.
// TODO for removal in major release
Duration ghostResourceCacheCheckInterval) {
this(resourceClass, resourceGroupVersionKind);
Expand All @@ -91,6 +93,7 @@ protected InformerConfiguration(
this.informerListLimit = informerListLimit;
this.fieldSelector = fieldSelector;
this.comparableResourceVersions = comparableResourceVersions;
this.withoutNamespaceIndex = withoutNamespaceIndex;
}

private InformerConfiguration(Class<R> resourceClass, GroupVersionKind resourceGroupVersionKind) {
Expand Down Expand Up @@ -140,6 +143,7 @@ public static <R extends HasMetadata> InformerConfiguration<R>.Builder builder(
original.informerListLimit,
original.fieldSelector,
original.comparableResourceVersions,
original.withoutNamespaceIndex,
null)
.builder;
}
Expand Down Expand Up @@ -332,6 +336,15 @@ public boolean isComparableResourceVersions() {
return comparableResourceVersions;
}

/**
* Whether the namespace index the underlying informer maintains by default is removed.
*
* @see Informer#withoutNamespaceIndex()
*/
public boolean isWithoutNamespaceIndex() {
return withoutNamespaceIndex;
}

@SuppressWarnings("UnusedReturnValue")
public class Builder {

Expand All @@ -349,6 +362,9 @@ public InformerConfiguration<R> buildForController() {
if (comparableResourceVersions == null) {
comparableResourceVersions = DEFAULT_COMPARABLE_RESOURCE_VERSION;
}
if (withoutNamespaceIndex == null) {
withoutNamespaceIndex = DEFAULT_WITHOUT_NAMESPACE_INDEX;
}

return InformerConfiguration.this;
}
Expand All @@ -364,6 +380,9 @@ public InformerConfiguration<R> build() {
if (comparableResourceVersions == null) {
comparableResourceVersions = DEFAULT_COMPARABLE_RESOURCE_VERSION;
}
if (withoutNamespaceIndex == null) {
withoutNamespaceIndex = DEFAULT_WITHOUT_NAMESPACE_INDEX;
}

return InformerConfiguration.this;
}
Expand Down Expand Up @@ -417,6 +436,7 @@ public InformerConfiguration<R>.Builder initFromAnnotation(
.map(f -> new FieldSelector.Field(f.path(), f.value(), f.negated()))
.toList()));
withComparableResourceVersions(informerConfig.comparableResourceVersions());
withoutNamespaceIndex(informerConfig.withoutNamespaceIndex());
}
return this;
}
Expand Down Expand Up @@ -533,6 +553,16 @@ private static boolean isEmpty(FieldSelector fieldSelector) {
|| fieldSelector.getFields().isEmpty();
}

/**
* Whether to remove the namespace index the underlying informer maintains by default.
*
* @see Informer#withoutNamespaceIndex()
*/
public Builder withoutNamespaceIndex(boolean withoutNamespaceIndex) {
InformerConfiguration.this.withoutNamespaceIndex = withoutNamespaceIndex;
return this;
}

public Builder withComparableResourceVersions(boolean comparableResourceVersions) {
InformerConfiguration.this.comparableResourceVersions = comparableResourceVersions;
return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,11 @@ public Builder<R> withComparableResourceVersion(boolean comparableResourceVersio
return this;
}

public Builder<R> withoutNamespaceIndex(boolean withoutNamespaceIndex) {
config.withoutNamespaceIndex(withoutNamespaceIndex);
return this;
}

@Deprecated(forRemoval = true)
public Builder<R> withGhostResourceCacheCheckInterval(
Duration ghostResourceCacheCheckInterval) {
Expand All @@ -317,6 +322,7 @@ public void updateFrom(InformerConfiguration<R> informerConfig) {
.withGenericFilter(informerConfig.getGenericFilter())
.withInformerListLimit(informerConfig.getInformerListLimit())
.withComparableResourceVersions(informerConfig.isComparableResourceVersions())
.withoutNamespaceIndex(informerConfig.isWithoutNamespaceIndex())
.withFieldSelector(informerConfig.getFieldSelector());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public final class Constants {
public static final String CONTROLLER_NAME = "controller.name";
public static final boolean DEFAULT_FOLLOW_CONTROLLER_NAMESPACE_CHANGES = true;
public static final boolean DEFAULT_COMPARABLE_RESOURCE_VERSION = true;
public static final boolean DEFAULT_WITHOUT_NAMESPACE_INDEX = false;

@Deprecated(forRemoval = true)
public static final long DEFAULT_GHOST_RESOURCE_CHECK_INTERVAL_MILLIS = 3 * 60 * 1000;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,8 @@ private InformerClassifier<R> getClassifier(String namespaceIdentifier) {
configuration.getInformerConfig().getResourceGroupVersionKind(),
configuration.getInformerConfig().getFieldSelector(),
configuration.getInformerConfig().getInformerListLimit(),
configuration.getInformerConfig().getItemStore());
configuration.getInformerConfig().getItemStore(),
configuration.getInformerConfig().isWithoutNamespaceIndex());
}

private KubernetesClient getTargetClient() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public void setConfigurationService(ConfigurationService configurationService) {
*/
public abstract long numberOfInformersForResource(Class<? extends HasMetadata> resourceClass);

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

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

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

if (classifier.withoutNamespaceIndex()) {
// the framework only reads the indexes registered through the resource cache, never this one
informer.removeNamespaceIndex();
}

configurationService
.getInformerStoppedHandler()
.ifPresent(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,14 @@ public <R extends HasMetadata> SharedIndexInformer<R> getInformer(
synchronized (this) {
var pooled = informers.get(classifier);
if (pooled == null) {
if (informers.keySet().stream()
.anyMatch(existing -> existing.differsOnlyByNamespaceIndex(classifier))) {
log.warn(
"Creating a second informer for classifier {} that differs from an existing one only"
+ " by withoutNamespaceIndex, so the resource type is cached twice. Set the"
+ " option the same way on both to share one informer.",
classifier);
}
informer = createInformer(classifier);
informers.put(classifier, new PooledInformer(informer, new AtomicInteger(1)));
log.debug(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@
* limit still share an informer; the limit of whichever classifier created the informer is
* kept (a pool is expected to warn about this, see {@link
* #differsOnlyByInformerListLimit(InformerClassifier)}).
* <li>{@link #withoutNamespaceIndex()} on the other hand <strong>is</strong> part of the
* identity: the namespace index cannot be present for one event source and absent for another
* that shares the same informer, so event sources disagreeing on it get separate informers.
* <li>Indexers are not part of the classifier at all: they are registered on the informer under a
* name qualified with the event source that added them, so those of different event sources
* can live side by side on a shared informer without colliding.
Expand All @@ -61,7 +64,8 @@ public record InformerClassifier<R extends HasMetadata>(
GroupVersionKind groupVersionKind,
FieldSelector fieldSelector,
Long informerListLimit,
ItemStore<R> itemStore) {
ItemStore<R> itemStore,
boolean withoutNamespaceIndex) {

@Override
public boolean equals(Object o) {
Expand All @@ -71,6 +75,16 @@ public boolean equals(Object o) {
if (!(o instanceof InformerClassifier<?> that)) {
return false;
}
return equalsIgnoringNamespaceIndex(that)
&& withoutNamespaceIndex == that.withoutNamespaceIndex;
}

/**
* Equality of everything the identity is made of except {@link #withoutNamespaceIndex()}, so that
* {@link #equals(Object)} and {@link #differsOnlyByNamespaceIndex(InformerClassifier)} cannot
* drift apart when a component is added.
*/
private boolean equalsIgnoringNamespaceIndex(InformerClassifier<?> that) {
return client == that.client
&& Objects.equals(labelSelector, that.labelSelector)
&& Objects.equals(shardSelector, that.shardSelector)
Expand All @@ -91,7 +105,8 @@ public int hashCode() {
resourceClass,
groupVersionKind,
fieldSelector,
itemStore);
itemStore,
withoutNamespaceIndex);
}

/**
Expand Down Expand Up @@ -123,6 +138,8 @@ public String toString() {
+ informerListLimit
+ ", itemStore="
+ itemStore
+ ", withoutNamespaceIndex="
+ withoutNamespaceIndex
+ "]";
}

Expand All @@ -140,4 +157,14 @@ private String masterUrl() {
public boolean differsOnlyByInformerListLimit(InformerClassifier<?> other) {
return equals(other) && !Objects.equals(informerListLimit, other.informerListLimit);
}

/**
* Checks whether this classifier and the other are equal in every attribute except for {@link
* #withoutNamespaceIndex()}, which differs between them. Unlike the list limit, that setting is
* part of the identity, so such a pair is served by two informers rather than one.
*/
public boolean differsOnlyByNamespaceIndex(InformerClassifier<?> other) {
return equalsIgnoringNamespaceIndex(other)
&& withoutNamespaceIndex != other.withoutNamespaceIndex;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,28 @@ void nullLabelSelectorByDefault() {
assertNull(informerConfig.getLabelSelector());
}

@Test
void keepsTheNamespaceIndexByDefault() {
final var informerConfig = InformerConfiguration.builder(ConfigMap.class).build();
assertFalse(informerConfig.isWithoutNamespaceIndex());
}

@Test
void keepsTheNamespaceIndexByDefaultForController() {
final var informerConfig = InformerConfiguration.builder(ConfigMap.class).buildForController();
assertFalse(informerConfig.isWithoutNamespaceIndex());
}

@Test
void withoutNamespaceIndexIsCarriedOverWhenCopyingTheConfiguration() {
final var original =
InformerConfiguration.builder(ConfigMap.class).withoutNamespaceIndex(true).build();

final var copy = InformerConfiguration.builder(original).build();

assertTrue(copy.isWithoutNamespaceIndex());
}

@Test
void nullShardSelectorByDefault() {
final var informerConfig = InformerConfiguration.builder(ConfigMap.class).build();
Expand Down
Loading
Loading