Skip to content

fix: remove cache pruning #1694

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jan 5, 2023
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
24 changes: 0 additions & 24 deletions docs/documentation/features.md
Original file line number Diff line number Diff line change
Expand Up @@ -711,30 +711,6 @@ setting, where this flag usually needs to be set to false, in order to control t
See also an example implementation in the
[WebPage sample](https://github.com/java-operator-sdk/java-operator-sdk/blob/3e2e7c4c834ef1c409d636156b988125744ca911/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageOperator.java#L38-L43)

## Optimization of Caches

** Cache pruning is an experimental feature. Might a subject of change or even removal in the future. **

Operators using informers will initially cache the data for all known resources when starting up
so that access to resources can be performed quickly. Consequently, the memory required for the
operator to run and startup time will both increase quite dramatically when dealing with large
clusters with numerous resources.

It is thus possible to configure the operator to cache only pruned versions of the resources to
alleviate the memory usage of the primary and secondary caches. This setup, however, has
implications on how reconcilers deal with resources since they will only work with partial
objects. As a consequence, resources need to be updated using PATCH operations only, sending
only required changes.

To see how to use, and how to handle related caveats regarding how to deal with pruned objects
that leverage
[server side apply](https://kubernetes.io/docs/reference/using-api/server-side-apply/) patches,
please check the provided
[integration test](https://github.com/java-operator-sdk/java-operator-sdk/blob/c688524e64205690ba15587e7ed96a64dc231430/operator-framework/src/test/java/io/javaoperatorsdk/operator/CachePruneIT.java)
and associates reconciler.

Pruned caches are currently not supported with the Dependent Resources feature.

## Automatic Generation of CRDs

Note that this feature is provided by the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.function.Function;
import java.util.function.UnaryOperator;
import java.util.stream.Collectors;

import org.slf4j.Logger;
Expand Down Expand Up @@ -144,8 +143,6 @@ protected <P extends HasMetadata> ControllerConfiguration<P> configFor(Reconcile
Utils.contextFor(name, null, null)),
Utils.instantiate(annotation.genericFilter(), GenericFilter.class,
Utils.contextFor(name, null, null)),
Utils.instantiate(annotation.cachePruneFunction(), UnaryOperator.class,
Utils.contextFor(name, null, null)),
Set.of(valueOrDefault(annotation,
io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration::namespaces,
DEFAULT_NAMESPACES_SET.toArray(String[]::new))),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.UnaryOperator;

import io.fabric8.kubernetes.api.model.HasMetadata;
import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec;
Expand Down Expand Up @@ -36,7 +35,6 @@ public class ControllerConfigurationOverrider<R extends HasMetadata> {
private OnUpdateFilter<R> onUpdateFilter;
private GenericFilter<R> genericFilter;
private RateLimiter rateLimiter;
private UnaryOperator<R> cachePruneFunction;
private Map<DependentResourceSpec, Object> configurations;

private ControllerConfigurationOverrider(ControllerConfiguration<R> original) {
Expand All @@ -52,7 +50,6 @@ private ControllerConfigurationOverrider(ControllerConfiguration<R> original) {
this.genericFilter = original.genericFilter().orElse(null);
this.original = original;
this.rateLimiter = original.getRateLimiter();
this.cachePruneFunction = original.cachePruneFunction().orElse(null);
}

public ControllerConfigurationOverrider<R> withFinalizer(String finalizer) {
Expand Down Expand Up @@ -155,12 +152,6 @@ public ControllerConfigurationOverrider<R> withGenericFilter(GenericFilter<R> ge
return this;
}

public ControllerConfigurationOverrider<R> withCachePruneFunction(
UnaryOperator<R> cachePruneFunction) {
this.cachePruneFunction = cachePruneFunction;
return this;
}

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

Expand All @@ -182,7 +173,7 @@ public ControllerConfiguration<R> build() {
final var overridden = new ResolvedControllerConfiguration<>(
original.getResourceClass(), original.getName(),
generationAware, original.getAssociatedReconcilerClassName(), retry, rateLimiter,
reconciliationMaxInterval, onAddFilter, onUpdateFilter, genericFilter, cachePruneFunction,
reconciliationMaxInterval, onAddFilter, onUpdateFilter, genericFilter,
original.getDependentResources(),
namespaces, finalizer, labelSelector, configurations);
overridden.setEventFilter(customResourcePredicate);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import java.util.Optional;
import java.util.Set;
import java.util.function.UnaryOperator;

import io.fabric8.kubernetes.api.model.HasMetadata;
import io.javaoperatorsdk.operator.ReconcilerUtils;
Expand All @@ -20,12 +19,10 @@ public class DefaultResourceConfiguration<R extends HasMetadata>
private final GenericFilter<R> genericFilter;
private final String labelSelector;
private final Set<String> namespaces;
private final UnaryOperator<R> cachePruneFunction;

protected DefaultResourceConfiguration(Class<R> resourceClass,
Set<String> namespaces, String labelSelector, OnAddFilter<R> onAddFilter,
OnUpdateFilter<R> onUpdateFilter, GenericFilter<R> genericFilter,
UnaryOperator<R> cachePruneFunction) {
OnUpdateFilter<R> onUpdateFilter, GenericFilter<R> genericFilter) {
this.resourceClass = resourceClass;
this.resourceTypeName = ReconcilerUtils.getResourceTypeName(resourceClass);
this.onAddFilter = onAddFilter;
Expand All @@ -34,7 +31,6 @@ protected DefaultResourceConfiguration(Class<R> resourceClass,

this.namespaces = ResourceConfiguration.ensureValidNamespaces(namespaces);
this.labelSelector = ResourceConfiguration.ensureValidLabelSelector(labelSelector);
this.cachePruneFunction = cachePruneFunction;
}

@Override
Expand All @@ -52,11 +48,6 @@ public Set<String> getNamespaces() {
return namespaces;
}

@Override
public Optional<UnaryOperator<R>> cachePruneFunction() {
return Optional.ofNullable(this.cachePruneFunction);
}

@Override
public Class<R> getResourceClass() {
return resourceClass;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,8 @@
package io.javaoperatorsdk.operator.api.config;

import java.time.Duration;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.*;
import java.util.concurrent.TimeUnit;
import java.util.function.UnaryOperator;

import io.fabric8.kubernetes.api.model.HasMetadata;
import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceConfigurationProvider;
Expand Down Expand Up @@ -43,7 +38,7 @@ public ResolvedControllerConfiguration(Class<P> resourceClass, ControllerConfigu
other.getAssociatedReconcilerClassName(), other.getRetry(), other.getRateLimiter(),
other.maxReconciliationInterval().orElse(null),
other.onAddFilter().orElse(null), other.onUpdateFilter().orElse(null),
other.genericFilter().orElse(null), other.cachePruneFunction().orElse(null),
other.genericFilter().orElse(null),
other.getDependentResources(), other.getNamespaces(),
other.getFinalizerName(), other.getLabelSelector(), Collections.emptyMap());
}
Expand All @@ -69,12 +64,12 @@ public ResolvedControllerConfiguration(Class<P> resourceClass, String name,
boolean generationAware, String associatedReconcilerClassName, Retry retry,
RateLimiter rateLimiter, Duration maxReconciliationInterval,
OnAddFilter<P> onAddFilter, OnUpdateFilter<P> onUpdateFilter,
GenericFilter<P> genericFilter, UnaryOperator<P> cachePruneFunction,
GenericFilter<P> genericFilter,
List<DependentResourceSpec> dependentResources,
Set<String> namespaces, String finalizer, String labelSelector,
Map<DependentResourceSpec, Object> configurations) {
this(resourceClass, name, generationAware, associatedReconcilerClassName, retry, rateLimiter,
maxReconciliationInterval, onAddFilter, onUpdateFilter, genericFilter, cachePruneFunction,
maxReconciliationInterval, onAddFilter, onUpdateFilter, genericFilter,
namespaces, finalizer, labelSelector, configurations);
setDependentResources(dependentResources);
}
Expand All @@ -83,11 +78,9 @@ protected ResolvedControllerConfiguration(Class<P> resourceClass, String name,
boolean generationAware, String associatedReconcilerClassName, Retry retry,
RateLimiter rateLimiter, Duration maxReconciliationInterval,
OnAddFilter<P> onAddFilter, OnUpdateFilter<P> onUpdateFilter, GenericFilter<P> genericFilter,
UnaryOperator<P> cachePruneFunction,
Set<String> namespaces, String finalizer, String labelSelector,
Map<DependentResourceSpec, Object> configurations) {
super(resourceClass, namespaces, labelSelector, onAddFilter, onUpdateFilter, genericFilter,
cachePruneFunction);
super(resourceClass, namespaces, labelSelector, onAddFilter, onUpdateFilter, genericFilter);
this.name = ControllerConfiguration.ensureValidName(name, associatedReconcilerClassName);
this.generationAware = generationAware;
this.associatedReconcilerClassName = associatedReconcilerClassName;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,11 @@
import java.util.Collections;
import java.util.Optional;
import java.util.Set;
import java.util.function.UnaryOperator;

import io.fabric8.kubernetes.api.model.HasMetadata;
import io.javaoperatorsdk.operator.OperatorException;
import io.javaoperatorsdk.operator.ReconcilerUtils;
import io.javaoperatorsdk.operator.api.reconciler.Constants;
import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
import io.javaoperatorsdk.operator.processing.event.source.filter.GenericFilter;
import io.javaoperatorsdk.operator.processing.event.source.filter.OnAddFilter;
import io.javaoperatorsdk.operator.processing.event.source.filter.OnUpdateFilter;
Expand Down Expand Up @@ -124,11 +122,4 @@ default Set<String> getEffectiveNamespaces() {
}
return targetNamespaces;
}

/**
* See {@link ControllerConfiguration#cachePruneFunction()} for details.
*/
default Optional<UnaryOperator<R>> cachePruneFunction() {
return Optional.empty();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.function.UnaryOperator;

import io.fabric8.kubernetes.api.model.HasMetadata;
import io.javaoperatorsdk.operator.api.config.DefaultResourceConfiguration;
Expand Down Expand Up @@ -39,10 +38,8 @@ protected DefaultInformerConfiguration(String labelSelector,
OnAddFilter<R> onAddFilter,
OnUpdateFilter<R> onUpdateFilter,
OnDeleteFilter<R> onDeleteFilter,
GenericFilter<R> genericFilter,
UnaryOperator<R> cachePruneFunction) {
super(resourceClass, namespaces, labelSelector, onAddFilter, onUpdateFilter, genericFilter,
cachePruneFunction);
GenericFilter<R> genericFilter) {
super(resourceClass, namespaces, labelSelector, onAddFilter, onUpdateFilter, genericFilter);
this.followControllerNamespaceChanges = followControllerNamespaceChanges;

this.primaryToSecondaryMapper = primaryToSecondaryMapper;
Expand Down Expand Up @@ -106,7 +103,6 @@ class InformerConfigurationBuilder<R extends HasMetadata> {
private OnDeleteFilter<R> onDeleteFilter;
private GenericFilter<R> genericFilter;
private boolean inheritControllerNamespacesOnChange = false;
private UnaryOperator<R> cachePruneFunction;

private InformerConfigurationBuilder(Class<R> resourceClass) {
this.resourceClass = resourceClass;
Expand Down Expand Up @@ -207,18 +203,12 @@ public InformerConfigurationBuilder<R> withGenericFilter(GenericFilter<R> generi
return this;
}

public InformerConfigurationBuilder<R> withCachePruneFunction(
UnaryOperator<R> cachePruneFunction) {
this.cachePruneFunction = cachePruneFunction;
return this;
}

public InformerConfiguration<R> build() {
return new DefaultInformerConfiguration<>(labelSelector, resourceClass,
primaryToSecondaryMapper,
secondaryToPrimaryMapper,
namespaces, inheritControllerNamespacesOnChange, onAddFilter, onUpdateFilter,
onDeleteFilter, genericFilter, cachePruneFunction);
onDeleteFilter, genericFilter);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.util.function.UnaryOperator;

import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent;
import io.javaoperatorsdk.operator.processing.event.rate.LinearRateLimiter;
Expand Down Expand Up @@ -119,25 +118,4 @@ MaxReconciliationInterval maxReconciliationInterval() default @MaxReconciliation
* accessible no-arg constructor.
*/
Class<? extends RateLimiter> rateLimiter() default LinearRateLimiter.class;

/**
* <p>
* <b>This is an experimental feature, might be a subject of change and even removal in the
* future.</b>
* </p>
* <p>
* In order to optimize cache, thus set null on some attributes, this function can be set. Note
* that this has subtle implications how updates on the resources should be handled. Notably only
* patching of the resource can be used from that point, since update would remove not cached
* parts of the resource.
* </p>
* <p>
* Note that this feature does not work with Dependent Resources.
* </p>
*
*
*
* @return function to remove parts of the resource.
*/
Class<? extends UnaryOperator> cachePruneFunction() default UnaryOperator.class;
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
import io.fabric8.kubernetes.client.KubernetesClientException;
import io.fabric8.kubernetes.client.dsl.MixedOperation;
import io.fabric8.kubernetes.client.dsl.Resource;
import io.fabric8.kubernetes.client.dsl.base.PatchContext;
import io.fabric8.kubernetes.client.dsl.base.PatchType;
import io.javaoperatorsdk.operator.OperatorException;
import io.javaoperatorsdk.operator.api.ObservedGenerationAware;
import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider;
Expand All @@ -36,7 +34,7 @@
*/
class ReconciliationDispatcher<P extends HasMetadata> {

public static final int MAX_FINALIZER_REMOVAL_RETRY = 10;
public static final int MAX_UPDATE_RETRY = 10;

private static final Logger log = LoggerFactory.getLogger(ReconciliationDispatcher.class);

Expand Down Expand Up @@ -295,8 +293,7 @@ private PostExecutionControl<P> handleCleanup(P originalResource, P resource,
// cleanup is finished, nothing left to done
final var finalizerName = configuration().getFinalizerName();
if (deleteControl.isRemoveFinalizer() && resource.hasFinalizer(finalizerName)) {
P customResource = conflictRetryingPatch(resource, originalResource,
r -> r.removeFinalizer(finalizerName));
P customResource = conflictRetryingUpdate(resource, r -> r.removeFinalizer(finalizerName));
return PostExecutionControl.customResourceFinalizerRemoved(customResource);
}
}
Expand All @@ -315,8 +312,7 @@ private P updateCustomResourceWithFinalizer(P resourceForExecution, P originalRe
log.debug(
"Adding finalizer for resource: {} version: {}", getUID(originalResource),
getVersion(originalResource));

return conflictRetryingPatch(resourceForExecution, originalResource,
return conflictRetryingUpdate(resourceForExecution,
r -> r.addFinalizer(configuration().getFinalizerName()));
}

Expand All @@ -330,8 +326,7 @@ ControllerConfiguration<P> configuration() {
return controller.getConfiguration();
}

public P conflictRetryingPatch(P resource, P originalResource,
Function<P, Boolean> modificationFunction) {
public P conflictRetryingUpdate(P resource, Function<P, Boolean> modificationFunction) {
if (log.isDebugEnabled()) {
log.debug("Removing finalizer on resource: {}", ResourceID.fromResource(resource));
}
Expand All @@ -342,17 +337,17 @@ public P conflictRetryingPatch(P resource, P originalResource,
if (Boolean.FALSE.equals(modified)) {
return resource;
}
return customResourceFacade.serverSideApplyLockResource(resource, originalResource);
return customResourceFacade.updateResource(resource);
} catch (KubernetesClientException e) {
log.trace("Exception during patch for resource: {}", resource);
retryIndex++;
// only retry on conflict (HTTP 409), otherwise fail
if (e.getCode() != 409) {
throw e;
}
if (retryIndex >= MAX_FINALIZER_REMOVAL_RETRY) {
if (retryIndex >= MAX_UPDATE_RETRY) {
throw new OperatorException(
"Exceeded maximum (" + MAX_FINALIZER_REMOVAL_RETRY
"Exceeded maximum (" + MAX_UPDATE_RETRY
+ ") retry attempts to patch resource: "
+ ResourceID.fromResource(resource));
}
Expand Down Expand Up @@ -380,13 +375,6 @@ public R getResource(String namespace, String name) {
}
}

public R serverSideApplyLockResource(R resource, R originalResource) {
var patchContext = PatchContext.of(PatchType.SERVER_SIDE_APPLY);
patchContext.setForce(true);
return resource(originalResource).patch(patchContext,
resource);
}

public R updateResource(R resource) {
log.debug(
"Trying to replace resource {}, version: {}",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,6 @@ private InformerWrapper<T> createEventSource(
FilterWatchListDeletable<T, KubernetesResourceList<T>, Resource<T>> filteredBySelectorClient,
ResourceEventHandler<T> eventHandler, String namespaceIdentifier) {
var informer = filteredBySelectorClient.runnableInformer(0);
configuration.cachePruneFunction()
.ifPresent(f -> informer.itemStore(new TransformingItemStore<>(f)));
var source =
new InformerWrapper<>(informer, namespaceIdentifier);
source.addEventHandler(eventHandler);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ public abstract class ManagedInformerEventSource<R extends HasMetadata, P extend
protected ManagedInformerEventSource(
MixedOperation<R, KubernetesResourceList<R>, Resource<R>> client, C configuration) {
super(configuration.getResourceClass());
temporaryResourceCache = new TemporaryResourceCache<>(this,
configuration.cachePruneFunction().orElse(null));
temporaryResourceCache = new TemporaryResourceCache<>(this);
manager().initSources(client, configuration, this);
this.configuration = configuration;
}
Expand Down
Loading