Skip to content

Commit

Permalink
Merge pull request #3376 from candlepin/crog/4.1/2069160
Browse files Browse the repository at this point in the history
[4.1] 2069160: Added grace period for product deletion during refresh/import (ENT-4881)
  • Loading branch information
nikosmoum committed Jun 3, 2022
2 parents e537c5f + f245ef5 commit 2a9ebd9
Show file tree
Hide file tree
Showing 12 changed files with 1,189 additions and 43 deletions.
5 changes: 5 additions & 0 deletions src/main/java/org/candlepin/config/ConfigProperties.java
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,10 @@ private ConfigProperties() {
// How long (in seconds) to wait for job threads to finish during a graceful Tomcat shutdown
public static final String ASYNC_JOBS_THREAD_SHUTDOWN_TIMEOUT = "candlepin.async.thread.shutdown.timeout";

// How many days to allow a product to be orphaned before removing it on the next refresh or
// manifest import. Default: 30 days
public static final String ORPHANED_ENTITY_GRACE_PERIOD = "candlepin.refresh.orphan_entity_grace_period";

/**
* Fetches a string representing the prefix for all per-job configuration for the specified job.
* The job key or class name may be used, but the usage must be consistent.
Expand Down Expand Up @@ -487,6 +491,7 @@ public static String jobConfig(String jobKey, String cfgName) {
// Set the triggerable jobs list
this.put(ASYNC_JOBS_TRIGGERABLE_JOBS, String.join(", ", ASYNC_JOBS_TRIGGERABLE_JOBS_LIST));

this.put(ORPHANED_ENTITY_GRACE_PERIOD, "30");
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,8 @@ void refreshPoolsWithRegeneration(SubscriptionServiceAdapter subAdapter,
Owner resolvedOwner = this.resolveOwner(owner);
log.info("Refreshing pools for owner: {}", resolvedOwner);

RefreshWorker refresher = this.refreshWorkerProvider.get();
RefreshWorker refresher = this.refreshWorkerProvider.get()
.setOrphanedEntityGracePeriod(this.config.getInt(ConfigProperties.ORPHANED_ENTITY_GRACE_PERIOD));

log.debug("Fetching subscriptions from adapter...");
refresher.addSubscriptions(subAdapter.getSubscriptions(resolvedOwner.getKey()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,17 +70,21 @@ public class RefreshWorker {
*/
private static final int VERSIONING_CONSTRAINT_VIOLATION_RETRIES = 4;

/** Default grace period for orphaned entities */
private static final int ORPHANED_ENTITY_DEFAULT_GRACE_PERIOD = -1;

private PoolCurator poolCurator;
private ContentCurator contentCurator;
private OwnerContentCurator ownerContentCurator;
private OwnerProductCurator ownerProductCurator;
private ProductCurator productCurator;
private final PoolCurator poolCurator;
private final ContentCurator contentCurator;
private final OwnerContentCurator ownerContentCurator;
private final OwnerProductCurator ownerProductCurator;
private final ProductCurator productCurator;

private PoolMapper poolMapper;
private ProductMapper productMapper;
private ContentMapper contentMapper;

private int orphanedEntityGracePeriod;


/**
* Creates a new RefreshWorker
Expand All @@ -99,6 +103,8 @@ public RefreshWorker(PoolCurator poolCurator, ProductCurator productCurator,
this.poolMapper = new PoolMapper();
this.productMapper = new ProductMapper();
this.contentMapper = new ContentMapper();

this.orphanedEntityGracePeriod = ORPHANED_ENTITY_DEFAULT_GRACE_PERIOD;
}

/**
Expand All @@ -110,6 +116,35 @@ public void clear() {
this.contentMapper.clear();
}

/**
* Sets the orphaned entity grace period, which determines how long a given entity must be
* orphaned before it will be deleted as part of the cleanup step. The behavior of entity
* cleanup will differ depending on this value:
* <ul>
* <li>
* If the grace period is a positive integer, entities which have been orphaned for more
* than the number of days will be removed
* </li>
* <li>
* If the grace period is zero, entities which are orphaned will be cleaned up immediately
* </li>
* <li>
* If the grace period is a negative integer, orphaned entities will not be removed at all
* </li>
* </ul>
*
* @param period
* the orphaned entity grace period in days, or a negative value to disable orphaned entity
* cleanup
*
* @return
* a reference to this refresh worker
*/
public RefreshWorker setOrphanedEntityGracePeriod(int period) {
this.orphanedEntityGracePeriod = period;
return this;
}

/**
* Adds the specified subscriptions to this refresher. If a given subscription has already
* been added, but differs from the existing version, a warning will be generated and the
Expand Down Expand Up @@ -390,6 +425,18 @@ public RefreshWorker addContent(Collection<? extends ContentInfo> contents) {
return this;
}

/**
* Fetches the current orphaned entity grace period for this refresher. See the documentation
* associated with the setOrphanedEntityGracePeriod method for details on the meaning of
* specific values.
*
* @return
* the current orphaned entity grace period for this refresher
*/
public int getOrphanedEntityGracePeriod() {
return this.orphanedEntityGracePeriod;
}

/**
* Fetches the compiled set of subscriptions to import, mapped by subscription ID. If no subscriptions
* have been added, this method returns an empty map.
Expand Down Expand Up @@ -429,6 +476,7 @@ public RefreshWorker addContent(Collection<? extends ContentInfo> contents) {
* @return
* the result of this refresh operation
*/
@SuppressWarnings("indentation")
public RefreshResult execute(Owner owner) {
Transactional<RefreshResult> block = this.poolCurator.transactional((args) -> {
NodeMapper nodeMapper = new NodeMapper();
Expand All @@ -445,7 +493,8 @@ public RefreshResult execute(Owner owner) {
NodeProcessor nodeProcessor = new NodeProcessor()
.setNodeMapper(nodeMapper)
.addVisitor(new PoolNodeVisitor(this.poolCurator))
.addVisitor(new ProductNodeVisitor(this.productCurator, this.ownerProductCurator))
.addVisitor(new ProductNodeVisitor(this.productCurator, this.ownerProductCurator,
this.orphanedEntityGracePeriod))
.addVisitor(new ContentNodeVisitor(this.contentCurator, this.ownerContentCurator));

// Obtain system locks on products and content so we don't need to worry about
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.time.Instant;
import java.time.temporal.ChronoUnit;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
Expand All @@ -50,6 +52,7 @@ public class ProductNodeVisitor implements NodeVisitor<Product, ProductInfo> {

private final ProductCurator productCurator;
private final OwnerProductCurator ownerProductCurator;
private final int orphanProductGracePeriod;

// Various cross-stage cache collections
private Set<OwnerProduct> ownerProductEntities;
Expand All @@ -58,6 +61,12 @@ public class ProductNodeVisitor implements NodeVisitor<Product, ProductInfo> {
private Map<Owner, Set<Long>> ownerEntityVersions;
private Map<Owner, Map<String, List<Product>>> ownerVersionedEntityMap;

// Orphan date tracking and grace period state caching
private Map<Owner, Map<String, Instant>> ownerOrphanedDateMap;
private Map<Owner, Set<String>> ownerOrphanEntityIdPrecache;
private Map<Owner, Set<String>> ownerOrphanedEntities;
private Map<Owner, Set<String>> ownerUnorphanedEntities;


/**
* Creates a new ProductNodeVisitor that uses the provided curators for performing database
Expand All @@ -69,10 +78,15 @@ public class ProductNodeVisitor implements NodeVisitor<Product, ProductInfo> {
* @param ownerProductCurator
* the OwnerProductCurator to use for owner-product database operations
*
* @param orphanProductGracePeriod
* the number of days a product is allowed to be orphaned before it will be removed
*
* @throws IllegalArgumentException
* if any of the provided curators are null
*/
public ProductNodeVisitor(ProductCurator productCurator, OwnerProductCurator ownerProductCurator) {
public ProductNodeVisitor(ProductCurator productCurator, OwnerProductCurator ownerProductCurator,
int orphanProductGracePeriod) {

if (productCurator == null) {
throw new IllegalArgumentException("productCurator is null");
}
Expand All @@ -83,12 +97,18 @@ public ProductNodeVisitor(ProductCurator productCurator, OwnerProductCurator own

this.productCurator = productCurator;
this.ownerProductCurator = ownerProductCurator;
this.orphanProductGracePeriod = orphanProductGracePeriod;

this.ownerProductEntities = new HashSet<>();
this.ownerProductUuidMap = new HashMap<>();
this.deletedProductUuids = new HashMap<>();
this.ownerEntityVersions = new HashMap<>();
this.ownerVersionedEntityMap = new HashMap<>();

this.ownerOrphanedDateMap = new HashMap<>();
this.ownerOrphanEntityIdPrecache = new HashMap<>();
this.ownerOrphanedEntities = new HashMap<>();
this.ownerUnorphanedEntities = new HashMap<>();
}

/**
Expand Down Expand Up @@ -122,6 +142,10 @@ public void processNode(EntityNode<Product, ProductInfo> node) {
node.setNodeState(NodeState.UNCHANGED);

if (existingEntity != null) {
// Cache the ID of any existing entities for later bulk fetching orphan dates
this.ownerOrphanEntityIdPrecache.computeIfAbsent(node.getOwner(), key -> new HashSet<>())
.add(node.getEntityId());

if (importedEntity != null) {
nodeChanged = ProductManager.isChangedBy(existingEntity, importedEntity);
}
Expand Down Expand Up @@ -168,20 +192,95 @@ public void pruneNode(EntityNode<Product, ProductInfo> node) {
*/
private boolean clearedForDeletion(EntityNode<Product, ProductInfo> node) {
// We don't delete custom entities, ever.
if (!node.getExistingEntity().isLocked()) {
return false;
}
boolean cleared = node.getExistingEntity().isLocked();

// If the node is still defined upstream and is part of this refresh, we should keep it
// around locally
if (node.getImportedEntity() != null) {
return false;
}
cleared = cleared && node.getImportedEntity() == null;

// Otherwise, if the node is referenced by one or more parent nodes that are not being
// deleted themselves, we should keep it.
return !node.getParentNodes()
// If the node is referenced by one or more parent nodes that are not being deleted
// themselves, we should keep it.
cleared = cleared && !node.getParentNodes()
.anyMatch(elem -> elem.getNodeState() != NodeState.DELETED);

// If the grace period is set to infinity (negative values), we are never clear for deletion
cleared = cleared && this.orphanProductGracePeriod >= 0;

if (cleared) {
// At this point we're *almost* clear for deletion; just need to wait out the grace period:
// - If there is not a date set, it has not been previously cleared for deletion. Set the
// date, update the owner-product ref to store the new date and return false
// - if there is a date set, check if we're beyond the grace period (in days). If the date
// has been exceeded, flag the product for deletion
// - if the grace period is zero, we're setup to delete without delay; skip any further
// checks

if (this.orphanProductGracePeriod > 0) {
Instant orphanedDate = this.getOwnerProductOrphanedDate(node);

if (orphanedDate == null) {
this.ownerOrphanedEntities.computeIfAbsent(node.getOwner(), key -> new HashSet<>())
.add(node.getEntityId());

cleared = false;
}
else {
Instant cutoff = Instant.now()
.truncatedTo(ChronoUnit.DAYS)
.minus(this.orphanProductGracePeriod, ChronoUnit.DAYS);

cleared = orphanedDate.truncatedTo(ChronoUnit.DAYS)
.isBefore(cutoff);
}
}
}
else {
// Entity is not cleared for deletion. Remove its orphaned date if it's been set.
if (this.getOwnerProductOrphanedDate(node) != null) {
this.ownerUnorphanedEntities.computeIfAbsent(node.getOwner(), key -> new HashSet<>())
.add(node.getEntityId());
}
}

return cleared;
}

/**
* Fetches the orphaned date of the entity represented by the given node. If the node does not
* represent an orphaned entity, this method returns null.
*
* @param node
* the entity node for which to fetch the orphaned entity date
*
* @return
* the date the entity represented by the given node was orphaned, or null if the node does not
* represent an orphaned entity
*/
private Instant getOwnerProductOrphanedDate(EntityNode<Product, ProductInfo> node) {
Map<String, Instant> orphanDateMap = this.ownerOrphanedDateMap
.computeIfAbsent(node.getOwner(), key -> new HashMap<>());

// Check if we have some precached IDs to convert...
Set<String> entityIdPrecache = this.ownerOrphanEntityIdPrecache.remove(node.getOwner());
if (entityIdPrecache != null) {
Map<String, Instant> fetched = this.ownerProductCurator
.getOwnerProductOrphanedDates(node.getOwner(), entityIdPrecache);

orphanDateMap.putAll(fetched);
}

// If this node somehow missed the processing/precaching step, do an individual lookup and
// cache the result
if (!orphanDateMap.containsKey(node.getEntityId())) {
OwnerProduct op = this.ownerProductCurator
.getOwnerProduct(node.getOwner().getId(), node.getEntityId());

if (op != null) {
orphanDateMap.put(node.getEntityId(), op.getOrphanedDate());
}
}

return orphanDateMap.get(node.getEntityId());
}

/**
Expand Down Expand Up @@ -272,6 +371,17 @@ public void complete() {
this.ownerProductCurator.removeOwnerProductReferences(entry.getKey(), entry.getValue());
}

// Clear orphaned dates on unorphaned products
for (Map.Entry<Owner, Set<String>> entry : this.ownerUnorphanedEntities.entrySet()) {
this.ownerProductCurator.updateOwnerProductOrphanedDates(entry.getKey(), entry.getValue(), null);
}

// Set orphaned dates on newly-orphaned products
for (Map.Entry<Owner, Set<String>> entry : this.ownerOrphanedEntities.entrySet()) {
this.ownerProductCurator.updateOwnerProductOrphanedDates(entry.getKey(), entry.getValue(),
Instant.now());
}

// Save new owner-product entities
this.ownerProductEntities.stream()
.forEach(elem -> this.ownerProductCurator.create(elem, false));
Expand All @@ -288,6 +398,11 @@ public void complete() {
this.deletedProductUuids.clear();
this.ownerEntityVersions.clear();
this.ownerVersionedEntityMap.clear();

this.ownerOrphanEntityIdPrecache.clear();
this.ownerOrphanedDateMap.clear();
this.ownerOrphanedEntities.clear();
this.ownerUnorphanedEntities.clear();
}

private EntityNode<Product, ProductInfo> lookupProductNode(EntityNode<Product, ProductInfo> parentNode,
Expand Down

0 comments on commit 2a9ebd9

Please sign in to comment.