Skip to content

Commit 4c9adb5

Browse files
committed
MC-35903: Refactor place where lock mechanism is used regarding proper lock description
1 parent 89d3cda commit 4c9adb5

File tree

4 files changed

+48
-43
lines changed

4 files changed

+48
-43
lines changed

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

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -79,22 +79,20 @@ 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
83-
$output->writeln('<error>Consumer with the same name is running</error>');
84-
return \Magento\Framework\Console\Cli::RETURN_FAILURE;
85-
}
86-
87-
if ($singleThread) {
88-
$this->lockManager->lock(md5($consumerName)); //phpcs:ignore
89-
}
90-
91-
$this->appState->setAreaCode($areaCode ?? 'global');
92-
93-
$consumer = $this->consumerFactory->get($consumerName, $batchSize);
94-
$consumer->process($numberOfMessages);
95-
96-
if ($singleThread) {
97-
$this->lockManager->unlock(md5($consumerName)); //phpcs:ignore
82+
try {
83+
if ($singleThread && !$this->lockManager->lock(md5($consumerName),0)) { //phpcs:ignore
84+
$output->writeln('<error>Consumer with the same name is running</error>');
85+
return \Magento\Framework\Console\Cli::RETURN_FAILURE;
86+
}
87+
88+
$this->appState->setAreaCode($areaCode ?? 'global');
89+
90+
$consumer = $this->consumerFactory->get($consumerName, $batchSize);
91+
$consumer->process($numberOfMessages);
92+
} finally {
93+
if ($singleThread) {
94+
$this->lockManager->unlock(md5($consumerName)); //phpcs:ignore
95+
}
9896
}
9997

10098
return \Magento\Framework\Console\Cli::RETURN_SUCCESS;
@@ -163,7 +161,7 @@ protected function configure()
163161
To specify the preferred area:
164162
165163
<comment>%command.full_name% someConsumer --area-code='adminhtml'</comment>
166-
164+
167165
To do not run multiple copies of one consumer simultaneously:
168166
169167
<comment>%command.full_name% someConsumer --single-thread'</comment>

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

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -47,16 +47,10 @@ public function testParallelLock(): void
4747
{
4848
$identifier1 = \uniqid('lock_name_1_', true);
4949

50-
$this->assertTrue($this->cacheInstance1->lock($identifier1, 2));
50+
$this->assertTrue($this->cacheInstance1->lock($identifier1));
5151

52-
$this->assertFalse($this->cacheInstance1->lock($identifier1, 2));
53-
$this->assertFalse($this->cacheInstance2->lock($identifier1, 2));
54-
sleep(4);
55-
$this->assertFalse($this->cacheInstance1->isLocked($identifier1));
56-
57-
$this->assertTrue($this->cacheInstance2->lock($identifier1, -1));
58-
sleep(4);
59-
$this->assertTrue($this->cacheInstance1->isLocked($identifier1));
52+
$this->assertFalse($this->cacheInstance1->lock($identifier1, 0));
53+
$this->assertFalse($this->cacheInstance2->lock($identifier1, 0));
6054
}
6155

6256
/**
@@ -66,19 +60,16 @@ public function testParallelLock(): void
6660
*/
6761
public function testParallelLockExpired(): void
6862
{
69-
$identifier1 = \uniqid('lock_name_1_', true);
63+
$lifeTime = \Closure::bind(function (Cache $class) {
64+
return $class->defaultLifetime;
65+
}, null, $this->cacheInstance1)($this->cacheInstance1);
7066

71-
$this->assertTrue($this->cacheInstance1->lock($identifier1, 1));
72-
sleep(2);
73-
$this->assertFalse($this->cacheInstance1->isLocked($identifier1));
67+
$identifier1 = \uniqid('lock_name_1_', true);
7468

75-
$this->assertTrue($this->cacheInstance1->lock($identifier1, 1));
76-
sleep(2);
77-
$this->assertFalse($this->cacheInstance1->isLocked($identifier1));
69+
$this->assertTrue($this->cacheInstance1->lock($identifier1, 0));
70+
$this->assertTrue($this->cacheInstance2->lock($identifier1, $lifeTime + 1));
7871

79-
$this->assertTrue($this->cacheInstance2->lock($identifier1, 1));
80-
sleep(2);
81-
$this->assertFalse($this->cacheInstance1->isLocked($identifier1));
72+
$this->cacheInstance2->unlock($identifier1);
8273
}
8374

8475
/**

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ public function lockedLoadData(
131131
return $dataCollector();
132132
}
133133

134-
if ($this->locker->lock($lockName, $this->lockTimeout / 1000)) {
134+
if ($this->locker->lock($lockName, 0)) {
135135
try {
136136
$data = $dataCollector();
137137
$dataSaver($data);

lib/internal/Magento/Framework/Lock/Backend/Cache.php

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,20 @@ class Cache implements \Magento\Framework\Lock\LockManagerInterface
3131
*/
3232
private $lockSign;
3333

34+
/**
35+
* How many microseconds to wait before re-try to acquire a lock
36+
*
37+
* @var int
38+
*/
39+
private $sleepCycle = 100000;
40+
41+
/**
42+
* Lifetime of lock data in seconds.
43+
*
44+
* @var int
45+
*/
46+
private $defaultLifetime = 10;
47+
3448
/**
3549
* @param FrontendInterface $cache
3650
*/
@@ -49,14 +63,16 @@ public function lock(string $name, int $timeout = -1): bool
4963
$this->lockSign = $this->generateLockSign();
5064
}
5165

52-
$data = $this->cache->load($this->getIdentifier($name));
53-
54-
if (false !== $data) {
55-
return false;
66+
$skipDeadline = $timeout < 0;
67+
$deadline = microtime(true) + $timeout;
68+
while ($this->cache->load($this->getIdentifier($name))) {
69+
if (!$skipDeadline && $deadline <= microtime(true)) {
70+
return false;
71+
}
72+
usleep($this->sleepCycle);
5673
}
5774

58-
$timeout = $timeout <= 0 ? null : $timeout;
59-
$this->cache->save($this->lockSign, $this->getIdentifier($name), [], $timeout);
75+
$this->cache->save($this->lockSign, $this->getIdentifier($name), [], $this->defaultLifetime);
6076

6177
$data = $this->cache->load($this->getIdentifier($name));
6278

0 commit comments

Comments
 (0)