SUPESC-558 Fixed memory leak during repository synchronization export…

…. (#9417) SUPESC-558 Fixed memory leak during repository synchronization export.
spryker · Dec 14, 2022 · 443dc17 · 443dc17
1 parent fbfb12a
commit 443dc17
Show file tree

Hide file tree

Showing 6 changed files with 156 additions and 10 deletions.
diff --git a/composer.json b/composer.json
@@ -12,7 +12,7 @@
         "spryker/search": "^8.3.0",
         "spryker/storage": "^3.4.1",
         "spryker/symfony": "^3.1.0",
-        "spryker/synchronization-extension": "^1.2.0",
+        "spryker/synchronization-extension": "^1.3.0",
         "spryker/transfer": "^3.25.0",
         "spryker/util-encoding": "^2.0.0"
     },

diff --git a/src/Spryker/Zed/Synchronization/Business/Export/RepositoryExporter.php b/src/Spryker/Zed/Synchronization/Business/Export/RepositoryExporter.php
@@ -10,17 +10,21 @@
 use Generated\Shared\Transfer\SynchronizationDataTransfer;
 use Generated\Shared\Transfer\SynchronizationQueueMessageTransfer;
 use Iterator;
+use Spryker\Zed\Kernel\Persistence\EntityManager\InstancePoolingTrait;
 use Spryker\Zed\Synchronization\Business\Iterator\SynchronizationDataBulkRepositoryPluginIterator;
 use Spryker\Zed\Synchronization\Business\Iterator\SynchronizationDataRepositoryPluginIterator;
 use Spryker\Zed\Synchronization\Business\Message\QueueMessageCreatorInterface;
 use Spryker\Zed\Synchronization\Dependency\Client\SynchronizationToQueueClientInterface;
 use Spryker\Zed\Synchronization\Dependency\Service\SynchronizationToUtilEncodingServiceInterface;
+use Spryker\Zed\Synchronization\SynchronizationConfig;
 use Spryker\Zed\SynchronizationExtension\Dependency\Plugin\SynchronizationDataBulkRepositoryPluginInterface;
 use Spryker\Zed\SynchronizationExtension\Dependency\Plugin\SynchronizationDataPluginInterface;
 use Spryker\Zed\SynchronizationExtension\Dependency\Plugin\SynchronizationDataRepositoryPluginInterface;
 
 class RepositoryExporter implements ExporterInterface
 {
+    use InstancePoolingTrait;
+
     /**
      * @var \Spryker\Zed\Synchronization\Dependency\Client\SynchronizationToQueueClientInterface
      */
@@ -37,9 +41,9 @@ class RepositoryExporter implements ExporterInterface
     protected $synchronizationDataPlugins;
 
     /**
-     * @var int
+     * @var \Spryker\Zed\Synchronization\SynchronizationConfig
      */
-    protected $chunkSize;
+    protected SynchronizationConfig $synchronizationConfig;
 
     /**
      * @var \Spryker\Zed\Synchronization\Dependency\Service\SynchronizationToUtilEncodingServiceInterface
@@ -50,18 +54,18 @@ class RepositoryExporter implements ExporterInterface
      * @param \Spryker\Zed\Synchronization\Dependency\Client\SynchronizationToQueueClientInterface $queueClient
      * @param \Spryker\Zed\Synchronization\Business\Message\QueueMessageCreatorInterface $synchronizationQueueMessageCreator
      * @param \Spryker\Zed\Synchronization\Dependency\Service\SynchronizationToUtilEncodingServiceInterface $utilEncodingService
-     * @param int $chunkSize
+     * @param \Spryker\Zed\Synchronization\SynchronizationConfig $synchronizationConfig
      */
     public function __construct(
         SynchronizationToQueueClientInterface $queueClient,
         QueueMessageCreatorInterface $synchronizationQueueMessageCreator,
         SynchronizationToUtilEncodingServiceInterface $utilEncodingService,
-        $chunkSize = 100
+        SynchronizationConfig $synchronizationConfig
     ) {
         $this->queueClient = $queueClient;
         $this->queueMessageCreator = $synchronizationQueueMessageCreator;
-        $this->chunkSize = $chunkSize;
         $this->utilEncodingService = $utilEncodingService;
+        $this->synchronizationConfig = $synchronizationConfig;
     }
 
     /**
@@ -72,6 +76,11 @@ public function __construct(
      */
     public function exportSynchronizedData(array $plugins, array $ids = []): void
     {
+        $isInstancePoolingDisabled = false;
+        if ($this->synchronizationConfig->isRepositorySyncExportPropelInstancePoolingDisabled()) {
+            $isInstancePoolingDisabled = $this->disableInstancePooling();
+        }
+
         foreach ($plugins as $plugin) {
             if ($plugin instanceof SynchronizationDataRepositoryPluginInterface) {
                 $this->exportData($ids, $plugin);
@@ -83,6 +92,10 @@ public function exportSynchronizedData(array $plugins, array $ids = []): void
                 $this->exportDataBulk($plugin, $ids);
             }
         }
+
+        if ($isInstancePoolingDisabled) {
+            $this->enableInstancePooling();
+        }
     }
 
     /**
@@ -106,7 +119,11 @@ protected function exportData(array $ids, SynchronizationDataRepositoryPluginInt
      */
     protected function createSynchronizationDataRepositoryPluginIterator(array $ids, SynchronizationDataRepositoryPluginInterface $plugin): Iterator
     {
-        return new SynchronizationDataRepositoryPluginIterator($plugin, $this->chunkSize, $ids);
+        return new SynchronizationDataRepositoryPluginIterator(
+            $plugin,
+            $this->synchronizationConfig->getSyncExportChunkSize(),
+            $ids,
+        );
     }
 
     /**
@@ -130,7 +147,11 @@ protected function exportDataBulk(SynchronizationDataBulkRepositoryPluginInterfa
      */
     protected function createSynchronizationDataBulkRepositoryPluginIterator(array $ids, SynchronizationDataBulkRepositoryPluginInterface $plugin): Iterator
     {
-        return new SynchronizationDataBulkRepositoryPluginIterator($plugin, $this->chunkSize, $ids);
+        return new SynchronizationDataBulkRepositoryPluginIterator(
+            $plugin,
+            $this->synchronizationConfig->getSyncExportChunkSize(),
+            $ids,
+        );
     }
 
     /**

diff --git a/...Zed/Synchronization/Business/Iterator/SynchronizationDataBulkRepositoryPluginIterator.php b/...Zed/Synchronization/Business/Iterator/SynchronizationDataBulkRepositoryPluginIterator.php
@@ -8,6 +8,7 @@
 namespace Spryker\Zed\Synchronization\Business\Iterator;
 
 use Spryker\Zed\SynchronizationExtension\Dependency\Plugin\SynchronizationDataBulkRepositoryPluginInterface;
+use Spryker\Zed\SynchronizationExtension\Dependency\Plugin\SynchronizationDataMaxIterationLimitPluginInterface;
 
 class SynchronizationDataBulkRepositoryPluginIterator extends AbstractSynchronizationDataPluginIterator
 {
@@ -19,7 +20,12 @@ class SynchronizationDataBulkRepositoryPluginIterator extends AbstractSynchroniz
     /**
      * @var array<int>
      */
-    protected $filterIds;
+    protected array $filterIds = [];
+
+    /**
+     * @var int
+     */
+    protected int $iterationLimit = 0;
 
     /**
      * @param \Spryker\Zed\SynchronizationExtension\Dependency\Plugin\SynchronizationDataBulkRepositoryPluginInterface $plugin
@@ -31,6 +37,10 @@ public function __construct(SynchronizationDataBulkRepositoryPluginInterface $pl
         parent::__construct($plugin, $chunkSize);
 
         $this->filterIds = $ids;
+
+        if ($plugin instanceof SynchronizationDataMaxIterationLimitPluginInterface) {
+            $this->iterationLimit = $plugin->getMaxIterationLimit();
+        }
     }
 
     /**
@@ -40,4 +50,26 @@ protected function updateCurrent(): void
     {
         $this->current = $this->plugin->getData($this->offset, $this->chunkSize, $this->filterIds);
     }
+
+    /**
+     * @return bool
+     */
+    public function valid(): bool
+    {
+        $valid = parent::valid();
+
+        if (!$valid && !$this->isIterationLimitExceeded()) {
+            return true;
+        }
+
+        return $valid;
+    }
+
+    /**
+     * @return bool
+     */
+    protected function isIterationLimitExceeded(): bool
+    {
+        return $this->offset + $this->chunkSize > $this->iterationLimit;
+    }
 }
diff --git a/src/Spryker/Zed/Synchronization/Business/SynchronizationBusinessFactory.php b/src/Spryker/Zed/Synchronization/Business/SynchronizationBusinessFactory.php
@@ -60,7 +60,7 @@ public function createRepositoryExporter()
             $this->getQueueClient(),
             $this->createQueueMessageCreator(),
             $this->getUtilEncodingService(),
-            $this->getConfig()->getSyncExportChunkSize(),
+            $this->getConfig(),
         );
     }
 

diff --git a/src/Spryker/Zed/Synchronization/SynchronizationConfig.php b/src/Spryker/Zed/Synchronization/SynchronizationConfig.php
@@ -67,4 +67,17 @@ public function getSyncExportChunkSize()
     {
         return $this->get(SynchronizationConstants::EXPORT_MESSAGE_CHUNK_SIZE, 100);
     }
+
+    /**
+     * Specification:
+     * - Disables Propel Instance Pooling for repository synchronization export if set to true.
+     *
+     * @api
+     *
+     * @return bool
+     */
+    public function isRepositorySyncExportPropelInstancePoolingDisabled(): bool
+    {
+        return false;
+    }
 }
diff --git a/tests/SprykerTest/Zed/Synchronization/Business/Export/RepositoryExporterTest.php b/tests/SprykerTest/Zed/Synchronization/Business/Export/RepositoryExporterTest.php
@@ -0,0 +1,80 @@
+<?php
+
+/**
+ * Copyright © 2016-present Spryker Systems GmbH. All rights reserved.
+ * Use of this software requires acceptance of the Evaluation License Agreement. See LICENSE file.
+ */
+
+namespace SprykerTest\Zed\Synchronization\Business\Export;
+
+use Codeception\Test\Unit;
+use Spryker\Zed\Synchronization\Business\Export\ExporterInterface;
+use Spryker\Zed\Synchronization\Business\Export\RepositoryExporter;
+use Spryker\Zed\Synchronization\Business\Message\QueueMessageCreatorInterface;
+use Spryker\Zed\Synchronization\Dependency\Client\SynchronizationToQueueClientInterface;
+use Spryker\Zed\Synchronization\Dependency\Service\SynchronizationToUtilEncodingServiceInterface;
+use Spryker\Zed\Synchronization\SynchronizationConfig;
+
+/**
+ * Auto-generated group annotations
+ *
+ * @group SprykerTest
+ * @group Zed
+ * @group Synchronization
+ * @group Business
+ * @group Export
+ * @group RepositoryExporterTest
+ * Add your own group annotations below this line
+ */
+class RepositoryExporterTest extends Unit
+{
+    /**
+     * @return void
+     */
+    public function testExportSynchronizedDataWillNotDisablePoolingWithDisableInstantPoolingParamFalse(): void
+    {
+        // Arrange
+        $repositoryExporterMock = $this->createRepositoryExporterMock(false);
+        $repositoryExporterMock->expects($this->never())->method('disableInstancePooling');
+        $repositoryExporterMock->expects($this->never())->method('enableInstancePooling');
+
+        // Act, Assert
+        $repositoryExporterMock->exportSynchronizedData([], []);
+    }
+
+    /**
+     * @return void
+     */
+    public function testExportSynchronizedDataWillDisablePoolingWithDisableInstantPoolingParamTrue(): void
+    {
+        // Arrange
+        $repositoryExporterMock = $this->createRepositoryExporterMock(true);
+        $repositoryExporterMock->expects($this->once())->method('disableInstancePooling')->willReturn(true);
+        $repositoryExporterMock->expects($this->once())->method('enableInstancePooling');
+
+        // Act, Assert
+        $repositoryExporterMock->exportSynchronizedData([], []);
+    }
+
+    /**
+     * @param bool $disablePropelInstancePool
+     *
+     * @return \Spryker\Zed\Synchronization\Business\Export\ExporterInterface
+     */
+    protected function createRepositoryExporterMock(bool $disablePropelInstancePool): ExporterInterface
+    {
+        $synchronizationConfigStub = $this->getMockBuilder(SynchronizationConfig::class)
+            ->onlyMethods(['isRepositorySyncExportPropelInstancePoolingDisabled'])->getMock();
+        $synchronizationConfigStub->method('isRepositorySyncExportPropelInstancePoolingDisabled')->willReturn($disablePropelInstancePool);
+
+        return $this->getMockBuilder(RepositoryExporter::class)
+            ->setConstructorArgs([
+                    $this->getMockBuilder(SynchronizationToQueueClientInterface::class)->getMock(),
+                    $this->getMockBuilder(QueueMessageCreatorInterface::class)->getMock(),
+                    $this->getMockBuilder(SynchronizationToUtilEncodingServiceInterface::class)->getMock(),
+                    $synchronizationConfigStub,
+            ])
+            ->onlyMethods(['disableInstancePooling', 'enableInstancePooling'])
+            ->getMock();
+    }
+}