Skip to content

Commit

Permalink
Optimise rollout create - single save + groups saveAll
Browse files Browse the repository at this point in the history
Signed-off-by: Avgustin Marinov <Avgustin.Marinov@bosch.com>
  • Loading branch information
avgustinmm committed Dec 6, 2024
1 parent 9d67202 commit f1c9bc1
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 74 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ public interface RolloutManagement {
* exceeded.
*/
@PreAuthorize(SpringEvalExpressions.HAS_AUTH_ROLLOUT_MANAGEMENT_CREATE)
Rollout create(@NotNull @Valid RolloutCreate create, int amountGroup, boolean confirmationRequired,
Rollout create(
@NotNull @Valid RolloutCreate create, int amountGroup, boolean confirmationRequired,
@NotNull RolloutGroupConditions conditions, DynamicRolloutGroupTemplate dynamicRolloutGroupTemplate);

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -850,7 +850,7 @@ private Long createActionsForTargetsInNewTransaction(final Rollout rollout, fina

if (targets.getNumberOfElements() > 0) {
final DistributionSet distributionSet = rollout.getDistributionSet();
entityManager.detach(distributionSet); // if lazy loaded with different session
entityManager.detach(distributionSet); // LAZY_LOAD - if lazy loaded with different session
final ActionType actionType = rollout.getActionType();
final long forceTime = rollout.getForcedTime();
createActions(targets.getContent(), distributionSet, actionType, forceTime, rollout, group);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,26 +132,26 @@ public Page<RolloutGroup> findByRolloutWithDetailedStatus(final Pageable pageabl
}

@Override
public Page<TargetWithActionStatus> findAllTargetsOfRolloutGroupWithActionStatus(final Pageable pageRequest,
final long rolloutGroupId) {

public Page<TargetWithActionStatus> findAllTargetsOfRolloutGroupWithActionStatus(final Pageable pageRequest, final long rolloutGroupId) {
throwExceptionIfRolloutGroupDoesNotExist(rolloutGroupId);

final CriteriaBuilder cb = entityManager.getCriteriaBuilder();
final CriteriaQuery<Object[]> query = cb.createQuery(Object[].class);
final Root<RolloutTargetGroup> targetRoot = query.distinct(true).from(RolloutTargetGroup.class);
final Join<RolloutTargetGroup, JpaTarget> targetJoin = targetRoot.join(RolloutTargetGroup_.target);
final ListJoin<RolloutTargetGroup, JpaAction> actionJoin = targetRoot.join(RolloutTargetGroup_.actions,
JoinType.LEFT);
final ListJoin<RolloutTargetGroup, JpaAction> actionJoin = targetRoot.join(RolloutTargetGroup_.actions, JoinType.LEFT);

final CriteriaQuery<Object[]> multiselect = query
.multiselect(targetJoin, actionJoin.get(JpaAction_.status),
actionJoin.get(JpaAction_.lastActionStatusCode))
.multiselect(targetJoin, actionJoin.get(JpaAction_.status), actionJoin.get(JpaAction_.lastActionStatusCode))
.where(getRolloutGroupTargetWithRolloutGroupJoinCondition(rolloutGroupId, cb, targetRoot))
.orderBy(getOrderBy(pageRequest, cb, targetJoin, actionJoin));
final List<TargetWithActionStatus> targetWithActionStatus = entityManager.createQuery(multiselect)
.setFirstResult((int) pageRequest.getOffset()).setMaxResults(pageRequest.getPageSize()).getResultList()
.stream().map(this::getTargetWithActionStatusFromQuery).collect(Collectors.toList());
.setFirstResult((int) pageRequest.getOffset())
.setMaxResults(pageRequest.getPageSize())
.getResultList()
.stream()
.map(this::getTargetWithActionStatusFromQuery)
.toList();

return new PageImpl<>(targetWithActionStatus, pageRequest, 0);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,8 @@ public long countByDistributionSetIdAndRolloutIsStoppable(final long setId) {
@Transactional
@Retryable(retryFor = { ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX,
backoff = @Backoff(delay = Constants.TX_RT_DELAY))
public Rollout create(final RolloutCreate rollout, final int amountGroup, final boolean confirmationRequired,
public Rollout create(
final RolloutCreate rollout, final int amountGroup, final boolean confirmationRequired,
final RolloutGroupConditions conditions, final DynamicRolloutGroupTemplate dynamicRolloutGroupTemplate) {
if (amountGroup < 0) {
throw new ValidationException("The amount of groups cannot be lower than or equal to zero for static rollouts");
Expand All @@ -201,8 +202,8 @@ public Rollout create(final RolloutCreate rollout, final int amountGroup, final
throw new ValidationException("Dynamic group template is only allowed for dynamic rollouts");
}

final JpaRollout savedRollout = createRollout(rolloutRequest, amountGroup == 0);
return createRolloutGroups(amountGroup, conditions, savedRollout, confirmationRequired, dynamicRolloutGroupTemplate);
return createRolloutGroups(
amountGroup, conditions, createRollout(rolloutRequest, amountGroup == 0), confirmationRequired, dynamicRolloutGroupTemplate);
}

@Override
Expand All @@ -224,9 +225,7 @@ public Rollout create(final RolloutCreate rollout, final List<RolloutGroupCreate
throw new ValidationException("The amount of groups cannot be 0");
}
RolloutHelper.verifyRolloutGroupAmount(groups.size(), quotaManagement);
final JpaRollout rolloutRequest = (JpaRollout) rollout.build();
final JpaRollout savedRollout = createRollout(rolloutRequest, false);
return createRolloutGroups(groups, conditions, savedRollout);
return createRolloutGroups(groups, conditions, createRollout((JpaRollout)rollout.build(), false));
}

@Override
Expand Down Expand Up @@ -571,15 +570,17 @@ private JpaRollout createRollout(final JpaRollout rollout, final boolean pureDyn
rollout.setWeight(repositoryProperties.getActionWeightIfAbsent());
}
contextAware.getCurrentContext().ifPresent(rollout::setAccessControlContext);
return rolloutRepository.save(rollout);
return rollout;
}

private Rollout createRolloutGroups(final int amountOfGroups, final RolloutGroupConditions conditions,
private Rollout createRolloutGroups(
final int amountOfGroups, final RolloutGroupConditions conditions,
final JpaRollout rollout, final boolean isConfirmationRequired, final DynamicRolloutGroupTemplate dynamicRolloutGroupTemplate) {
RolloutHelper.verifyRolloutInStatus(rollout, RolloutStatus.CREATING);
RolloutHelper.verifyRolloutGroupConditions(conditions);

JpaRolloutGroup lastSavedGroup = null;
final List<JpaRolloutGroup> groups = new ArrayList<>();
JpaRolloutGroup lastGroup = null;
if (amountOfGroups == 0) {
if (dynamicRolloutGroupTemplate == null) {
throw new ConstraintDeclarationException(
Expand All @@ -596,24 +597,21 @@ private Rollout createRolloutGroups(final int amountOfGroups, final RolloutGroup
group.setName(nameAndDesc);
group.setDescription(nameAndDesc);
group.setRollout(rollout);
group.setParent(lastSavedGroup);
group.setParent(lastGroup);
group.setStatus(RolloutGroupStatus.CREATING);
group.setConfirmationRequired(isConfirmationRequired);

addSuccessAndErrorConditionsAndActions(group, conditions);

// total percent of the all devices. Before, it was relative percent -
// the percent of the "rest" of the devices. Thus, if you have
// first a group 10% (the rest is 90%) and the second group is 50%
// then the percent would be 50% of 90% - 45%.
// This is very unintuitive and is switched in order to be interpreted easier.
// the "new style" (vs "old style") rollouts could be detected by
// JpaRollout#isNewStyleTargetPercent (which uses that old style rollouts
// have null as dynamic
// total percent of the all devices. Before, it was relative percent - the percent of the "rest" of the devices. Thus,
// if you have first a group 10% (the rest is 90%) and the second group is 50% then the percent would be 50% of 90% - 45%.
// This is very unintuitive and is switched in order to be interpreted easier. The "new style" (vs "old style") rollouts could
// be detected by JpaRollout#isNewStyleTargetPercent (which uses that old style rollouts have null as dynamic
group.setTargetPercentage(100.0F / amountOfGroups);

lastSavedGroup = rolloutGroupRepository.save(group);
publishRolloutGroupCreatedEventAfterCommit(lastSavedGroup, rollout);
groups.add(group);
lastGroup = group;
publishRolloutGroupCreatedEventAfterCommit(lastGroup, rollout);
}
}

Expand All @@ -624,7 +622,7 @@ private Rollout createRolloutGroups(final int amountOfGroups, final RolloutGroup
group.setName(nameAndDesc);
group.setDescription(nameAndDesc);
group.setRollout(rollout);
group.setParent(lastSavedGroup);
group.setParent(lastGroup);
group.setDynamic(true);
group.setStatus(RolloutGroupStatus.READY);
group.setConfirmationRequired(isConfirmationRequired);
Expand All @@ -634,44 +632,49 @@ private Rollout createRolloutGroups(final int amountOfGroups, final RolloutGroup
// for dynamic groups the target count is kept in target percentage
group.setTargetPercentage(dynamicRolloutGroupTemplate.getTargetCount());

lastSavedGroup = rolloutGroupRepository.save(group);
publishRolloutGroupCreatedEventAfterCommit(lastSavedGroup, rollout);
groups.add(group);
lastGroup = group;
publishRolloutGroupCreatedEventAfterCommit(lastGroup, rollout);
}

// lastSavedGroup is never null! amountOfGroups > 0 (and has static groups) or dynamicRolloutGroupTemplate is
// not null (validated) and (validated) the rollout is dynamic, so has dynamic group
rollout.setRolloutGroupsCreated(lastSavedGroup.isDynamic() ? amountOfGroups + 1 : amountOfGroups);
return rolloutRepository.save(rollout);
rollout.setRolloutGroupsCreated(lastGroup.isDynamic() ? amountOfGroups + 1 : amountOfGroups);
final JpaRollout savedRollout = rolloutRepository.save(rollout);
rolloutGroupRepository.saveAll(groups);
return savedRollout;
}

private Rollout createRolloutGroups(final List<RolloutGroupCreate> groupList,
final RolloutGroupConditions conditions, final Rollout rollout) {
private Rollout createRolloutGroups(
final List<RolloutGroupCreate> groupList, final RolloutGroupConditions conditions, final JpaRollout rollout) {
RolloutHelper.verifyRolloutInStatus(rollout, RolloutStatus.CREATING);
final JpaRollout savedRollout = (JpaRollout) rollout;
final DistributionSetType distributionSetType = savedRollout.getDistributionSet().getType();
final DistributionSetType distributionSetType = rollout.getDistributionSet().getType();

// prepare the groups
final List<RolloutGroup> groups = groupList.stream()
.map(group -> prepareRolloutGroupWithDefaultConditions(group, conditions)).collect(Collectors.toList());
groups.forEach(RolloutHelper::verifyRolloutGroupHasConditions);
final List<RolloutGroup> srcGroups = groupList.stream()
.map(group -> prepareRolloutGroupWithDefaultConditions(group, conditions))
.collect(Collectors.toList());
srcGroups.forEach(RolloutHelper::verifyRolloutGroupHasConditions);

RolloutHelper.verifyRemainingTargets(calculateRemainingTargets(groups, savedRollout.getTargetFilterQuery(),
savedRollout.getCreatedAt(), distributionSetType.getId()));
RolloutHelper.verifyRemainingTargets(calculateRemainingTargets(
srcGroups, rollout.getTargetFilterQuery(), rollout.getCreatedAt(), distributionSetType.getId()));

// check if we need to enforce the 'max targets per group' quota
if (quotaManagement.getMaxTargetsPerRolloutGroup() > 0) {
validateTargetsInGroups(groups, savedRollout.getTargetFilterQuery(), savedRollout.getCreatedAt(),
validateTargetsInGroups(
srcGroups, rollout.getTargetFilterQuery(), rollout.getCreatedAt(),
distributionSetType.getId()).getTargetsPerGroup().forEach(this::assertTargetsPerRolloutGroupQuota);
}

// create and persist the groups (w/o filling them with targets)
JpaRolloutGroup lastSavedGroup = null;
for (final RolloutGroup srcGroup : groups) {
final List<JpaRolloutGroup> groups = new ArrayList<>();
JpaRolloutGroup lastGroup = null;
for (final RolloutGroup srcGroup : srcGroups) {
final JpaRolloutGroup group = new JpaRolloutGroup();
group.setName(srcGroup.getName());
group.setDescription(srcGroup.getDescription());
group.setRollout(savedRollout);
group.setParent(lastSavedGroup);
group.setRollout(rollout);
group.setParent(lastGroup);
group.setStatus(RolloutGroupStatus.CREATING);
group.setConfirmationRequired(srcGroup.isConfirmationRequired());

Expand All @@ -687,12 +690,16 @@ private Rollout createRolloutGroups(final List<RolloutGroupCreate> groupList,
srcGroup.getErrorCondition(), srcGroup.getErrorConditionExp(), srcGroup.getErrorAction(),
srcGroup.getErrorActionExp());

lastSavedGroup = rolloutGroupRepository.save(group);
publishRolloutGroupCreatedEventAfterCommit(lastSavedGroup, rollout);
groups.add(group);
lastGroup = group;
publishRolloutGroupCreatedEventAfterCommit(lastGroup, rollout);
}

savedRollout.setRolloutGroupsCreated(groups.size());
return rolloutRepository.save(savedRollout);
rollout.setRolloutGroupsCreated(groups.size());

final JpaRollout savedRollout = rolloutRepository.save(rollout);
rolloutGroupRepository.saveAll(groups);
return savedRollout;
}

private JpaRollout getRolloutOrThrowExceptionIfNotFound(final Long rolloutId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,35 +59,27 @@ void nonExistingEntityAccessReturnsNotPresent() {
@Description("Verifies that management queries react as specified on calls for non existing entities " +
" by means of throwing EntityNotFoundException.")
@ExpectEvents({
@Expect(type = RolloutDeletedEvent.class, count = 0),
@Expect(type = RolloutCreatedEvent.class, count = 1),
@Expect(type = RolloutUpdatedEvent.class, count = 1),
@Expect(type = RolloutGroupCreatedEvent.class, count = 5),
@Expect(type = RolloutGroupUpdatedEvent.class, count = 5),
@Expect(type = RolloutDeletedEvent.class, count = 0),
@Expect(type = DistributionSetCreatedEvent.class, count = 1),
@Expect(type = SoftwareModuleCreatedEvent.class, count = 3),
@Expect(type = DistributionSetUpdatedEvent.class, count = 1), // implicit lock
@Expect(type = SoftwareModuleUpdatedEvent.class, count = 3), // implicit lock
@Expect(type = RolloutUpdatedEvent.class, count = 1),
@Expect(type = TargetCreatedEvent.class, count = 125),
@Expect(type = RolloutCreatedEvent.class, count = 1) })
@Expect(type = TargetCreatedEvent.class, count = 125) })
void entityQueriesReferringToNotExistingEntitiesThrowsException() {
testdataFactory.createRollout();

verifyThrownExceptionBy(() -> rolloutGroupManagement.countByRollout(NOT_EXIST_IDL), "Rollout");
verifyThrownExceptionBy(() -> rolloutGroupManagement.countTargetsOfRolloutsGroup(NOT_EXIST_IDL),
"RolloutGroup");
verifyThrownExceptionBy(() -> rolloutGroupManagement.findByRolloutWithDetailedStatus(PAGE, NOT_EXIST_IDL),
"Rollout");
verifyThrownExceptionBy(
() -> rolloutGroupManagement.findAllTargetsOfRolloutGroupWithActionStatus(PAGE, NOT_EXIST_IDL),
"RolloutGroup");
verifyThrownExceptionBy(() -> rolloutGroupManagement.findByRolloutAndRsql(PAGE, NOT_EXIST_IDL, "name==*"),
"Rollout");
verifyThrownExceptionBy(() -> rolloutGroupManagement.countTargetsOfRolloutsGroup(NOT_EXIST_IDL), "RolloutGroup");
verifyThrownExceptionBy(() -> rolloutGroupManagement.findByRolloutWithDetailedStatus(PAGE, NOT_EXIST_IDL), "Rollout");
verifyThrownExceptionBy(() -> rolloutGroupManagement.findAllTargetsOfRolloutGroupWithActionStatus(PAGE, NOT_EXIST_IDL), "RolloutGroup");
verifyThrownExceptionBy(() -> rolloutGroupManagement.findByRolloutAndRsql(PAGE, NOT_EXIST_IDL, "name==*"), "Rollout");

verifyThrownExceptionBy(() -> rolloutGroupManagement.findTargetsOfRolloutGroup(PAGE, NOT_EXIST_IDL),
"RolloutGroup");
verifyThrownExceptionBy(
() -> rolloutGroupManagement.findTargetsOfRolloutGroupByRsql(PAGE, NOT_EXIST_IDL, "name==*"),
"RolloutGroup");
verifyThrownExceptionBy(() -> rolloutGroupManagement.findTargetsOfRolloutGroup(PAGE, NOT_EXIST_IDL), "RolloutGroup");
verifyThrownExceptionBy(() -> rolloutGroupManagement.findTargetsOfRolloutGroupByRsql(PAGE, NOT_EXIST_IDL, "name==*"), "RolloutGroup");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1051,8 +1051,13 @@ public Rollout createRolloutByVariables(final String rolloutName, final String r
.errorAction(RolloutGroupErrorAction.PAUSE, null).build();

final Rollout rollout = rolloutManagement.create(
entityFactory.rollout().create().name(rolloutName).description(rolloutDescription)
.targetFilterQuery(filterQuery).distributionSetId(distributionSet).actionType(actionType).weight(weight)
entityFactory.rollout().create()
.name(rolloutName)
.description(rolloutDescription)
.targetFilterQuery(filterQuery)
.distributionSetId(distributionSet)
.actionType(actionType)
.weight(weight)
.dynamic(dynamic),
groupSize, confirmationRequired, conditions, dynamicRolloutGroupTemplate);

Expand Down

0 comments on commit f1c9bc1

Please sign in to comment.