Skip to content

Commit 359f79c

Browse files
bubasumabubasuma
committed
MC-38003: [Magento Cloud] Indexer Performance - Possible Bug Found with Cache Clearing process
- Flush indexer cache context after cleaning cache
1 parent 51b53be commit 359f79c

File tree

8 files changed

+228
-19
lines changed

8 files changed

+228
-19
lines changed

‎app/code/Magento/Catalog/Model/Indexer/Category/Product.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,8 @@ public function __construct(
6464
*/
6565
public function execute($ids)
6666
{
67-
$this->executeAction($ids);
6867
$this->registerEntities($ids);
68+
$this->executeAction($ids);
6969
}
7070

7171
/**

‎app/code/Magento/Indexer/Model/Indexer/CacheCleaner.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ private function cleanCache()
9595
$identities = $this->cacheContext->getIdentities();
9696
if (!empty($identities)) {
9797
$this->appCache->clean($identities);
98+
$this->cacheContext->flush();
9899
}
99100
}
100101
}

‎app/code/Magento/Indexer/Model/Processor/CleanCache.php

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,11 @@
55
*/
66
namespace Magento\Indexer\Model\Processor;
77

8-
use \Magento\Framework\App\CacheInterface;
8+
use Magento\Framework\App\CacheInterface;
99

10+
/**
11+
* Clear cache after reindex
12+
*/
1013
class CleanCache
1114
{
1215
/**
@@ -46,9 +49,7 @@ public function __construct(
4649
public function afterUpdateMview(\Magento\Indexer\Model\Processor $subject)
4750
{
4851
$this->eventManager->dispatch('clean_cache_after_reindex', ['object' => $this->context]);
49-
if (!empty($this->context->getIdentities())) {
50-
$this->getCache()->clean($this->context->getIdentities());
51-
}
52+
$this->cleanCache();
5253
}
5354

5455
/**
@@ -61,9 +62,7 @@ public function afterUpdateMview(\Magento\Indexer\Model\Processor $subject)
6162
public function afterReindexAllInvalid(\Magento\Indexer\Model\Processor $subject)
6263
{
6364
$this->eventManager->dispatch('clean_cache_by_tags', ['object' => $this->context]);
64-
if (!empty($this->context->getIdentities())) {
65-
$this->getCache()->clean($this->context->getIdentities());
66-
}
65+
$this->cleanCache();
6766
}
6867

6968
/**
@@ -79,4 +78,18 @@ private function getCache()
7978
}
8079
return $this->cache;
8180
}
81+
82+
/**
83+
* Clean cache.
84+
*
85+
* @return void
86+
*/
87+
private function cleanCache(): void
88+
{
89+
$identities = $this->context->getIdentities();
90+
if (!empty($identities)) {
91+
$this->getCache()->clean($identities);
92+
$this->context->flush();
93+
}
94+
}
8295
}

‎app/code/Magento/Indexer/Test/Unit/Model/CacheContextTest.php

Lines changed: 54 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@
1010
use Magento\Framework\Indexer\CacheContext;
1111
use PHPUnit\Framework\TestCase;
1212

13+
/**
14+
* Test indexer cache context
15+
*/
1316
class CacheContextTest extends TestCase
1417
{
1518
/**
@@ -38,20 +41,62 @@ public function testRegisterEntities()
3841
}
3942

4043
/**
41-
* test getIdentities
44+
* Test getIdentities
45+
*
46+
* @param array $entities
47+
* @param array $tags
48+
* @param array $expected
49+
* @dataProvider getIdentitiesDataProvider
4250
*/
43-
public function testGetIdentities()
51+
public function testGetIdentities(array $entities, array $tags = [], array $expected = []): void
4452
{
45-
$expectedIdentities = [
46-
'product_1', 'product_2', 'product_3', 'category_5', 'category_6', 'category_7',
47-
];
48-
$productTag = 'product';
49-
$categoryTag = 'category';
53+
foreach ($entities as $entity => $ids) {
54+
$this->context->registerEntities($entity, $ids);
55+
}
56+
$this->context->registerTags($tags);
57+
$this->assertEquals($expected, $this->context->getIdentities());
58+
}
59+
60+
/**
61+
* Test that flush() clears all data
62+
*/
63+
public function testFlush(): void
64+
{
65+
$productTag = 'cat_p';
66+
$categoryTag = 'cat_c';
67+
$additionalTags = ['cat_c_p'];
5068
$productIds = [1, 2, 3];
5169
$categoryIds = [5, 6, 7];
5270
$this->context->registerEntities($productTag, $productIds);
5371
$this->context->registerEntities($categoryTag, $categoryIds);
54-
$actualIdentities = $this->context->getIdentities();
55-
$this->assertEquals($expectedIdentities, $actualIdentities);
72+
$this->context->registerTags($additionalTags);
73+
$this->assertNotEmpty($this->context->getIdentities());
74+
$this->context->flush();
75+
$this->assertEmpty($this->context->getIdentities());
76+
}
77+
78+
/**
79+
* @return array[]
80+
*/
81+
public function getIdentitiesDataProvider(): array
82+
{
83+
return [
84+
'should return entities and tags' => [
85+
[
86+
'cat_p' => [1, 2, 3],
87+
'cat_c' => [5, 6, 7]
88+
],
89+
['cat_c_p1', 'cat_c_p2'],
90+
['cat_p_1', 'cat_p_2', 'cat_p_3', 'cat_c_5', 'cat_c_6', 'cat_c_7', 'cat_c_p1', 'cat_c_p2']
91+
],
92+
'should return unique values' => [
93+
[
94+
'cat_p' => [1, 2, 3, 1, 3],
95+
'cat_c' => [5, 6, 7, 6]
96+
],
97+
['cat_c_p1', 'cat_c_p2'],
98+
['cat_p_1', 'cat_p_2', 'cat_p_3', 'cat_c_5', 'cat_c_6', 'cat_c_7', 'cat_c_p1', 'cat_c_p2']
99+
]
100+
];
56101
}
57102
}

‎app/code/Magento/Indexer/Test/Unit/Model/Indexer/CacheCleanerTest.php

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\Indexer\Test\Unit\Model\Indexer;
9+
10+
use Magento\Framework\App\CacheInterface;
11+
use Magento\Framework\Event\Manager;
12+
use Magento\Framework\Indexer\ActionInterface;
13+
use Magento\Framework\Indexer\CacheContext;
14+
use Magento\Indexer\Model\Indexer\CacheCleaner;
15+
use PHPUnit\Framework\MockObject\MockObject;
16+
use PHPUnit\Framework\TestCase;
17+
18+
/**
19+
* Test cache cleaner plugin
20+
*/
21+
class CacheCleanerTest extends TestCase
22+
{
23+
/**
24+
* @var Manager|MockObject
25+
*/
26+
private $eventManager;
27+
/**
28+
* @var CacheContext|MockObject
29+
*/
30+
private $cacheContext;
31+
/**
32+
* @var CacheInterface|MockObject
33+
*/
34+
private $cache;
35+
/**
36+
* @var CacheCleaner
37+
*/
38+
private $model;
39+
/**
40+
* @var ActionInterface|MockObject
41+
*/
42+
private $action;
43+
44+
/**
45+
* @inheritDoc
46+
*/
47+
protected function setUp(): void
48+
{
49+
parent::setUp();
50+
$this->action = $this->getMockForAbstractClass(ActionInterface::class);
51+
$this->cacheContext = $this->createMock(CacheContext::class);
52+
$this->eventManager = $this->createMock(Manager::class);
53+
$this->cache = $this->getMockForAbstractClass(CacheInterface::class);
54+
$this->model = new CacheCleaner(
55+
$this->eventManager,
56+
$this->cacheContext,
57+
$this->cache
58+
);
59+
}
60+
61+
/**
62+
* @param array $tags
63+
* @param bool $isCacheClean
64+
* @dataProvider cacheTagsDataProvider
65+
*/
66+
public function testAfterExecuteFull(array $tags, bool $isCacheClean = true): void
67+
{
68+
$this->expectCacheClean($tags, $isCacheClean);
69+
$this->model->afterExecuteFull($this->action);
70+
}
71+
72+
/**
73+
* @param array $tags
74+
* @param bool $isCacheClean
75+
* @dataProvider cacheTagsDataProvider
76+
*/
77+
public function testAfterExecuteList(array $tags, bool $isCacheClean = true): void
78+
{
79+
$this->expectCacheClean($tags, $isCacheClean);
80+
$this->model->afterExecuteList($this->action);
81+
}
82+
83+
/**
84+
* @param array $tags
85+
* @param bool $isCacheClean
86+
* @dataProvider cacheTagsDataProvider
87+
*/
88+
public function testAfterExecuteRow(array $tags, bool $isCacheClean = true): void
89+
{
90+
$this->expectCacheClean($tags, $isCacheClean);
91+
$this->model->afterExecuteRow($this->action);
92+
}
93+
94+
/**
95+
* @return array[]
96+
*/
97+
public function cacheTagsDataProvider(): array
98+
{
99+
return [
100+
[[], false],
101+
[['cat_c_1', 'cat_c_2'], true]
102+
];
103+
}
104+
105+
/**
106+
* @param array $tags
107+
* @param bool $isCacheClean
108+
*/
109+
private function expectCacheClean(array $tags, bool $isCacheClean = true): void
110+
{
111+
$this->eventManager->expects($this->once())
112+
->method('dispatch')
113+
->with(
114+
'clean_cache_by_tags',
115+
['object' => $this->cacheContext]
116+
);
117+
118+
$this->cacheContext->expects($this->atLeastOnce())
119+
->method('getIdentities')
120+
->willReturn($tags);
121+
122+
$this->cache->expects($this->exactly($isCacheClean ? 1 : 0))
123+
->method('clean')
124+
->with($tags);
125+
126+
$this->cacheContext->expects($this->exactly($isCacheClean ? 1 : 0))
127+
->method('flush');
128+
}
129+
}

‎app/code/Magento/Indexer/Test/Unit/Model/Processor/CleanCacheTest.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@
1717
use PHPUnit\Framework\MockObject\MockObject;
1818
use PHPUnit\Framework\TestCase;
1919

20+
/**
21+
* Test cache clean plugin
22+
*/
2023
class CleanCacheTest extends TestCase
2124
{
2225
/**
@@ -103,6 +106,9 @@ public function testAfterUpdateMview()
103106
->method('clean')
104107
->with($tags);
105108

109+
$this->contextMock->expects($this->once())
110+
->method('flush');
111+
106112
$this->plugin->afterUpdateMview($this->subjectMock);
107113
}
108114
}

‎dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/ProductSearchTest.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use Magento\Catalog\Api\ProductRepositoryInterface;
1414
use Magento\Catalog\Model\Category;
1515
use Magento\Catalog\Model\CategoryLinkManagement;
16+
use Magento\Catalog\Model\Indexer\Product\Category\Processor;
1617
use Magento\Catalog\Model\Product;
1718
use Magento\Catalog\Model\ResourceModel\Category\Collection;
1819
use Magento\Eav\Api\Data\AttributeOptionInterface;
@@ -1170,6 +1171,11 @@ public function testSortByPosition()
11701171
$category->setPostedProducts($productPositions);
11711172
$category->save();
11721173

1174+
// Reindex products from the result to invalidate query cache.
1175+
/** @var $indexer Processor */
1176+
$indexer = Bootstrap::getObjectManager()->get(Processor::class);
1177+
$indexer->reindexList(array_keys($productPositions));
1178+
11731179
$queryDesc = <<<QUERY
11741180
{
11751181
products(filter: {category_id: {eq: "$categoryId"}}, sort: {position: ASC}) {

‎lib/internal/Magento/Framework/Indexer/CacheContext.php

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,18 @@ public function getIdentities()
7070
$identities = [];
7171
foreach ($this->entities as $cacheTag => $ids) {
7272
foreach ($ids as $id) {
73-
$identities[] = $cacheTag . '_' . $id;
73+
$identities[$cacheTag . '_' . $id] = true;
7474
}
7575
}
76-
return array_merge($identities, array_unique($this->tags));
76+
return array_merge(array_keys($identities), array_unique($this->tags));
77+
}
78+
79+
/**
80+
* Clear context data
81+
*/
82+
public function flush(): void
83+
{
84+
$this->tags = [];
85+
$this->entities = [];
7786
}
7887
}

0 commit comments

Comments
 (0)