Skip to content

Commit

Permalink
Avoid re-processing of completed projections
Browse files Browse the repository at this point in the history
The most important change is that we no longer clear
synchronizationPolicyDecision for completed projection contexts.
The primary motivation was to avoid loading deleted accounts but
the real consequences are much greater.

At some places (activation processing, assigned construction evaluation)
we added skipping when wave does not match and/or context is completed.

We also skip Projector.projectProjection if context is completed
(the wave check was already in place there).

When evaluating dependencies, we (maybe temporarily) relaxed
BROKEN/IGNORED check in wasProvisioned method. It was not functional
anyway, as synchronizationPolicyDecision was often cleared.

Related to MID-6228.
  • Loading branch information
mederly committed Sep 24, 2020
1 parent 081d4d8 commit 54a15cd
Show file tree
Hide file tree
Showing 19 changed files with 189 additions and 74 deletions.
Expand Up @@ -71,6 +71,8 @@ public static OpNode createOpNode(PrismContext prismContext, OperationResultType
return new FocusRepositoryLoadOpNode(prismContext, result, info, parent, traceInfo);
case FULL_PROJECTION_LOAD:
return new FullProjectionLoadOpNode(prismContext, result, info, parent, traceInfo);
case REPOSITORY_CACHE:
return new RepositoryCacheOpNode(prismContext, result, info, parent, traceInfo);
}
}
return new OpNode(prismContext, result, info, parent, traceInfo);
Expand Down
@@ -0,0 +1,19 @@
/*
* Copyright (c) 2020 Evolveum and contributors
*
* This work is dual-licensed under the Apache License 2.0
* and European Union Public License. See LICENSE file for details.
*/

package com.evolveum.midpoint.schema.traces;

import com.evolveum.midpoint.prism.PrismContext;
import com.evolveum.midpoint.xml.ns._public.common.common_3.OperationResultType;

public class RepositoryCacheOpNode extends RepositoryOpNode {

public RepositoryCacheOpNode(PrismContext prismContext,
OperationResultType result, OpResultInfo info, OpNode parent, TraceInfo traceInfo) {
super(prismContext, result, info, parent, traceInfo);
}
}
@@ -0,0 +1,30 @@
/*
* Copyright (c) 2020 Evolveum and contributors
*
* This work is dual-licensed under the Apache License 2.0
* and European Union Public License. See LICENSE file for details.
*/

package com.evolveum.midpoint.schema.traces;

import com.evolveum.midpoint.prism.PrismContext;
import com.evolveum.midpoint.xml.ns._public.common.common_3.OperationResultType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.RepositoryOperationTraceType;

/**
* General repository op (raw/cached, read/update, ...).
*/
public class RepositoryOpNode extends OpNode {

private final RepositoryOperationTraceType trace;

public RepositoryOpNode(PrismContext prismContext, OperationResultType result,
OpResultInfo info, OpNode parent, TraceInfo traceInfo) {
super(prismContext, result, info, parent, traceInfo);
trace = getTrace(RepositoryOperationTraceType.class);
}

public RepositoryOperationTraceType getTrace() {
return trace;
}
}
Expand Up @@ -347,6 +347,17 @@
</xsd:documentation>
</xsd:annotation>
</xsd:element>
<xsd:element name="completed" type="xsd:boolean" minOccurs="0">
<xsd:annotation>
<xsd:documentation>
Was the processing of this projection (in its execution wave) complete?
</xsd:documentation>
<xsd:appinfo>
<a:since>4.2</a:since>
<a:experimental>true</a:experimental>
</xsd:appinfo>
</xsd:annotation>
</xsd:element>
<xsd:element name="resourceShadowDiscriminator" type="c:ShadowDiscriminatorType" minOccurs="0">
<xsd:annotation>
<xsd:documentation>
Expand Down
Expand Up @@ -649,7 +649,6 @@ private <O extends ObjectType> void checkIndestructible(LensElementContext<O> el
}
}


private <F extends ObjectType> void processClockworkException(LensContext<F> context, Throwable e, Task task, OperationResult result, OperationResult overallResult)
throws SchemaException {
LOGGER.trace("Processing clockwork exception {}", e.toString());
Expand Down
Expand Up @@ -12,6 +12,7 @@
import com.evolveum.midpoint.model.impl.lens.projector.util.*;
import com.evolveum.midpoint.schema.cache.CacheConfigurationManager;
import com.evolveum.midpoint.task.api.Task;
import com.evolveum.midpoint.xml.ns._public.common.common_3.PartialProcessingOptionsType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.ProjectorComponentTraceType;

import org.springframework.beans.factory.annotation.Autowired;
Expand Down Expand Up @@ -211,7 +212,8 @@ private boolean shouldExecute(String componentName, ProjectorProcessor processor
return processorExecution == null ||
focusPresenceAndTypeCheckPasses(componentName, context, processorExecution)
&& focusDeletionCheckPasses(componentName, context.getFocusContext(), processorExecution)
&& projectionDeletionCheckPasses(componentName, projectionContext, processorExecution);
&& projectionDeletionCheckPasses(componentName, projectionContext, processorExecution)
&& projectionCurrentCheckPasses(componentName, projectionContext);
}

private boolean focusPresenceAndTypeCheckPasses(String componentName, LensContext<?> context,
Expand All @@ -235,7 +237,7 @@ private boolean focusPresenceAndTypeCheckPasses(String componentName, LensContex
}

private boolean focusDeletionCheckPasses(String componentName, LensFocusContext<?> focusContext,
ProcessorExecution processorExecution) throws SchemaException {
ProcessorExecution processorExecution) {
if (focusContext == null) {
// If we are OK with no focus context, then the deletion is irrelevant
return true;
Expand All @@ -262,6 +264,22 @@ private boolean projectionDeletionCheckPasses(String componentName, LensProjecti
}
}

/**
* Actually, in the current code (4.2) all methods that check this status are within
* {@link com.evolveum.midpoint.model.impl.lens.projector.Projector#projectProjection(LensContext, LensProjectionContext, PartialProcessingOptionsType, XMLGregorianCalendar, String, Task, OperationResult)}
* method - and it checks for both projection completed flag and its execution wave. Nevertheless, let us keep the check
* here for future use.
*/
@SuppressWarnings("JavadocReference")
private boolean projectionCurrentCheckPasses(String componentName, LensProjectionContext projectionContext) {
if (projectionContext != null && !projectionContext.isCurrentForProjection()) {
LOGGER.trace("Skipping '{}' because projection is not current (already completed or wrong wave)", componentName);
return false;
} else {
return true;
}
}

public void partialExecute(String baseComponentName, ProjectorComponentRunnable runnable,
Supplier<PartialProcessingTypeType> optionSupplier,
Class<?> executingClass, LensContext<?> context, LensProjectionContext projectionContext, OperationResult initialParentResult)
Expand Down Expand Up @@ -341,7 +359,7 @@ public void partialExecute(String baseComponentName, ProjectorComponentRunnable
}

public <F extends ObjectType> void traceContext(Trace logger, String activity, String phase,
boolean important, LensContext<F> context, boolean showTriples) throws SchemaException {
boolean important, LensContext<F> context, boolean showTriples) {
if (logger.isTraceEnabled()) {
logger.trace("Lens context:\n"+
"---[ {} context {} ]--------------------------------\n"+
Expand Down
Expand Up @@ -1557,6 +1557,9 @@ public boolean hasProjectionChange() {
if (!projectionContext.isCanProject()) {
continue;
}
if (projectionContext.isCompleted()) {
continue;
}
if (projectionContext.isTombstone()) {
continue;
}
Expand Down
Expand Up @@ -21,7 +21,6 @@
import com.evolveum.midpoint.common.refinery.*;
import com.evolveum.midpoint.model.impl.lens.construction.EvaluatedAssignedResourceObjectConstructionImpl;
import com.evolveum.midpoint.model.impl.lens.construction.PlainResourceObjectConstruction;
import com.evolveum.midpoint.model.impl.lens.construction.EvaluatedResourceObjectConstructionImpl;
import com.evolveum.midpoint.prism.*;
import com.evolveum.midpoint.prism.delta.*;
import com.evolveum.midpoint.prism.path.ItemPath;
Expand Down Expand Up @@ -114,6 +113,7 @@ public class LensProjectionContext extends LensElementContext<ShadowType> implem

/**
* True if the account should be part of the synchronization. E.g. outbound expression should be applied to it.
* TODO It looks like this is currently not used. Consider removing.
*/
private boolean isActive;

Expand Down Expand Up @@ -1014,13 +1014,22 @@ protected boolean isRequireSecondaryDeltaOid() {

@Override
public void cleanup() {
checkIfShouldArchive();

// We will clean up this projection context fully only if there's a chance we will touch it again.
if (!completed) {
synchronizationPolicyDecision = null;
isAssigned = null;
isActive = false;
}

// However, selected items are still cleaned up, in order to preserve existing behavior.
// This might be important e.g. for inbound mappings that take previous deltas into account.
secondaryDelta = null;
resetSynchronizationPolicyDecision();
// isLegal = null;
// isLegalOld = null;
isAssigned = null;
// isAssignedOld = false; // ??? [med]
isActive = false;

// isLegal = null;
// isLegalOld = null;
// isAssignedOld = false; // ??? [med]
}

@Override
Expand All @@ -1047,13 +1056,12 @@ public void normalize() {
// accountPasswordPolicy = null;
// }

protected void resetSynchronizationPolicyDecision() {
protected void checkIfShouldArchive() {
if (synchronizationPolicyDecision == SynchronizationPolicyDecision.DELETE || synchronizationPolicyDecision == SynchronizationPolicyDecision.UNLINK) {
toBeArchived = true;
} else if (synchronizationPolicyDecision != null) {
toBeArchived = false;
}
synchronizationPolicyDecision = null;
}

@Override
Expand Down Expand Up @@ -1342,6 +1350,7 @@ void addToPrismContainer(PrismContainer<LensProjectionContextType> lensProjectio
LensProjectionContextType lensProjectionContextType = lensProjectionContextTypeContainer.createNewValue().asContainerable();
super.storeIntoLensElementContextType(lensProjectionContextType, exportType);
lensProjectionContextType.setWave(wave);
lensProjectionContextType.setCompleted(completed);
lensProjectionContextType.setResourceShadowDiscriminator(resourceShadowDiscriminator != null ?
resourceShadowDiscriminator.toResourceShadowDiscriminatorType() : null);
lensProjectionContextType.setFullShadow(fullShadow);
Expand Down Expand Up @@ -1390,6 +1399,7 @@ public static LensProjectionContext fromLensProjectionContextType(LensProjection
projectionContext.fixProvisioningTypeInDelta(projectionContext.secondaryDelta, object, task, result);

projectionContext.wave = projectionContextType.getWave() != null ? projectionContextType.getWave() : 0;
projectionContext.completed = BooleanUtils.isTrue(projectionContextType.isCompleted());
projectionContext.fullShadow = projectionContextType.isFullShadow() != null ? projectionContextType.isFullShadow() : false;
projectionContext.isAssigned = projectionContextType.isIsAssigned() != null ? projectionContextType.isIsAssigned() : false;
projectionContext.isAssignedOld = projectionContextType.isIsAssignedOld() != null ? projectionContextType.isIsAssignedOld() : false;
Expand Down Expand Up @@ -1526,6 +1536,21 @@ boolean doesPrimaryDeltaApply() {
return true; // TODO is this OK?
}

/**
* @return True if the projection is "current" i.e. it was not completed and its wave is
* either not yet determined or equal to the current projection wave.
*/
@Experimental
public boolean isCurrentForProjection() {
if (completed) {
return false;
}
if (wave != -1 && wave != getLensContext().getProjectionWave()) {
return false;
}
return true;
}

public boolean isCompleted() {
return completed;
}
Expand Down
Expand Up @@ -19,7 +19,8 @@
/**
* Evaluation of an attribute mapping in resource object construction (assigned/plain).
*/
class AttributeEvaluation<AH extends AssignmentHolderType> extends ItemEvaluation<AH, PrismPropertyValue<?>, PrismPropertyDefinition<?>, RefinedAttributeDefinition<?>> {
class AttributeEvaluation<AH extends AssignmentHolderType>
extends ItemEvaluation<AH, PrismPropertyValue<?>, PrismPropertyDefinition<?>, RefinedAttributeDefinition<?>> {

AttributeEvaluation(ConstructionEvaluation<AH, ?> constructionEvaluation,
RefinedAttributeDefinition<?> refinedAttributeDefinition, MappingType mappingBean,
Expand Down
Expand Up @@ -13,6 +13,8 @@
import javax.xml.namespace.QName;

import com.evolveum.midpoint.prism.util.ObjectDeltaObject;
import com.evolveum.midpoint.util.logging.Trace;
import com.evolveum.midpoint.util.logging.TraceManager;
import com.evolveum.midpoint.xml.ns._public.common.common_3.*;

import com.google.common.annotations.VisibleForTesting;
Expand Down Expand Up @@ -44,6 +46,8 @@
public abstract class EvaluatedResourceObjectConstructionImpl<AH extends AssignmentHolderType, ROC extends ResourceObjectConstruction<AH, ?>>
implements EvaluatedAbstractConstruction<AH>, EvaluatedResourceObjectConstruction {

private static final Trace LOGGER = TraceManager.getTrace(EvaluatedResourceObjectConstructionImpl.class);

/**
* Parent construction to which this EvaluatedConstruction belongs.
*/
Expand Down Expand Up @@ -219,9 +223,15 @@ public NextRecompute evaluate(Task task, OperationResult result) throws Communic
throw new IllegalStateException("Attempting to evaluate an EvaluatedConstruction twice: " + this);
}
initializeProjectionContext();
evaluation = new ConstructionEvaluation<>(this, task, result);
evaluation.evaluate();
return evaluation.getNextRecompute();
if (projectionContext != null && !projectionContext.isCurrentForProjection()) {
LOGGER.trace("Skipping evaluation of construction for {} because this projection context is not current"
+ " (already completed or wrong wave)", projectionContext.getHumanReadableName());
return null;
} else {
evaluation = new ConstructionEvaluation<>(this, task, result);
evaluation.evaluate();
return evaluation.getNextRecompute();
}
}

/**
Expand Down
Expand Up @@ -123,6 +123,13 @@ private <O extends ObjectType, F extends FocusType> void processActivation(LensC
.addParam("projection", projectionContext.getHumanReadableName())
.build();
try {

if (!projectionContext.isCurrentForProjection()) {
LOGGER.trace("Projection {} is not current, skipping activation processing", projectionContext.getHumanReadableName());
result.recordNotApplicable();
return;
}

LensFocusContext<O> focusContext = context.getFocusContext();
if (focusContext == null || !FocusType.class.isAssignableFrom(focusContext.getObjectTypeClass())) {

Expand Down
Expand Up @@ -177,8 +177,9 @@ <F extends ObjectType> void load(LensContext<F> context, String activityDescript
} catch (Throwable e) {
projectionResult.recordFatalError(e);
throw e;
} finally {
projectionResult.computeStatusIfUnknown();
}
projectionResult.computeStatus();
}

context.checkConsistenceIfNeeded();
Expand Down Expand Up @@ -236,9 +237,7 @@ private <F extends ObjectType> void removeRottenContexts(LensContext<F> context)
continue;
}
if (!projectionContext.isFresh()) {
if (LOGGER.isTraceEnabled()) {
LOGGER.trace("Removing rotten context {}", projectionContext.getHumanReadableName());
}
LOGGER.trace("Removing rotten context {}", projectionContext.getHumanReadableName());

if (projectionContext.isToBeArchived()) {
context.getHistoricResourceObjects().add(projectionContext.getResourceShadowDiscriminator());
Expand Down Expand Up @@ -1173,9 +1172,15 @@ private <F extends ObjectType> void finishLoadOfProjectionContext(LensContext<F>
SecurityViolationException, ExpressionEvaluationException {

if (projContext.getSynchronizationPolicyDecision() == SynchronizationPolicyDecision.BROKEN) {
LOGGER.trace("Skipping loading of broken context {}", projContext.getHumanReadableName());
result.recordNotApplicable();
return;
}

// We could skip loading if the projection is completed, but it would cause problems e.g. in wasProvisioned
// method in dependency processor (it checks objectCurrent, among other things). So let's be conservative
// and load also completed projections.

// MID-2436 (volatile objects) - as a quick but effective hack, we set reconciliation:=TRUE for volatile accounts
ResourceObjectTypeDefinitionType objectDefinition = projContext.getResourceObjectTypeDefinitionType();
if (objectDefinition != null && objectDefinition.getVolatility() == ResourceObjectVolatilityType.UNPREDICTABLE && !projContext.isDoReconciliation()) {
Expand All @@ -1194,7 +1199,7 @@ private <F extends ObjectType> void finishLoadOfProjectionContext(LensContext<F>
boolean tombstone = false;
PrismObject<ShadowType> projectionObject = projContext.getObjectCurrent();
if (projContext.getObjectCurrent() == null || needToReload(context, projContext)) {
if (projContext.isAdd()) {
if (projContext.isAdd() && !projContext.isCompleted()) {
// No need to load old object, there is none
projContext.setExists(false);
projContext.recompute();
Expand All @@ -1219,17 +1224,13 @@ private <F extends ObjectType> void finishLoadOfProjectionContext(LensContext<F>
}
rootOptions.setAllowNotFound(true);
Collection<SelectorOptions<GetOperationOptions>> options = SelectorOptions.createCollection(rootOptions);
if (LOGGER.isTraceEnabled()) {
LOGGER.trace("Loading shadow {} for projection {}, options={}", projectionObjectOid, projContext.getHumanReadableName(), options);
}
LOGGER.trace("Loading shadow {} for projection {}, options={}", projectionObjectOid, projContext.getHumanReadableName(), options);

try {
PrismObject<ShadowType> objectOld = provisioningService.getObject(
projContext.getObjectTypeClass(), projectionObjectOid, options, task, result);
if (LOGGER.isTraceEnabled()) {
if (!GetOperationOptions.isNoFetch(rootOptions) && !GetOperationOptions.isRaw(rootOptions)) {
LOGGER.trace("Full shadow loaded for {}:\n{}", projContext.getHumanReadableName(), objectOld.debugDump(1));
}
if (!GetOperationOptions.isNoFetch(rootOptions) && !GetOperationOptions.isRaw(rootOptions)) {
LOGGER.trace("Full shadow loaded for {}:\n{}", projContext.getHumanReadableName(), objectOld.debugDumpLazily(1));
}
Validate.notNull(objectOld.getOid());
if (InternalsConfig.consistencyChecks) {
Expand Down

0 comments on commit 54a15cd

Please sign in to comment.