Skip to content

Commit

Permalink
Improve thread-safety in thresholds and fix tests
Browse files Browse the repository at this point in the history 
Added some synchronized sections and atomic incrementing
of counters. Fixed collecting of task assignments. Weakened
(fixed) asserts so now they seem to pass.
  • Loading branch information
@mederly
mederly committed Apr 1, 2020
1 parent 92368c6 commit 9921e3d
Show file tree
Hide file tree
Showing 11 changed files with 139 additions and 160 deletions.
4 changes: 2 additions & 2 deletions .../main/java/com/evolveum/midpoint/model/impl/lens/projector/SmartAssignmentCollection.java
View file
Expand Up @@ -50,7 +50,7 @@ public class SmartAssignmentCollection<F extends AssignmentHolderType> implement
private Map<Long, SmartAssignmentElement> idMap;

public void collect(PrismObject<F> objectCurrent, PrismObject<F> objectOld, ContainerDelta<AssignmentType> assignmentDelta,
Collection<AssignmentType> forcedAssignments, AssignmentType taskAssignment) throws SchemaException {
Collection<AssignmentType> forcedAssignments, Collection<AssignmentType> taskAssignments) throws SchemaException {
PrismContainer<AssignmentType> assignmentContainerCurrent;
if (objectCurrent != null) {
assignmentContainerCurrent = objectCurrent.findContainer(FocusType.F_ASSIGNMENT);
Expand All @@ -68,7 +68,7 @@ public void collect(PrismObject<F> objectCurrent, PrismObject<F> objectOld, Cont

collectVirtualAssignments(forcedAssignments);

if (taskAssignment != null) {
for (AssignmentType taskAssignment : taskAssignments) {
//noinspection unchecked
collectAssignment(taskAssignment.asPrismContainerValue(), Mode.CURRENT, true, null);
}
Expand Down
8 changes: 1 addition & 7 deletions ...java/com/evolveum/midpoint/model/impl/lens/projector/focus/AssignmentTripleEvaluator.java
View file
Expand Up @@ -207,13 +207,7 @@ public DeltaSetTriple<EvaluatedAssignmentImpl<AH>> processAllAssignments() throw

LOGGER.trace("Task assignment: {}", taskAssignments);

if (taskAssignments.isEmpty()) {
assignmentCollection.collect(focusContext.getObjectCurrent(), focusContext.getObjectOld(), assignmentDelta, forcedAssignments, null);
}

for (AssignmentType taskAssignment : taskAssignments) {
assignmentCollection.collect(focusContext.getObjectCurrent(), focusContext.getObjectOld(), assignmentDelta, forcedAssignments, taskAssignment);
}
assignmentCollection.collect(focusContext.getObjectCurrent(), focusContext.getObjectOld(), assignmentDelta, forcedAssignments, taskAssignments);

LOGGER.trace("Assignment collection:\n{}", assignmentCollection.debugDumpLazily(1));

Expand Down
44 changes: 15 additions & 29 deletions ...com/evolveum/midpoint/model/impl/lens/projector/policy/PolicyRuleSuspendTaskExecutor.java
View file
Expand Up @@ -44,41 +44,27 @@ public <O extends ObjectType> void execute(@NotNull ModelContext<O> context, Tas
}

String id = task.getTaskTreeId(result);

for (EvaluatedPolicyRule policyRule : focusCtx.getPolicyRules()) {
CounterSpecification counterSpec = counterManager.getCounterSpec(id, policyRule.getPolicyRuleIdentifier(), policyRule.getPolicyRule());
LOGGER.trace("Found counter specification {} for {}", counterSpec, DebugUtil.debugDumpLazily(policyRule));

int counter = 1;
if (counterSpec != null) {
counter = counterSpec.getCount();
}
counter = checkEvaluatedPolicyRule(task, policyRule, counter, result);

if (counterSpec != null) {
LOGGER.trace("Setting new count = {} to counter spec", counter);
counterSpec.setCount(counter);
}
if (id == null) {
LOGGER.trace("No persistent task context, no counting!");
return;
}

}

private synchronized int checkEvaluatedPolicyRule(Task task, EvaluatedPolicyRule policyRule, int counter, OperationResult result) throws ThresholdPolicyViolationException, ObjectNotFoundException, SchemaException {
if (policyRule.containsEnabledAction(SuspendTaskPolicyActionType.class)) {
counter++;
LOGGER.trace("Suspend task action enabled for {}, checking threshold settings", DebugUtil.debugDumpLazily(policyRule));
PolicyThresholdType thresholdSettings = policyRule.getPolicyThreshold();
if (isOverThreshold(thresholdSettings, counter)) {
throw new ThresholdPolicyViolationException("Policy rule violation: " + policyRule.getPolicyRule());
for (EvaluatedPolicyRule policyRule : focusCtx.getPolicyRules()) {
// In theory we could count events also for other kinds of actions (not only SuspendTask)
if (policyRule.containsEnabledAction(SuspendTaskPolicyActionType.class)) {
CounterSpecification counterSpec = counterManager.getCounterSpec(id, policyRule.getPolicyRuleIdentifier(), policyRule.getPolicyRule());
LOGGER.trace("Created/found counter specification {} for:\n{}", counterSpec, DebugUtil.debugDumpLazily(policyRule));
int countAfter = counterSpec.incrementAndGet();
if (isOverThreshold(policyRule.getPolicyThreshold(), countAfter)) {
throw new ThresholdPolicyViolationException("Policy rule violation: " + policyRule.getPolicyRule());
}
}
}

return counter;
}

private boolean isOverThreshold(PolicyThresholdType thresholdSettings, int counter) throws SchemaException {
// TODO: better implementation that takes hight water mark into account
WaterMarkType lowWaterMark = thresholdSettings.getLowWaterMark();
// TODO: better implementation that takes high water mark into account
WaterMarkType lowWaterMark = thresholdSettings != null ? thresholdSettings.getLowWaterMark() : null;
if (lowWaterMark == null) {
LOGGER.trace("No low water mark defined.");
return true;
Expand All @@ -87,7 +73,7 @@ private boolean isOverThreshold(PolicyThresholdType thresholdSettings, int count
if (lowWaterCount == null) {
throw new SchemaException("No count in low water mark in a policy rule");
}
return (counter >= lowWaterCount);
return counter >= lowWaterCount;
}
}

Expand Down
5 changes: 4 additions & 1 deletion repo/repo-api/src/main/java/com/evolveum/midpoint/repo/api/CounterManager.java
View file
Expand Up @@ -9,15 +9,18 @@
import java.util.Collection;

import com.evolveum.midpoint.xml.ns._public.common.common_3.PolicyRuleType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.TaskType;

import org.jetbrains.annotations.Contract;

/**
* @author katka
*
*/
public interface CounterManager {

@Contract("!null, _, _ -> !null; null, _, _ -> null")
CounterSpecification getCounterSpec(String taskId, String policyRuleId, PolicyRuleType policyRule);

void cleanupCounters(String oid);
Collection<CounterSpecification> listCounters();
void removeCounter(CounterSpecification counterSpecification);
Expand Down
29 changes: 23 additions & 6 deletions repo/repo-api/src/main/java/com/evolveum/midpoint/repo/api/CounterSpecification.java
View file
Expand Up @@ -11,13 +11,15 @@
import com.evolveum.midpoint.xml.ns._public.common.common_3.PolicyRuleType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.PolicyThresholdType;

import java.util.concurrent.atomic.AtomicInteger;

/**
* @author katka
*
*/
public class CounterSpecification implements DebugDumpable {

private int count = 0;
private final AtomicInteger count = new AtomicInteger(0);
private long counterStart;

private String oid;
Expand All @@ -31,14 +33,17 @@ public CounterSpecification(String oid, String policyRuleId, PolicyRuleType poli
}

public int getCount() {
return count;
return count.intValue();
}

public long getCounterStart() {
return counterStart;
}
public void setCount(int count) {
this.count = count;

public int incrementAndGet() {
return count.incrementAndGet();
}

public void setCounterStart(long counterStart) {
this.counterStart = counterStart;
}
Expand All @@ -59,12 +64,15 @@ public String getPolicyRuleId() {
return policyRuleId;
}


public void reset(long currentTimeMillis) {
count = 0;
resetCount();
counterStart = currentTimeMillis;
}

public void resetCount() {
count.set(0);
}

@Override
public String debugDump(int indent) {
StringBuilder sb = new StringBuilder();
Expand All @@ -76,4 +84,13 @@ public String debugDump(int indent) {
return sb.toString();
}

@Override
public String toString() {
return "CounterSpecification{" +
"oid='" + oid + '\'' +
", policyRuleId='" + policyRuleId + '\'' +
", count=" + count +
", counterStart=" + counterStart +
'}';
}
}

0 comments on commit 9921e3d

Please sign in to comment.