Skip to content

Commit a08360a

Browse files
cparticacpartica
authored
Merge pull request #3684 from magento-honey-badgers/MC-10891-adding-cache-flush-on-static-clear
[honey] MC-5926: Conflict of simultaneous write in Redis cache
2 parents d700436 + 2c86d78 commit a08360a

File tree

25 files changed

+1018
-211
lines changed

25 files changed

+1018
-211
lines changed

‎app/code/Magento/Config/App/Config/Type/System.php‎

Lines changed: 116 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use Magento\Framework\App\Config\Spi\PreProcessorInterface;
1212
use Magento\Framework\App\ObjectManager;
1313
use Magento\Config\App\Config\Type\System\Reader;
14+
use Magento\Framework\Lock\LockManagerInterface;
1415
use Magento\Framework\Serialize\Serializer\Sensitive as SensitiveSerializer;
1516
use Magento\Framework\Serialize\Serializer\SensitiveFactory as SensitiveSerializerFactory;
1617
use Magento\Framework\App\ScopeInterface;
@@ -24,12 +25,43 @@
2425
*
2526
* @api
2627
* @since 100.1.2
28+
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
2729
*/
2830
class System implements ConfigTypeInterface
2931
{
32+
/**
33+
* Config cache tag.
34+
*/
3035
const CACHE_TAG = 'config_scopes';
36+
37+
/**
38+
* System config type.
39+
*/
3140
const CONFIG_TYPE = 'system';
3241

42+
/**
43+
* @var string
44+
*/
45+
private static $lockName = 'SYSTEM_CONFIG';
46+
47+
/**
48+
* Timeout between retrieves to load the configuration from the cache.
49+
*
50+
* Value of the variable in microseconds.
51+
*
52+
* @var int
53+
*/
54+
private static $delayTimeout = 50000;
55+
56+
/**
57+
* Lifetime of the lock for write in cache.
58+
*
59+
* Value of the variable in seconds.
60+
*
61+
* @var int
62+
*/
63+
private static $lockTimeout = 8;
64+
3365
/**
3466
* @var array
3567
*/
@@ -71,6 +103,11 @@ class System implements ConfigTypeInterface
71103
*/
72104
private $availableDataScopes;
73105

106+
/**
107+
* @var LockManagerInterface
108+
*/
109+
private $locker;
110+
74111
/**
75112
* @param ConfigSourceInterface $source
76113
* @param PostProcessorInterface $postProcessor
@@ -82,7 +119,7 @@ class System implements ConfigTypeInterface
82119
* @param string $configType
83120
* @param Reader $reader
84121
* @param SensitiveSerializerFactory|null $sensitiveFactory
85-
*
122+
* @param LockManagerInterface|null $locker
86123
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
87124
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
88125
*/
@@ -96,7 +133,8 @@ public function __construct(
96133
$cachingNestedLevel = 1,
97134
$configType = self::CONFIG_TYPE,
98135
Reader $reader = null,
99-
SensitiveSerializerFactory $sensitiveFactory = null
136+
SensitiveSerializerFactory $sensitiveFactory = null,
137+
LockManagerInterface $locker = null
100138
) {
101139
$this->postProcessor = $postProcessor;
102140
$this->cache = $cache;
@@ -110,6 +148,7 @@ public function __construct(
110148
$this->serializer = $sensitiveFactory->create(
111149
['serializer' => $serializer]
112150
);
151+
$this->locker = $locker ?: ObjectManager::getInstance()->get(LockManagerInterface::class);
113152
}
114153

115154
/**
@@ -153,7 +192,7 @@ private function getWithParts($path)
153192

154193
if (count($pathParts) === 1 && $pathParts[0] !== ScopeInterface::SCOPE_DEFAULT) {
155194
if (!isset($this->data[$pathParts[0]])) {
156-
$data = $this->readData();
195+
$data = $this->loadAllData();
157196
$this->data = array_replace_recursive($data, $this->data);
158197
}
159198

@@ -186,21 +225,60 @@ private function getWithParts($path)
186225
}
187226

188227
/**
189-
* Load configuration data for all scopes
228+
* Make lock on data load.
190229
*
230+
* @param callable $dataLoader
231+
* @param bool $flush
191232
* @return array
192233
*/
193-
private function loadAllData()
234+
private function lockedLoadData(callable $dataLoader, bool $flush = false): array
194235
{
195-
$cachedData = $this->cache->load($this->configType);
236+
$cachedData = $dataLoader(); //optimistic read
196237

197-
if ($cachedData === false) {
198-
$data = $this->readData();
199-
} else {
200-
$data = $this->serializer->unserialize($cachedData);
238+
while ($cachedData === false && $this->locker->isLocked(self::$lockName)) {
239+
usleep(self::$delayTimeout);
240+
$cachedData = $dataLoader();
201241
}
202242

203-
return $data;
243+
while ($cachedData === false) {
244+
try {
245+
if ($this->locker->lock(self::$lockName, self::$lockTimeout)) {
246+
if (!$flush) {
247+
$data = $this->readData();
248+
$this->cacheData($data);
249+
$cachedData = $data;
250+
} else {
251+
$this->cache->clean(\Zend_Cache::CLEANING_MODE_MATCHING_TAG, [self::CACHE_TAG]);
252+
$cachedData = [];
253+
}
254+
}
255+
} finally {
256+
$this->locker->unlock(self::$lockName);
257+
}
258+
259+
if ($cachedData === false) {
260+
usleep(self::$delayTimeout);
261+
$cachedData = $dataLoader();
262+
}
263+
}
264+
265+
return $cachedData;
266+
}
267+
268+
/**
269+
* Load configuration data for all scopes
270+
*
271+
* @return array
272+
*/
273+
private function loadAllData()
274+
{
275+
return $this->lockedLoadData(function () {
276+
$cachedData = $this->cache->load($this->configType);
277+
if ($cachedData === false) {
278+
return $cachedData;
279+
}
280+
return $this->serializer->unserialize($cachedData);
281+
});
204282
}
205283

206284
/**
@@ -211,16 +289,13 @@ private function loadAllData()
211289
*/
212290
private function loadDefaultScopeData($scopeType)
213291
{
214-
$cachedData = $this->cache->load($this->configType . '_' . $scopeType);
215-
216-
if ($cachedData === false) {
217-
$data = $this->readData();
218-
$this->cacheData($data);
219-
} else {
220-
$data = [$scopeType => $this->serializer->unserialize($cachedData)];
221-
}
222-
223-
return $data;
292+
return $this->lockedLoadData(function () use ($scopeType) {
293+
$cachedData = $this->cache->load($this->configType . '_' . $scopeType);
294+
if ($cachedData === false) {
295+
return $cachedData;
296+
}
297+
return [$scopeType => $this->serializer->unserialize($cachedData)];
298+
});
224299
}
225300

226301
/**
@@ -232,25 +307,22 @@ private function loadDefaultScopeData($scopeType)
232307
*/
233308
private function loadScopeData($scopeType, $scopeId)
234309
{
235-
$cachedData = $this->cache->load($this->configType . '_' . $scopeType . '_' . $scopeId);
236-
237-
if ($cachedData === false) {
238-
if ($this->availableDataScopes === null) {
239-
$cachedScopeData = $this->cache->load($this->configType . '_scopes');
240-
if ($cachedScopeData !== false) {
241-
$this->availableDataScopes = $this->serializer->unserialize($cachedScopeData);
310+
return $this->lockedLoadData(function () use ($scopeType, $scopeId) {
311+
$cachedData = $this->cache->load($this->configType . '_' . $scopeType . '_' . $scopeId);
312+
if ($cachedData === false) {
313+
if ($this->availableDataScopes === null) {
314+
$cachedScopeData = $this->cache->load($this->configType . '_scopes');
315+
if ($cachedScopeData !== false) {
316+
$this->availableDataScopes = $this->serializer->unserialize($cachedScopeData);
317+
}
242318
}
319+
if (is_array($this->availableDataScopes) && !isset($this->availableDataScopes[$scopeType][$scopeId])) {
320+
return [$scopeType => [$scopeId => []]];
321+
}
322+
return false;
243323
}
244-
if (is_array($this->availableDataScopes) && !isset($this->availableDataScopes[$scopeType][$scopeId])) {
245-
return [$scopeType => [$scopeId => []]];
246-
}
247-
$data = $this->readData();
248-
$this->cacheData($data);
249-
} else {
250-
$data = [$scopeType => [$scopeId => $this->serializer->unserialize($cachedData)]];
251-
}
252-
253-
return $data;
324+
return [$scopeType => [$scopeId => $this->serializer->unserialize($cachedData)]];
325+
});
254326
}
255327

256328
/**
@@ -340,6 +412,11 @@ private function readData(): array
340412
public function clean()
341413
{
342414
$this->data = [];
343-
$this->cache->clean(\Zend_Cache::CLEANING_MODE_MATCHING_TAG, [self::CACHE_TAG]);
415+
$this->lockedLoadData(
416+
function () {
417+
return false;
418+
},
419+
true
420+
);
344421
}
345422
}

‎app/code/Magento/Config/Setup/ConfigOptionsList.php‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
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\Config\Setup;
9+
10+
use Magento\Framework\App\DeploymentConfig;
11+
use Magento\Framework\Config\Data\ConfigDataFactory;
12+
use Magento\Framework\Config\File\ConfigFilePool;
13+
use Magento\Framework\Setup\ConfigOptionsListInterface;
14+
use Magento\Framework\Setup\Option\SelectConfigOption;
15+
16+
/**
17+
* Deployment configuration options required for the Config module.
18+
*/
19+
class ConfigOptionsList implements ConfigOptionsListInterface
20+
{
21+
/**
22+
* Input key for the option.
23+
*/
24+
const INPUT_KEY_DEBUG_LOGGING = 'enable-debug-logging';
25+
26+
/**
27+
* Path to the value in the deployment config.
28+
*/
29+
const CONFIG_PATH_DEBUG_LOGGING = 'dev/debug/debug_logging';
30+
31+
/**
32+
* @var ConfigDataFactory
33+
*/
34+
private $configDataFactory;
35+
36+
/**
37+
* @param ConfigDataFactory $configDataFactory
38+
*/
39+
public function __construct(ConfigDataFactory $configDataFactory)
40+
{
41+
$this->configDataFactory = $configDataFactory;
42+
}
43+
44+
/**
45+
* @inheritdoc
46+
*/
47+
public function getOptions()
48+
{
49+
return [
50+
new SelectConfigOption(
51+
self::INPUT_KEY_DEBUG_LOGGING,
52+
SelectConfigOption::FRONTEND_WIZARD_RADIO,
53+
[true, false, 1, 0],
54+
self::CONFIG_PATH_DEBUG_LOGGING,
55+
'Enable debug logging'
56+
)
57+
];
58+
}
59+
60+
/**
61+
* @inheritdoc
62+
*/
63+
public function createConfig(array $options, DeploymentConfig $deploymentConfig)
64+
{
65+
$config = [];
66+
if (isset($options[self::INPUT_KEY_DEBUG_LOGGING])) {
67+
$configData = $this->configDataFactory->create(ConfigFilePool::APP_ENV);
68+
if ($options[self::INPUT_KEY_DEBUG_LOGGING] === 'true'
69+
|| $options[self::INPUT_KEY_DEBUG_LOGGING] === '1') {
70+
$value = 1;
71+
} else {
72+
$value = 0;
73+
}
74+
$configData->set(self::CONFIG_PATH_DEBUG_LOGGING, $value);
75+
$config[] = $configData;
76+
}
77+
78+
return $config;
79+
}
80+
81+
/**
82+
* @inheritdoc
83+
*/
84+
public function validate(array $options, DeploymentConfig $deploymentConfig)
85+
{
86+
return [];
87+
}
88+
}

‎app/code/Magento/Config/etc/di.xml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,11 @@
7777
</argument>
7878
</arguments>
7979
</type>
80+
<type name="Magento\Framework\Lock\Backend\Cache">
81+
<arguments>
82+
<argument name="cache" xsi:type="object">Magento\Framework\App\Cache\Type\Config</argument>
83+
</arguments>
84+
</type>
8085
<type name="Magento\Config\App\Config\Type\System">
8186
<arguments>
8287
<argument name="source" xsi:type="object">systemConfigSourceAggregatedProxy</argument>
@@ -85,6 +90,7 @@
8590
<argument name="preProcessor" xsi:type="object">Magento\Framework\App\Config\PreProcessorComposite</argument>
8691
<argument name="serializer" xsi:type="object">Magento\Framework\Serialize\Serializer\Serialize</argument>
8792
<argument name="reader" xsi:type="object">Magento\Config\App\Config\Type\System\Reader\Proxy</argument>
93+
<argument name="locker" xsi:type="object">Magento\Framework\Lock\Backend\Cache</argument>
8894
</arguments>
8995
</type>
9096
<type name="Magento\Config\App\Config\Type\System\Reader">

0 commit comments

Comments
 (0)