Skip to content

Commit c8fd00a

Browse files
authored
Merge pull request #553 from magento-performance/ACPT-1413
ACPT-1413: Child process mustn't close database and redis connections in parent
2 parents 5ca31cf + 8868ebe commit c8fd00a

File tree

6 files changed

+52
-40
lines changed

6 files changed

+52
-40
lines changed

lib/internal/Magento/Framework/App/ResourceConnection.php

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ public function closeConnection($resourceName = self::DEFAULT_CONNECTION)
128128
}
129129
$this->connections = [];
130130
} else {
131-
$processConnectionName = $this->getProcessConnectionName($this->config->getConnectionName($resourceName));
131+
$processConnectionName = $this->config->getConnectionName($resourceName);
132132
if (isset($this->connections[$processConnectionName])) {
133133
if ($this->connections[$processConnectionName] !== null) {
134134
$this->connections[$processConnectionName]->closeConnection();
@@ -147,9 +147,8 @@ public function closeConnection($resourceName = self::DEFAULT_CONNECTION)
147147
*/
148148
public function getConnectionByName($connectionName)
149149
{
150-
$processConnectionName = $this->getProcessConnectionName($connectionName);
151-
if (isset($this->connections[$processConnectionName])) {
152-
return $this->connections[$processConnectionName];
150+
if (isset($this->connections[$connectionName])) {
151+
return $this->connections[$connectionName];
153152
}
154153

155154
$connectionConfig = $this->deploymentConfig->get(
@@ -162,21 +161,10 @@ public function getConnectionByName($connectionName)
162161
throw new \DomainException('Connection "' . $connectionName . '" is not defined');
163162
}
164163

165-
$this->connections[$processConnectionName] = $connection;
164+
$this->connections[$connectionName] = $connection;
166165
return $connection;
167166
}
168167

169-
/**
170-
* Get conneciton name for process.
171-
*
172-
* @param string $connectionName
173-
* @return string
174-
*/
175-
private function getProcessConnectionName($connectionName)
176-
{
177-
return $connectionName . '_process_' . getmypid();
178-
}
179-
180168
/**
181169
* Get resource table name, validated by db adapter.
182170
*

lib/internal/Magento/Framework/Cache/Frontend/Adapter/Zend.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,13 @@ class Zend implements \Magento\Framework\Cache\FrontendInterface
2929
*/
3030
private $pid;
3131

32+
/**
33+
* We need to keep references to parent's frontends so that they don't get destroyed
34+
*
35+
* @var array
36+
*/
37+
private $parentFrontends = [];
38+
3239
/**
3340
* @param \Closure $frontendFactory
3441
*/
@@ -147,6 +154,9 @@ private function getFrontEnd()
147154
if (getmypid() === $this->pid) {
148155
return $this->_frontend;
149156
}
157+
// Note: We hide the parent process's _frontend so that the destructor won't get called on it.
158+
// If the destructor were called, then the parent process's connection would be disconnected.
159+
$this->parentFrontends[] = $this->_frontend;
150160
$frontendFactory = $this->frontendFactory;
151161
$this->_frontend = $frontendFactory();
152162
$this->pid = getmypid();

lib/internal/Magento/Framework/Config/Data/Scoped.php

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -22,27 +22,6 @@ class Scoped extends \Magento\Framework\Config\Data
2222
*/
2323
protected $_configScope;
2424

25-
/**
26-
* Configuration reader
27-
*
28-
* @var \Magento\Framework\Config\ReaderInterface
29-
*/
30-
protected $_reader;
31-
32-
/**
33-
* Configuration cache
34-
*
35-
* @var \Magento\Framework\Config\CacheInterface
36-
*/
37-
protected $_cache;
38-
39-
/**
40-
* Cache tag
41-
*
42-
* @var string
43-
*/
44-
protected $_cacheId;
45-
4625
/**
4726
* Scope priority loading scheme
4827
*

lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,20 @@ class Mysql extends \Zend_Db_Adapter_Pdo_Mysql implements AdapterInterface, Rese
237237
*/
238238
private $schemaListener;
239239

240+
/**
241+
* Process id that the connection is associated with
242+
*
243+
* @var int|null
244+
*/
245+
private ?int $pid = null;
246+
247+
/**
248+
* Parent process's database connection
249+
*
250+
* @var array
251+
*/
252+
private $parentConnections = [];
253+
240254
/**
241255
* Constructor
242256
*
@@ -255,6 +269,7 @@ public function __construct(
255269
array $config = [],
256270
SerializerInterface $serializer = null
257271
) {
272+
$this->pid = getmypid();
258273
$this->string = $string;
259274
$this->dateTime = $dateTime;
260275
$this->logger = $logger;
@@ -295,6 +310,7 @@ public function _resetState() : void
295310
$this->_isDdlCacheAllowed = true;
296311
$this->isMysql8Engine = null;
297312
$this->_queryHook = null;
313+
$this->avoidReusingParentProcessConnection();
298314
$this->closeConnection();
299315
}
300316

@@ -397,6 +413,23 @@ public function convertDateTime($datetime)
397413
return $this->formatDate($datetime, true);
398414
}
399415

416+
/**
417+
* If the connection is associated to a different process id, then we need to not use it.
418+
*
419+
* @return void
420+
*/
421+
private function avoidReusingParentProcessConnection()
422+
{
423+
if (getmypid() != $this->pid) {
424+
// Note: we hide parent's connection into parentConnections so that the destructor isn't called on it.
425+
// Because if destructor is called, it causes parent's connection to die
426+
// We store in array, if parent is also hiding its parent's connection
427+
$this->parentConnections[] = $this->_connection;
428+
$this->_connection = null;
429+
$this->pid = getmypid();
430+
}
431+
}
432+
400433
/**
401434
* Creates a PDO object and connects to the database.
402435
*
@@ -409,6 +442,7 @@ public function convertDateTime($datetime)
409442
*/
410443
protected function _connect()
411444
{
445+
$this->avoidReusingParentProcessConnection();
412446
if ($this->_connection) {
413447
return;
414448
}

lib/internal/Magento/Framework/Locale/Resolver.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,8 @@ public function revert()
180180
*/
181181
public function _resetState(): void
182182
{
183-
$this->locale = null;
184-
$this->emulatedLocales = [];
183+
while (!empty($this->emulatedLocales)) {
184+
$this->revert();
185+
}
185186
}
186187
}

lib/internal/Magento/Framework/Test/Unit/App/ResourceConnectionTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ public function testGetExistingConnectionByName()
125125
ResourceConnection::class,
126126
[
127127
'deploymentConfig' => $this->deploymentConfigMock,
128-
'connections' => ['default_process_' . getmypid() => 'existing_connection']
128+
'connections' => ['default' => 'existing_connection']
129129
]
130130
);
131131
$this->deploymentConfigMock->expects($this->never())->method('get');

0 commit comments

Comments
 (0)