Merge pull request #9 from Flowpack/only-count-successful-release-for-removing

TASK: only count successful releases for retention count

Author: batabana

Classes/Transfer/ContentReleaseCleaner.php

@@ -8,6 +8,8 @@
 use Flowpack\DecoupledContentStore\Core\Infrastructure\ContentReleaseLogger;
 use Flowpack\DecoupledContentStore\Core\Infrastructure\RedisClientManager;
 use Flowpack\DecoupledContentStore\Core\RedisKeyService;
+use Flowpack\DecoupledContentStore\NodeRendering\Dto\NodeRenderingCompletionStatus;
+use Flowpack\DecoupledContentStore\NodeRendering\Infrastructure\RedisRenderingErrorManager;
 use Flowpack\DecoupledContentStore\PrepareContentRelease\Infrastructure\RedisContentReleaseService;
 use Flowpack\DecoupledContentStore\ReleaseSwitch\Infrastructure\RedisReleaseSwitchService;
 use Flowpack\DecoupledContentStore\Transfer\Dto\RedisKeyPostfixesForEachRelease;
@@ -42,6 +44,11 @@ class ContentReleaseCleaner
      */
     protected $redisReleaseSwitchService;
 
+    /**
+     * @Flow\Inject
+     * @var RedisRenderingErrorManager
+     */
+    protected $redisRenderingErrorManager;
 
     /**
      * @Flow\Inject
@@ -66,26 +73,38 @@ public function removeOldReleases(RedisInstanceIdentifier $redisInstanceIdentifi
             throw new \RuntimeException('contentReleaseRetentionCount must be at least 2, found: ' . $contentReleasesToKeep);
         }
 
-        $i = 0;
+        $healthyReleaseCounter = 0;
         $releasesToRemove = [];
         foreach ($contentReleaseIds as $id) {
             if ($id->equals($currentRelease)) {
                 $contentReleaseLogger->info('- ' . $id->getIdentifier() . ' (LIVE)');
             } elseif ($id->equals($contentReleaseIdentifierOfUpcomingRelease)) {
                 $contentReleaseLogger->info('- ' . $id->getIdentifier() . ' (UPCOMING)');
             } else {
-                $i++;
+                if (!$redisInstanceIdentifier->isPrimary()) {
+                    // We are on a PRODUCTION content store (in case of a replicated setup) - there are no errors transferred there
+                    // by design and we want to NEVER run into a memory problem (that's why the cleanup should always run).
+                    $healthyReleaseCounter++;
+                } else {
+                    // We are on the primary content store (the one which the pipeline uses as scratch space).
+                    // In case of errors, we want to keep more "good" content releases ("success" state and no errors), so that we can
+                    // more easily switch back to an older release.
+                    // -> We accept potential Redis out of memory errors in this case.
+                    if (($this->redisContentReleaseService->fetchMetadataForContentRelease($id)->getStatus()->getStatus() === NodeRenderingCompletionStatus::success()->getStatus() && count($this->redisRenderingErrorManager->getRenderingErrors($id)) === 0)) {
+                        $healthyReleaseCounter++;
+                    }
+                }
 
                 // we always want to keep $currentRelease and $contentReleaseIdentifierOfUpcomingRelease; thus
                 // we need to remove 2 from $contentReleasesToKeep
-               $shouldRemoveRelease = ($i > $contentReleasesToKeep - 2);
-
-               if ($shouldRemoveRelease) {
-                   $releasesToRemove[] = $id;
-                   $contentReleaseLogger->info('- ' . $id->getIdentifier() . ' <-- to remove');
-               } else {
-                   $contentReleaseLogger->info('- ' . $id->getIdentifier());
-               }
+                $shouldRemoveRelease = ($healthyReleaseCounter > $contentReleasesToKeep - 2);
+
+                if ($shouldRemoveRelease) {
+                    $releasesToRemove[] = $id;
+                    $contentReleaseLogger->info('- ' . $id->getIdentifier() . ' <-- to remove');
+                } else {
+                    $contentReleaseLogger->info('- ' . $id->getIdentifier());
+                }
             }
         }