Merge pull request #1 from Flowpack/bugfix-no-concurrent-runs-in-clou…

…d-environments

BUGFIX: ensure in cloud environments, only one content release is building at any given time

Classes/Command/ContentReleasePrepareCommandController.php

@@ -3,6 +3,7 @@
 
 namespace Flowpack\DecoupledContentStore\Command;
 
+use Flowpack\DecoupledContentStore\Core\ConcurrentBuildLockService;
 use Flowpack\DecoupledContentStore\Core\Domain\ValueObject\PrunnerJobId;
 use Flowpack\DecoupledContentStore\PrepareContentRelease\Infrastructure\RedisContentReleaseService;
 use Neos\Flow\Annotations as Flow;
@@ -21,6 +22,12 @@ class ContentReleasePrepareCommandController extends CommandController
      */
     protected $redisContentReleaseService;
 
+    /**
+     * @Flow\Inject
+     * @var ConcurrentBuildLockService
+     */
+    protected $concurrentBuildLock;
+
     public function createContentReleaseCommand(string $contentReleaseIdentifier, string $prunnerJobId)
     {
         $contentReleaseIdentifier = ContentReleaseIdentifier::fromString($contentReleaseIdentifier);
@@ -30,6 +37,13 @@ public function createContentReleaseCommand(string $contentReleaseIdentifier, st
         $this->redisContentReleaseService->createContentRelease($contentReleaseIdentifier, $prunnerJobId, $logger);
     }
 
+    public function ensureAllOtherInProgressContentReleasesWillBeTerminatedCommand(string $contentReleaseIdentifier)
+    {
+        $contentReleaseIdentifier = ContentReleaseIdentifier::fromString($contentReleaseIdentifier);
+
+        $this->concurrentBuildLock->ensureAllOtherInProgressContentReleasesWillBeTerminated($contentReleaseIdentifier);
+    }
+
     public function registerManualTransferJobCommand(string $contentReleaseIdentifier, string $prunnerJobId)
     {
         $contentReleaseIdentifier = ContentReleaseIdentifier::fromString($contentReleaseIdentifier);

Classes/Core/ConcurrentBuildLockService.php

@@ -0,0 +1,64 @@
+<?php
+
+namespace Flowpack\DecoupledContentStore\Core;
+
+use Flowpack\DecoupledContentStore\Core\Domain\ValueObject\ContentReleaseIdentifier;
+use Flowpack\DecoupledContentStore\Core\Infrastructure\RedisClientManager;
+use Neos\Flow\Annotations as Flow;
+
+/**
+ * We usually rely on prunner to ensure that only one build is running at any given time.
+ *
+ * However, when running in a cloud environment with no shared storage, the prunner data folder is not shared between
+ * instances. In this case, during a deployment, two containers run concurrently, with two separate prunner instances
+ * (the old and the new one), which do not see each other.
+ *
+ * We could fix this in prunner itself, but this would be a bigger undertaking (different storage backends for prunner),
+ * or we can work around this in DecoupledContentStore. This is what this class does.
+ *
+ * ## Main Idea
+ *
+ * - We use a special redis key "contentStore:concurrentBuildLock" which is set to the current being-built release ID in
+ *   `./flow contentReleasePrepare:ensureAllOtherInProgressContentReleasesWillBeTerminated`
+ * - In the "Enumerate" and "Render" phases, we periodically check whether the concurrentBuildLock is set to the currently
+ *   in-progress content release. If NO, we abort.
+ *
+ * @Flow\Scope("singleton")
+ */
+class ConcurrentBuildLockService
+{
+
+    /**
+     * @Flow\Inject
+     * @var RedisClientManager
+     */
+    protected $redisClientManager;
+
+    public function ensureAllOtherInProgressContentReleasesWillBeTerminated(ContentReleaseIdentifier $contentReleaseIdentifier)
+    {
+        $this->redisClientManager->getPrimaryRedis()->set('contentStore:concurrentBuildLock', $contentReleaseIdentifier->getIdentifier());
+    }
+
+    public function assertNoOtherContentReleaseWasStarted(ContentReleaseIdentifier $contentReleaseIdentifier)
+    {
+        $concurrentBuildLockString = $this->redisClientManager->getPrimaryRedis()->get('contentStore:concurrentBuildLock');
+
+        if (empty($concurrentBuildLockString)) {
+            echo '!!!!! Hard-aborting the current job ' . $contentReleaseIdentifier->getIdentifier() . ' because the concurrentBuildLock does not exist.' . "\n\n";
+            echo "This should never happen for correctly configured jobs (that run after prepare_finished).\n\n";
+            exit(1);
+        }
+
+        $concurrentBuildLock = ContentReleaseIdentifier::fromString($concurrentBuildLockString);
+
+        if (!$contentReleaseIdentifier->equals($concurrentBuildLock)) {
+            // the concurrent build lock is different (i.e. newer) than our currently-running content release.
+            // Thus, we abort the in-progress content release as quickly as we can - by DYING.
+
+            echo '!!!!! Hard-aborting the current job ' . $contentReleaseIdentifier->getIdentifier() . ' because the concurrentBuildLock contains ' . $concurrentBuildLock->getIdentifier() . "\n\n";
+            echo "This is no error during deployment, but should never happen outside a deployment.\n\n It can only happen if two prunner instances run concurrently.\n\n";
+            exit(1);
+        }
+    }
+
+}

Classes/NodeEnumeration/NodeEnumerator.php

@@ -3,6 +3,7 @@
 namespace Flowpack\DecoupledContentStore\NodeEnumeration;
 
 
+use Flowpack\DecoupledContentStore\Core\ConcurrentBuildLockService;
 use Flowpack\DecoupledContentStore\Core\Domain\ValueObject\ContentReleaseIdentifier;
 use Flowpack\DecoupledContentStore\Core\Domain\ValueObject\RedisInstanceIdentifier;
 use Flowpack\DecoupledContentStore\Core\Infrastructure\ContentReleaseLogger;
@@ -37,6 +38,12 @@ class NodeEnumerator
      */
     protected $redisContentReleaseService;
 
+    /**
+     * @Flow\Inject
+     * @var ConcurrentBuildLockService
+     */
+    protected $concurrentBuildLockService;
+
     /**
      * @Flow\InjectConfiguration("nodeRendering.nodeTypeWhitelist")
      * @var string
@@ -55,6 +62,7 @@ public function enumerateAndStoreInRedis(?Site $site, ContentReleaseLogger $cont
 
         $this->redisEnumerationRepository->clearDocumentNodesEnumeration($releaseIdentifier);
         foreach (GeneratorUtility::createArrayBatch($this->enumerateAll($site, $contentReleaseLogger), 100) as $enumeration) {
+            $this->concurrentBuildLockService->assertNoOtherContentReleaseWasStarted($releaseIdentifier);
             // $enumeration is an array of EnumeratedNode, with at most 100 elements in it.
             // TODO: EXTENSION POINT HERE, TO ADD ADDITIONAL ENUMERATIONS (.metadata.json f.e.)
             // TODO: not yet fully sure how to handle Enumeration

Classes/NodeRendering/NodeRenderOrchestrator.php

@@ -4,6 +4,7 @@
 
 namespace Flowpack\DecoupledContentStore\NodeRendering;
 
+use Flowpack\DecoupledContentStore\Core\ConcurrentBuildLockService;
 use Flowpack\DecoupledContentStore\Core\Domain\ValueObject\RedisInstanceIdentifier;
 use Flowpack\DecoupledContentStore\NodeRendering\Dto\DocumentNodeCacheKey;
 use Flowpack\DecoupledContentStore\NodeRendering\Dto\RenderingStatistics;
@@ -93,6 +94,12 @@ class NodeRenderOrchestrator
      */
     protected $redisContentReleaseService;
 
+    /**
+     * @Flow\Inject
+     * @var ConcurrentBuildLockService
+     */
+    protected $concurrentBuildLockService;
+
     private const EXIT_ERRORSTATUSCODE_RELEASE_ALREADY_COMPLETED = 1;
     private const EXIT_ERRORSTATUSCODE_EMPTY_ENUMERATION = 2;
     private const EXIT_ERRORSTATUSCODE_RETRY_LIMIT_REACHED = 3;
@@ -140,6 +147,7 @@ public function renderContentRelease(ContentReleaseIdentifier $contentReleaseIde
             }
 
             $contentReleaseLogger->info('Starting iteration ' . $i);
+            $this->concurrentBuildLockService->assertNoOtherContentReleaseWasStarted($contentReleaseIdentifier);
 
             $this->redisRenderingStatisticsStore->addStatisticsIteration($contentReleaseIdentifier, RenderingStatistics::create(0, 0, []));
 
@@ -200,11 +208,12 @@ public function renderContentRelease(ContentReleaseIdentifier $contentReleaseIde
                     $jobsWorkedThroughOverLastTenSeconds = $previousRemainingJobs - $remainingJobsCount;
                     $renderingsPerSecondDataPoints[] = $jobsWorkedThroughOverLastTenSeconds / 10;
 
-
                     $contentReleaseLogger->debug('Waiting... ', [
                         'numberOfQueuedJobs' => $remainingJobsCount,
                         'numberOfRenderingsInProgress' => $this->redisRenderingQueue->numberOfRenderingsInProgress($contentReleaseIdentifier),
                     ]);
+
+                    $this->concurrentBuildLockService->assertNoOtherContentReleaseWasStarted($contentReleaseIdentifier);
                 }
             }
 

Classes/NodeRendering/NodeRenderer.php

@@ -5,6 +5,7 @@
 namespace Flowpack\DecoupledContentStore\NodeRendering;
 
 use Flowpack\DecoupledContentStore\ContentReleaseManager;
+use Flowpack\DecoupledContentStore\Core\ConcurrentBuildLockService;
 use Flowpack\DecoupledContentStore\NodeRendering\ProcessEvents\DocumentRenderedEvent;
 use Flowpack\DecoupledContentStore\NodeRendering\ProcessEvents\ExitEvent;
 use Flowpack\DecoupledContentStore\NodeRendering\ProcessEvents\QueueEmptyEvent;
@@ -112,6 +113,12 @@ class NodeRenderer
      */
     protected $persistenceManager;
 
+    /**
+     * @Flow\Inject
+     * @var ConcurrentBuildLockService
+     */
+    protected $concurrentBuildLockService;
+
 
     public function render(ContentReleaseIdentifier $contentReleaseIdentifier, ContentReleaseLogger $contentReleaseLogger, RendererIdentifier $rendererIdentifier)
     {
@@ -133,6 +140,7 @@ public function render(ContentReleaseIdentifier $contentReleaseIdentifier, Conte
                 // determining what needs to be done. We just need to wait a bit and retry.
                 $contentReleaseLogger->debug('Rendering queue currently empty; we wait a bit see if there is work for us.');
                 sleep(2);
+                $this->concurrentBuildLockService->assertNoOtherContentReleaseWasStarted($contentReleaseIdentifier);
                 continue;
             }
 
@@ -149,6 +157,11 @@ public function render(ContentReleaseIdentifier $contentReleaseIdentifier, Conte
             yield DocumentRenderedEvent::create();
 
             $i++;
+
+            if ($i % 5 === 0) {
+                $this->concurrentBuildLockService->assertNoOtherContentReleaseWasStarted($contentReleaseIdentifier);
+            }
+
             if ($i % 20 === 0) {
                 $contentReleaseLogger->info('Restarting after 20 renders.');
                 yield ExitEvent::createWithStatusCode(193);

pipelines_template.yml

@@ -33,6 +33,7 @@ pipelines:
       prepare_finished:
         script:
           - ./flow contentReleasePrepare:createContentRelease {{ .contentReleaseId }} {{ .__jobID }}
+          - ./flow contentReleasePrepare:ensureAllOtherInProgressContentReleasesWillBeTerminated {{ .contentReleaseId }}
 
       ################################################################################
       # 1) ENUMERATION