Skip to content

Commit

Permalink
Avoid double-loading of resource objects
Browse files Browse the repository at this point in the history 
During recomputation the resource objects were (fully) loaded twice
in succession - during initial context load. This commit eliminates
that.

This resolves MID-7041.
  • Loading branch information
@mederly
mederly committed May 7, 2021
1 parent d4afef6 commit a158f3b
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 29 deletions.
2 changes: 2 additions & 0 deletions infra/schema/src/main/java/com/evolveum/midpoint/schema/GetOperationOptionsBuilder.java
View file
Expand Up @@ -52,6 +52,8 @@ public interface GetOperationOptionsBuilder {
GetOperationOptionsBuilder staleness(Long value);
GetOperationOptionsBuilder forceRefresh();
GetOperationOptionsBuilder forceRefresh(Boolean value);
GetOperationOptionsBuilder forceRetry();
GetOperationOptionsBuilder forceRetry(Boolean value);
GetOperationOptionsBuilder distinct();
GetOperationOptionsBuilder distinct(Boolean value);
GetOperationOptionsBuilder attachDiagData();
Expand Down
10 changes: 10 additions & 0 deletions infra/schema/src/main/java/com/evolveum/midpoint/schema/GetOperationOptionsBuilderImpl.java
View file
Expand Up @@ -201,6 +201,16 @@ public GetOperationOptionsBuilder forceRefresh(Boolean value) {
return forPaths(opts -> opts.setForceRefresh(value));
}

@Override
public GetOperationOptionsBuilder forceRetry() {
return forceRetry(true);
}

@Override
public GetOperationOptionsBuilder forceRetry(Boolean value) {
return forPaths(opts -> opts.setForceRetry(value));
}

@Override
public GetOperationOptionsBuilder distinct() {
return distinct(true);
Expand Down
70 changes: 43 additions & 27 deletions ...del-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/projector/ContextLoader.java
View file
Expand Up @@ -666,9 +666,9 @@ private <F extends FocusType> void loadLinkRefsFromFocus(LensContext<F> context,
LOGGER.trace("Inactive linkRef: will only refresh it, no processing. Relation={}, ref={}",
linkRefVal.getRelation(), linkRefVal);
// We do this just to restore old behavior that ensures that linked shadows are quick-refreshed,
// deleting e.g. expired pending operations (see TestMultiResource.test429).
var options = createGetOptions(context);
refreshLinkedShadow(oid, options, task, result);
// deleting e.g. expired pending operations (see TestMultiResource.test429). But only if full reconciliation
// is requested.
refreshLinkedShadow(oid, context, task, result);
continue;
}
if (StringUtils.isBlank(oid)) {
Expand All @@ -686,7 +686,7 @@ private <F extends FocusType> void loadLinkRefsFromFocus(LensContext<F> context,
//noinspection unchecked
PrismObject<ShadowType> shadowFromLink = linkRefVal.getObject();
if (shadowFromLink == null) {
var options = createGetOptions(context);
var options = createStandardGetOptions();
LOGGER.trace("Loading shadow {} from linkRef, options={}", oid, options);
try {
shadow = provisioningService.getObject(ShadowType.class, oid, options, task, result);
Expand Down Expand Up @@ -726,22 +726,27 @@ private <F extends FocusType> void loadLinkRefsFromFocus(LensContext<F> context,
}
}

private <F extends FocusType> Collection<SelectorOptions<GetOperationOptions>> createGetOptions(LensContext<F> context) {
GetOperationOptions rootOpts;
private <F extends FocusType> Collection<SelectorOptions<GetOperationOptions>> createStandardGetOptions() {
// Using NO_FETCH so we avoid reading in a full account. This is more efficient as we don't need full account here.
// We need to fetch from provisioning and not repository so the correct definition will be set.
return SchemaService.get().getOperationOptionsBuilder()
.noFetch()
.pointInTime(PointInTimeType.FUTURE)
.build();
}

private void refreshLinkedShadow(String oid, LensContext<?> context, Task task, OperationResult result) {
Collection<SelectorOptions<GetOperationOptions>> options;
if (context.isDoReconciliationForAllProjections()) {
rootOpts = GetOperationOptions.createForceRetry();
options = SchemaService.get().getOperationOptionsBuilder()
.forceRetry()
.build();
} else {
// Using NO_FETCH so we avoid reading in a full account. This is more efficient as we don't need full account here.
// We need to fetch from provisioning and not repository so the correct definition will be set.
rootOpts = GetOperationOptions.createNoFetch();
rootOpts.setPointInTimeType(PointInTimeType.FUTURE);
options = createStandardGetOptions();
}
return SelectorOptions.createCollection(rootOpts);
}

private void refreshLinkedShadow(String oid, Collection<SelectorOptions<GetOperationOptions>> options, Task task,
OperationResult result) {
try {
// TODO should we even do this if no reconciliation is requested?
// We call it with noFetch and we even ignore any exceptions.
provisioningService.getObject(ShadowType.class, oid, options, task, result);
} catch (Exception e) {
result.muteLastSubresultError();
Expand Down Expand Up @@ -1262,17 +1267,7 @@ private <F extends ObjectType> void finishLoadOfProjectionContext(LensContext<F>
"Projection "+projContext.getHumanReadableName()+" with null OID, no representation and no resource OID in account sync context "+projContext);
}
} else {
GetOperationOptions rootOptions = GetOperationOptions.createPointInTimeType(PointInTimeType.FUTURE);
if (projContext.isDoReconciliation()) {
rootOptions.setForceRefresh(true);
if (SchemaConstants.CHANNEL_DISCOVERY_URI.equals(context.getChannel())) {
// Avoid discovery loops
rootOptions.setDoNotDiscovery(true);
}
} else {
rootOptions.setNoFetch(true);
}
rootOptions.setAllowNotFound(true);
GetOperationOptions rootOptions = createRootOptions(context, projContext);
Collection<SelectorOptions<GetOperationOptions>> options = SelectorOptions.createCollection(rootOptions);
LOGGER.trace("Loading shadow {} for projection {}, options={}", projectionObjectOid, projContext.getHumanReadableName(), options);

Expand Down Expand Up @@ -1430,6 +1425,27 @@ private <F extends ObjectType> void finishLoadOfProjectionContext(LensContext<F>
setPrimaryDeltaOldValue(projContext);
}

@NotNull
private <F extends ObjectType> GetOperationOptions createRootOptions(LensContext<F> context,
LensProjectionContext projContext) {
GetOperationOptions rootOptions = GetOperationOptions.createPointInTimeType(PointInTimeType.FUTURE);
if (context.isDoReconciliationForAllProjections()) {
// Not sure why exactly in this way, but this is copied from existing code.
rootOptions.setForceRetry(true);
}
if (projContext.isDoReconciliation() || context.isDoReconciliationForAllProjections()) {
rootOptions.setForceRefresh(true);
if (SchemaConstants.CHANNEL_DISCOVERY_URI.equals(context.getChannel())) {
// Avoid discovery loops
rootOptions.setDoNotDiscovery(true);
}
} else {
rootOptions.setNoFetch(true);
}
rootOptions.setAllowNotFound(true);
return rootOptions;
}

private <F extends ObjectType> boolean needToReload(LensContext<F> context,
LensProjectionContext projContext) {
ResourceShadowDiscriminator discr = projContext.getResourceShadowDiscriminator();
Expand Down
4 changes: 2 additions & 2 deletions ...odel-intest/src/test/java/com/evolveum/midpoint/model/intest/orgstruct/TestOrgStruct.java
View file
Expand Up @@ -552,8 +552,8 @@ public void test232JackDestroyRefsAndRecompute() throws Exception {

// Why so many operations? But this is a very special case. As long as we do not see significant
// increase of operation count in normal scenarios we are quite OK.
assertCounterIncrement(InternalCounters.SHADOW_FETCH_OPERATION_COUNT, 4);
assertCounterIncrement(InternalCounters.CONNECTOR_OPERATION_COUNT, 8);
assertCounterIncrement(InternalCounters.SHADOW_FETCH_OPERATION_COUNT, 2);
assertCounterIncrement(InternalCounters.CONNECTOR_OPERATION_COUNT, 5);
}

/**
Expand Down

0 comments on commit a158f3b

Please sign in to comment.