Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Batch products together during catalog rule indexer calculation to improve performance #37889

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 35 additions & 11 deletions app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

namespace Magento\CatalogRule\Model\Indexer;

use Exception;
use Magento\Catalog\Model\Product;
use Magento\Catalog\Model\ProductFactory;
use Magento\Catalog\Model\ResourceModel\Indexer\ActiveTableSwitcher;
Expand All @@ -28,6 +29,7 @@
use Magento\Store\Model\ScopeInterface;
use Magento\Store\Model\StoreManagerInterface;
use Psr\Log\LoggerInterface;
use Zend_Db_Statement_Exception;

/**
* Catalog rule index builder
Expand Down Expand Up @@ -181,6 +183,16 @@ class IndexBuilder
*/
private $productCollectionFactory;

/**
* @var ReindexRuleProductsPrice
*/
private $reindexRuleProductsPrice;

/**
* @var int
*/
private $productBatchSize;

/**
* @param RuleCollectionFactory $ruleCollectionFactory
* @param PriceCurrencyInterface $priceCurrency
Expand All @@ -204,6 +216,8 @@ class IndexBuilder
* @param TimezoneInterface|null $localeDate
* @param ProductCollectionFactory|null $productCollectionFactory
* @param IndexerRegistry|null $indexerRegistry
* @param ReindexRuleProductsPrice|null $reindexRuleProductsPrice
* @param int $productBatchSize
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
*/
Expand All @@ -229,7 +243,9 @@ public function __construct(
TableSwapper $tableSwapper = null,
TimezoneInterface $localeDate = null,
ProductCollectionFactory $productCollectionFactory = null,
IndexerRegistry $indexerRegistry = null
IndexerRegistry $indexerRegistry = null,
ReindexRuleProductsPrice $reindexRuleProductsPrice = null,
int $productBatchSize = 1000
) {
$this->resource = $resource;
$this->connection = $resource->getConnection();
Expand All @@ -242,6 +258,7 @@ public function __construct(
$this->dateTime = $dateTime;
$this->productFactory = $productFactory;
$this->batchCount = $batchCount;
$this->productBatchSize = $productBatchSize;

$this->productPriceCalculator = $productPriceCalculator ?? ObjectManager::getInstance()->get(
ProductPriceCalculator::class
Expand Down Expand Up @@ -275,6 +292,8 @@ public function __construct(
ObjectManager::getInstance()->get(IndexerRegistry::class);
$this->productCollectionFactory = $productCollectionFactory ??
ObjectManager::getInstance()->get(ProductCollectionFactory::class);
$this->reindexRuleProductsPrice = $reindexRuleProductsPrice ??
ObjectManager::getInstance()->get(ReindexRuleProductsPrice::class);
}

/**
Expand All @@ -296,7 +315,7 @@ public function reindexById($id)
}

$this->reindexRuleGroupWebsite->execute();
} catch (\Exception $e) {
} catch (Exception $e) {
$this->critical($e);
throw new LocalizedException(
__('Catalog rule indexing failed. See details in exception log.')
Expand All @@ -315,7 +334,7 @@ public function reindexByIds(array $ids)
{
try {
$this->doReindexByIds($ids);
} catch (\Exception $e) {
} catch (Exception $e) {
$this->critical($e);
throw new LocalizedException(
__("Catalog rule indexing failed. See details in exception log.")
Expand All @@ -328,6 +347,8 @@ public function reindexByIds(array $ids)
*
* @param array $ids
* @return void
* @throws LocalizedException
* @throws Zend_Db_Statement_Exception
*/
protected function doReindexByIds($ids)
{
Expand All @@ -340,9 +361,10 @@ protected function doReindexByIds($ids)
$this->reindexRuleProduct->execute($rule, $this->batchCount);
}

foreach ($ids as $productId) {
$this->cleanProductPriceIndex([$productId]);
$this->reindexRuleProductPrice->execute($this->batchCount, $productId);
// batch products together, using configurable batch size parameter
foreach (array_chunk($ids, $this->productBatchSize) as $productIds) {
$this->cleanProductPriceIndex($productIds);
$this->reindexRuleProductsPrice->execute($this->batchCount, $productIds);
}

//the case was not handled via indexer dependency decorator or via mview configuration
Expand All @@ -367,7 +389,7 @@ public function reindexFull()
{
try {
$this->doReindexFull();
} catch (\Exception $e) {
} catch (Exception $e) {
$this->critical($e);
throw new LocalizedException(
__("Catalog rule indexing failed. See details in exception log.")
Expand Down Expand Up @@ -441,6 +463,7 @@ protected function cleanByIds($productIds)
* @param int $productEntityId
* @param array $websiteIds
* @return void
* @throws Exception
*/
private function assignProductToRule(Rule $rule, int $productEntityId, array $websiteIds): void
{
Expand Down Expand Up @@ -502,7 +525,7 @@ private function assignProductToRule(Rule $rule, int $productEntityId, array $we
* @param Rule $rule
* @param Product $product
* @return $this
* @throws \Exception
* @throws Exception
* @deprecated 101.1.5
* @see ReindexRuleProduct::execute
* @SuppressWarnings(PHPMD.NPathComplexity)
Expand All @@ -525,6 +548,7 @@ protected function applyRule(Rule $rule, $product)
* @param RuleCollection $ruleCollection
* @param Product $product
* @return void
* @throws LocalizedException
*/
private function applyRules(RuleCollection $ruleCollection, Product $product): void
{
Expand Down Expand Up @@ -590,7 +614,7 @@ protected function updateRuleProductData(Rule $rule)
* Apply all rules
*
* @param Product|null $product
* @throws \Exception
* @throws Exception
* @return $this
* @deprecated 101.0.0
* @see ReindexRuleProductPrice::execute
Expand Down Expand Up @@ -661,7 +685,7 @@ protected function getRuleProductsStmt($websiteId, Product $product = null)
*
* @param array $arrData
* @return $this
* @throws \Exception
* @throws Exception
* @deprecated 101.0.0
* @see RuleProductPricesPersistor::execute
*/
Expand Down Expand Up @@ -708,7 +732,7 @@ protected function getProduct($productId)
/**
* Log critical exception
*
* @param \Exception $e
* @param Exception $e
* @return void
*/
protected function critical($e)
Expand Down
121 changes: 27 additions & 94 deletions app/code/Magento/CatalogRule/Model/Indexer/ReindexRuleProductPrice.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@

namespace Magento\CatalogRule\Model\Indexer;

use Magento\Framework\App\ObjectManager;
use Magento\Framework\Exception\LocalizedException;
use Magento\Framework\Stdlib\DateTime\TimezoneInterface;
use Magento\Store\Model\Store;
use Magento\Store\Model\StoreManagerInterface;
use Zend_Db_Statement_Exception;

/**
* Reindex product prices according rule settings.
Expand Down Expand Up @@ -45,28 +47,37 @@ class ReindexRuleProductPrice
*/
private $useWebsiteTimezone;

/**
* @var ReindexRuleProductsPriceProcessor
*/
private $reindexRuleProductsPriceProcessor;

/**
* @param StoreManagerInterface $storeManager
* @param RuleProductsSelectBuilder $ruleProductsSelectBuilder
* @param ProductPriceCalculator $productPriceCalculator
* @param TimezoneInterface $localeDate
* @param RuleProductPricesPersistor $pricesPersistor
* @param bool $useWebsiteTimezone
* @param ReindexRuleProductsPriceProcessor|null $reindexRuleProductsPriceProcessor
*/
public function __construct(
StoreManagerInterface $storeManager,
RuleProductsSelectBuilder $ruleProductsSelectBuilder,
ProductPriceCalculator $productPriceCalculator,
TimezoneInterface $localeDate,
RuleProductPricesPersistor $pricesPersistor,
bool $useWebsiteTimezone = true
bool $useWebsiteTimezone = true,
ReindexRuleProductsPriceProcessor $reindexRuleProductsPriceProcessor = null
) {
$this->storeManager = $storeManager;
$this->ruleProductsSelectBuilder = $ruleProductsSelectBuilder;
$this->productPriceCalculator = $productPriceCalculator;
$this->localeDate = $localeDate;
$this->pricesPersistor = $pricesPersistor;
$this->useWebsiteTimezone = $useWebsiteTimezone;
$this->reindexRuleProductsPriceProcessor = $reindexRuleProductsPriceProcessor ??
ObjectManager::getInstance()->get(ReindexRuleProductsPriceProcessor::class);
}

/**
Expand All @@ -76,6 +87,8 @@ public function __construct(
* @param int|null $productId
* @param bool $useAdditionalTable
* @return bool
* @throws LocalizedException
* @throws Zend_Db_Statement_Exception
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
*/
public function execute(int $batchCount, ?int $productId = null, bool $useAdditionalTable = false)
Expand All @@ -85,100 +98,20 @@ public function execute(int $batchCount, ?int $productId = null, bool $useAdditi
* because for each website date in website's timezone should be used
*/
foreach ($this->storeManager->getWebsites() as $website) {
$productsStmt = $this->ruleProductsSelectBuilder->build($website->getId(), $productId, $useAdditionalTable);
$dayPrices = [];
$stopFlags = [];
$prevKey = null;

$storeGroup = $this->storeManager->getGroup($website->getDefaultGroupId());
$dateInterval = $this->useWebsiteTimezone
? $this->getDateInterval((int)$storeGroup->getDefaultStoreId())
: $this->getDateInterval(Store::DEFAULT_STORE_ID);

while ($ruleData = $productsStmt->fetch()) {
$ruleProductId = $ruleData['product_id'];
$productKey = $ruleProductId .
'_' .
$ruleData['website_id'] .
'_' .
$ruleData['customer_group_id'];

if ($prevKey && $prevKey != $productKey) {
$stopFlags = [];
if (count($dayPrices) > $batchCount) {
$this->pricesPersistor->execute($dayPrices, $useAdditionalTable);
$dayPrices = [];
}
}

/**
* Build prices for each day
*/
foreach ($dateInterval as $date) {
$time = $date->getTimestamp();
if (($ruleData['from_time'] == 0 ||
$time >= $ruleData['from_time']) && ($ruleData['to_time'] == 0 ||
$time <= $ruleData['to_time'])
) {
$priceKey = $time . '_' . $productKey;

if (isset($stopFlags[$priceKey])) {
continue;
}

if (!isset($dayPrices[$priceKey])) {
$dayPrices[$priceKey] = [
'rule_date' => $date,
'website_id' => $ruleData['website_id'],
'customer_group_id' => $ruleData['customer_group_id'],
'product_id' => $ruleProductId,
'rule_price' => $this->productPriceCalculator->calculate($ruleData),
'latest_start_date' => $ruleData['from_time'],
'earliest_end_date' => $ruleData['to_time'],
];
} else {
$dayPrices[$priceKey]['rule_price'] = $this->productPriceCalculator->calculate(
$ruleData,
$dayPrices[$priceKey]
);
$dayPrices[$priceKey]['latest_start_date'] = max(
$dayPrices[$priceKey]['latest_start_date'],
$ruleData['from_time']
);
$dayPrices[$priceKey]['earliest_end_date'] = min(
$dayPrices[$priceKey]['earliest_end_date'],
$ruleData['to_time']
);
}

if ($ruleData['action_stop']) {
$stopFlags[$priceKey] = true;
}
}
}

$prevKey = $productKey;
}
$this->pricesPersistor->execute($dayPrices, $useAdditionalTable);
$productsStmt = $this->ruleProductsSelectBuilder->build(
(int)$website->getId(),
$productId,
$useAdditionalTable
);
$this->reindexRuleProductsPriceProcessor->execute(
$productsStmt,
$website,
$batchCount,
$useAdditionalTable,
$this->useWebsiteTimezone
);
}

return true;
}

/**
* Retrieve date sequence in store time zone
*
* @param int $storeId
* @return \DateTime[]
*/
private function getDateInterval(int $storeId): array
{
$currentDate = $this->localeDate->scopeDate($storeId, null, true);
$previousDate = (clone $currentDate)->modify('-1 day');
$previousDate->setTime(23, 59, 59);
$nextDate = (clone $currentDate)->modify('+1 day');
$nextDate->setTime(0, 0, 0);

return [$previousDate, $currentDate, $nextDate];
}
}
Loading