Skip to content

Commit f5fba63

Browse files
vzabaznovvzabaznov
authored
Merge pull request #5967 from magento-performance/MC-35903-2
MC-35903 [2.3]
2 parents 6702be5 + 030b74d commit f5fba63

File tree

6 files changed

+321
-28
lines changed

6 files changed

+321
-28
lines changed

‎app/code/Magento/MessageQueue/Console/StartConsumerCommand.php

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,15 +79,11 @@ protected function execute(InputInterface $input, OutputInterface $output)
7979

8080
$singleThread = $input->getOption(self::OPTION_SINGLE_THREAD);
8181

82-
if ($singleThread && $this->lockManager->isLocked(md5($consumerName))) { //phpcs:ignore
82+
if ($singleThread && !$this->lockManager->lock(md5($consumerName),0)) { //phpcs:ignore
8383
$output->writeln('<error>Consumer with the same name is running</error>');
8484
return \Magento\Framework\Console\Cli::RETURN_FAILURE;
8585
}
8686

87-
if ($singleThread) {
88-
$this->lockManager->lock(md5($consumerName)); //phpcs:ignore
89-
}
90-
9187
$this->appState->setAreaCode($areaCode ?? 'global');
9288

9389
$consumer = $this->consumerFactory->get($consumerName, $batchSize);
@@ -163,7 +159,7 @@ protected function configure()
163159
To specify the preferred area:
164160
165161
<comment>%command.full_name% someConsumer --area-code='adminhtml'</comment>
166-
162+
167163
To do not run multiple copies of one consumer simultaneously:
168164
169165
<comment>%command.full_name% someConsumer --single-thread'</comment>

‎app/code/Magento/MessageQueue/Test/Unit/Console/StartConsumerCommandTest.php

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,6 @@ public function testExecute(
9191
$pidFilePath,
9292
$singleThread,
9393
$lockExpects,
94-
$isLockedExpects,
9594
$isLocked,
9695
$unlockExpects,
9796
$runProcessExpects,
@@ -129,14 +128,11 @@ public function testExecute(
129128
->method('get')->with($consumerName, $batchSize)->willReturn($consumer);
130129
$consumer->expects($this->exactly($runProcessExpects))->method('process')->with($numberOfMessages);
131130

132-
$this->lockManagerMock->expects($this->exactly($isLockedExpects))
133-
->method('isLocked')
134-
->with(md5($consumerName)) //phpcs:ignore
135-
->willReturn($isLocked);
136-
137131
$this->lockManagerMock->expects($this->exactly($lockExpects))
138132
->method('lock')
139-
->with(md5($consumerName)); //phpcs:ignore
133+
->with(md5($consumerName))//phpcs:ignore
134+
->willReturn($isLocked);
135+
140136
$this->lockManagerMock->expects($this->exactly($unlockExpects))
141137
->method('unlock')
142138
->with(md5($consumerName)); //phpcs:ignore
@@ -157,8 +153,7 @@ public function executeDataProvider()
157153
'pidFilePath' => null,
158154
'singleThread' => false,
159155
'lockExpects' => 0,
160-
'isLockedExpects' => 0,
161-
'isLocked' => false,
156+
'isLocked' => true,
162157
'unlockExpects' => 0,
163158
'runProcessExpects' => 1,
164159
'expectedReturn' => \Magento\Framework\Console\Cli::RETURN_SUCCESS,
@@ -167,18 +162,16 @@ public function executeDataProvider()
167162
'pidFilePath' => '/var/consumer.pid',
168163
'singleThread' => true,
169164
'lockExpects' => 1,
170-
'isLockedExpects' => 1,
171-
'isLocked' => false,
165+
'isLocked' => true,
172166
'unlockExpects' => 1,
173167
'runProcessExpects' => 1,
174168
'expectedReturn' => \Magento\Framework\Console\Cli::RETURN_SUCCESS,
175169
],
176170
[
177171
'pidFilePath' => '/var/consumer.pid',
178172
'singleThread' => true,
179-
'lockExpects' => 0,
180-
'isLockedExpects' => 1,
181-
'isLocked' => true,
173+
'lockExpects' => 1,
174+
'isLocked' => false,
182175
'unlockExpects' => 0,
183176
'runProcessExpects' => 0,
184177
'expectedReturn' => \Magento\Framework\Console\Cli::RETURN_FAILURE,

‎dev/tests/integration/testsuite/Magento/Framework/Lock/Backend/CacheTest.php

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
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\Framework\Lock\Backend;
9+
10+
use Magento\Framework\Lock\Backend\Cache;
11+
12+
/**
13+
* \Magento\Framework\Lock\Backend\Cache test case.
14+
*/
15+
class CacheTest extends \PHPUnit\Framework\TestCase
16+
{
17+
/**
18+
* @var Cache
19+
*/
20+
private $cacheInstance1;
21+
22+
/**
23+
* @var Cache
24+
*/
25+
private $cacheInstance2;
26+
27+
/**
28+
* @inheritDoc
29+
*/
30+
protected function setUp(): void
31+
{
32+
$objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();
33+
34+
$frontendInterface1 = $objectManager->create(\Magento\Framework\App\Cache\Type\Config::class);
35+
$this->cacheInstance1 = new Cache($frontendInterface1);
36+
37+
$frontendInterface2 = $objectManager->create(\Magento\Framework\App\Cache\Type\Config::class);
38+
$this->cacheInstance2 = new Cache($frontendInterface2);
39+
}
40+
41+
/**
42+
* Verify lock mechanism in general.
43+
*
44+
* @return void
45+
*/
46+
public function testParallelLock(): void
47+
{
48+
$identifier1 = \uniqid('lock_name_1_', true);
49+
50+
$this->assertTrue($this->cacheInstance1->lock($identifier1));
51+
52+
$this->assertFalse($this->cacheInstance1->lock($identifier1, 0));
53+
$this->assertFalse($this->cacheInstance2->lock($identifier1, 0));
54+
}
55+
56+
/**
57+
* Verify that lock will be released after timeout expiration.
58+
*
59+
* @return void
60+
*/
61+
public function testParallelLockExpired(): void
62+
{
63+
$testLifeTime = 2;
64+
\Closure::bind(function (Cache $class) use ($testLifeTime) {
65+
$class->defaultLifetime = $testLifeTime;
66+
}, null, $this->cacheInstance1)($this->cacheInstance1);
67+
68+
$identifier1 = \uniqid('lock_name_1_', true);
69+
70+
$this->assertTrue($this->cacheInstance1->lock($identifier1, 0));
71+
$this->assertTrue($this->cacheInstance2->lock($identifier1, $testLifeTime + 1));
72+
73+
$this->cacheInstance2->unlock($identifier1);
74+
}
75+
76+
/**
77+
* Verify that lock will not be released by another lock name.
78+
*
79+
* @return void
80+
*/
81+
public function testParallelUnlock(): void
82+
{
83+
$identifier1 = \uniqid('lock_name_1_', true);
84+
$identifier2 = \uniqid('lock_name_2_', true);
85+
86+
$this->assertTrue($this->cacheInstance1->lock($identifier1, 30));
87+
$this->assertTrue($this->cacheInstance2->lock($identifier2, 30));
88+
89+
$this->assertFalse($this->cacheInstance2->unlock($identifier1));
90+
$this->assertTrue($this->cacheInstance2->unlock($identifier2));
91+
92+
$this->assertTrue($this->cacheInstance2->isLocked($identifier1));
93+
$this->assertFalse($this->cacheInstance2->isLocked($identifier2));
94+
}
95+
96+
/**
97+
* Verify that lock will not be released by another lock name when both locks will never be expired.
98+
*
99+
* @return void
100+
*/
101+
public function testParallelUnlockNoExpiration(): void
102+
{
103+
$identifier1 = \uniqid('lock_name_1_', true);
104+
$identifier2 = \uniqid('lock_name_2_', true);
105+
106+
$this->assertTrue($this->cacheInstance1->lock($identifier1, -1));
107+
$this->assertTrue($this->cacheInstance2->lock($identifier2, -1));
108+
109+
$this->assertFalse($this->cacheInstance2->unlock($identifier1));
110+
$this->assertTrue($this->cacheInstance2->unlock($identifier2));
111+
112+
$this->assertTrue($this->cacheInstance2->isLocked($identifier1));
113+
$this->assertFalse($this->cacheInstance2->isLocked($identifier2));
114+
}
115+
}

‎lib/internal/Magento/Framework/Cache/LockGuardedCacheLoader.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,14 +91,14 @@ public function lockedLoadData(
9191
callable $dataSaver
9292
) {
9393
$cachedData = $dataLoader(); //optimistic read
94-
$deadline = microtime(true) + $this->loadTimeout / 100;
94+
$deadline = microtime(true) + $this->loadTimeout / 1000;
9595

9696
while ($cachedData === false) {
9797
if ($deadline <= microtime(true)) {
9898
return $dataCollector();
9999
}
100100

101-
if ($this->locker->lock($lockName, $this->lockTimeout / 1000)) {
101+
if ($this->locker->lock($lockName, 0)) {
102102
try {
103103
$data = $dataCollector();
104104
$dataSaver($data);

‎lib/internal/Magento/Framework/Cache/Test/Unit/LockGuardedCacheLoaderTest.php

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
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\Framework\Cache\Test\Unit;
9+
10+
use Magento\Framework\Cache\LockGuardedCacheLoader;
11+
use Magento\Framework\Lock\LockManagerInterface;
12+
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper;
13+
use PHPUnit\Framework\MockObject\MockObject;
14+
use PHPUnit\Framework\TestCase;
15+
16+
class LockGuardedCacheLoaderTest extends TestCase
17+
{
18+
/**
19+
* @var LockManagerInterface|MockObject
20+
*/
21+
private $lockManagerInterfaceMock;
22+
23+
/**
24+
* @var LockGuardedCacheLoader
25+
*/
26+
private $LockGuardedCacheLoader;
27+
28+
/**
29+
* @inheritDoc
30+
*/
31+
protected function setUp(): void
32+
{
33+
$this->lockManagerInterfaceMock = $this->getMockForAbstractClass(LockManagerInterface::class);
34+
35+
$objectManager = new ObjectManagerHelper($this);
36+
37+
$this->LockGuardedCacheLoader = $objectManager->getObject(
38+
LockGuardedCacheLoader::class,
39+
[
40+
'locker' => $this->lockManagerInterfaceMock
41+
]
42+
);
43+
}
44+
45+
/**
46+
* Verify optimistic data read from cache.
47+
*
48+
* @return void
49+
*/
50+
public function testOptimisticDataRead(): void
51+
{
52+
$lockName = \uniqid('lock_name_1_', true);
53+
54+
$dataLoader = function () {
55+
return 'loaded_data';
56+
};
57+
58+
$dataCollector = function () {
59+
return true;
60+
};
61+
62+
$dataSaver = function () {
63+
return true;
64+
};
65+
66+
$this->lockManagerInterfaceMock->expects($this->never())->method('lock');
67+
$this->lockManagerInterfaceMock->expects($this->never())->method('unlock');
68+
69+
$this->assertEquals(
70+
'loaded_data',
71+
$this->LockGuardedCacheLoader->lockedLoadData($lockName, $dataLoader, $dataCollector, $dataSaver)
72+
);
73+
}
74+
75+
/**
76+
* Verify data is collected when deadline to read from cache is reached.
77+
*
78+
* @return void
79+
*/
80+
public function testDataCollectedAfterDeadlineReached(): void
81+
{
82+
$lockName = \uniqid('lock_name_1_', true);
83+
84+
$dataLoader = function () {
85+
return false;
86+
};
87+
88+
$dataCollector = function () {
89+
return 'collected_data';
90+
};
91+
92+
$dataSaver = function () {
93+
return true;
94+
};
95+
96+
$this->lockManagerInterfaceMock
97+
->expects($this->atLeastOnce())->method('lock')
98+
->with($lockName, 0)
99+
->willReturn(false);
100+
101+
$this->lockManagerInterfaceMock->expects($this->never())->method('unlock');
102+
103+
$this->assertEquals(
104+
'collected_data',
105+
$this->LockGuardedCacheLoader->lockedLoadData($lockName, $dataLoader, $dataCollector, $dataSaver)
106+
);
107+
}
108+
109+
/**
110+
* Verify data write to cache.
111+
*
112+
* @return void
113+
*/
114+
public function testDataWrite(): void
115+
{
116+
$lockName = \uniqid('lock_name_1_', true);
117+
118+
$dataLoader = function () {
119+
return false;
120+
};
121+
122+
$dataCollector = function () {
123+
return 'collected_data';
124+
};
125+
126+
$dataSaver = function () {
127+
return true;
128+
};
129+
130+
$this->lockManagerInterfaceMock
131+
->expects($this->once())->method('lock')
132+
->with($lockName, 0)
133+
->willReturn(true);
134+
135+
$this->lockManagerInterfaceMock->expects($this->once())->method('unlock');
136+
137+
$this->assertEquals(
138+
'collected_data',
139+
$this->LockGuardedCacheLoader->lockedLoadData($lockName, $dataLoader, $dataCollector, $dataSaver)
140+
);
141+
}
142+
}

0 commit comments

Comments
 (0)