mirror of
https://github.com/karatektus/pl_mail.git
synced 2026-10-25 01:54:10 +00:00
fix(deploy): the cache table exists before any worker reads it
A fresh install logged a warning about once a second from the worker, until an administrator happened to press "Restart workers": Failed to fetch key "workers.restart_requested_timestamp": ... relation "cache_items" does not exist The restart signal and the notice throttle live in cache pools on Symfony's DBAL adapter, which creates cache_items on its first write. Every Messenger consumer reads the signal on every loop, long before anything writes it, and each failed read is logged. The migrations leave the table out on purpose, because the adapter owns its DDL. app:db:migrate now has each DBAL adapter create its table when it is missing, after the migrations and inside the advisory lock, so php and the worker cannot race each other to it. The adapter still owns the DDL: it names its table through configureSchema() and creates it with createTable(). Best effort, like the statement statistics: a failure is a warning, not a container that will not start. Existing installs get the table on their next boot. The adapters are collected by class in a compiler pass rather than injected by pool id. In dev and test the profiler wraps every pool in a TraceableAdapter, which has no createTable(). It is the lookup DoctrineBundle uses for its schema listener, and it covers any DBAL pool added later. Checked on two fresh compose.yaml + compose.demo.yaml stacks: 95 warnings in 75 seconds without this, none with it, and Restart workers still reaches every consumer through the table created at boot. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
996ae3539a
commit
6edc6c324c
8 files changed
+217
-2
No files matched your search
@@ -110,6 +110,12 @@ services:
|
||||
App\Infrastructure\Encryption\Encryptor:
|
||||
public: true
|
||||
|
||||
# Filled in by DatabaseCachePoolsPass. Autowiring runs before the profiler
|
||||
# wraps the cache pools, too early to tell an adapter from its wrapper.
|
||||
App\Infrastructure\Setup\DatabaseCacheTables:
|
||||
arguments:
|
||||
$pools: !abstract 'every DoctrineDbalAdapter in the container'
|
||||
|
||||
# The policy is enforced in production and reported under debug, because the
|
||||
# profiler toolbar and VarDumper inject inline scripts nothing can nonce.
|
||||
App\Security\Csp\ContentSecurityPolicyListener:
|
||||
|
||||
@@ -33,6 +33,10 @@ use Doctrine\Migrations\AbstractMigration;
|
||||
*
|
||||
* cache_items is deliberately absent despite showing up in the diff: the DBAL
|
||||
* cache adapter creates that table itself on first write and owns its DDL.
|
||||
* First write is not soon enough, though — every consumer reads the
|
||||
* restart-workers signal long before anything writes it, and each failed read
|
||||
* is logged — so app:db:migrate has the adapter create it at boot. See
|
||||
* App\Infrastructure\Setup\DatabaseCacheTables.
|
||||
*/
|
||||
final class Version20260731222453 extends AbstractMigration
|
||||
{
|
||||
|
||||
@@ -4,6 +4,7 @@ declare(strict_types=1);
|
||||
|
||||
namespace App\Command\Setup;
|
||||
|
||||
use App\Infrastructure\Setup\DatabaseCacheTables;
|
||||
use App\Repository\Monitoring\PostgresStatusRepository;
|
||||
use Doctrine\DBAL\Connection;
|
||||
use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
|
||||
@@ -88,6 +89,7 @@ final class MigrateCommand extends Command
|
||||
public function __construct(
|
||||
private readonly Connection $connection,
|
||||
private readonly PostgresStatusRepository $statistics,
|
||||
private readonly DatabaseCacheTables $cacheTables,
|
||||
) {
|
||||
parent::__construct();
|
||||
}
|
||||
@@ -114,7 +116,7 @@ final class MigrateCommand extends Command
|
||||
// migrate unprotected, which is exactly what happened before this
|
||||
// command existed.
|
||||
if (!$this->connection->getDatabasePlatform() instanceof PostgreSQLPlatform) {
|
||||
return $this->runMigrations($output);
|
||||
return $this->migrate($io, $output);
|
||||
}
|
||||
|
||||
$timeout = max(0, (int) $input->getOption('lock-timeout'));
|
||||
@@ -132,7 +134,7 @@ final class MigrateCommand extends Command
|
||||
}
|
||||
|
||||
try {
|
||||
$result = $this->runMigrations($output);
|
||||
$result = $this->migrate($io, $output);
|
||||
|
||||
// Under the same lock, so only the container that migrated tries it
|
||||
// rather than all four racing a CREATE EXTENSION. Best effort by
|
||||
@@ -148,6 +150,38 @@ final class MigrateCommand extends Command
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The migrations, then the cache tables they deliberately leave out — see
|
||||
* DatabaseCacheTables for why those cannot wait for their first write.
|
||||
*
|
||||
* Here because this is the step every container finishes before it starts
|
||||
* anything that reads them, and, on Postgres, inside the lock, so php and
|
||||
* the worker booting together cannot both find the table missing and race
|
||||
* to create it.
|
||||
*
|
||||
* Best effort, like the statement statistics: the adapter still creates its
|
||||
* table on its first write, so a failure here costs log noise until then,
|
||||
* which is no reason to keep a container from starting.
|
||||
*/
|
||||
private function migrate(SymfonyStyle $io, OutputInterface $output): int
|
||||
{
|
||||
$result = $this->runMigrations($output);
|
||||
|
||||
if (Command::SUCCESS !== $result) {
|
||||
return $result;
|
||||
}
|
||||
|
||||
try {
|
||||
foreach ($this->cacheTables->createMissing() as $table) {
|
||||
$io->text(sprintf('Created the cache table %s.', $table));
|
||||
}
|
||||
} catch (\Throwable $error) {
|
||||
$io->warning(sprintf('Could not create the cache tables: %s', $error->getMessage()));
|
||||
}
|
||||
|
||||
return $result;
|
||||
}
|
||||
|
||||
/**
|
||||
* The real command, with the flags the entrypoint used to pass it.
|
||||
*
|
||||
|
||||
@@ -0,0 +1,48 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Infrastructure\Setup;
|
||||
|
||||
use Symfony\Component\Cache\Adapter\DoctrineDbalAdapter;
|
||||
use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
|
||||
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
|
||||
use Symfony\Component\DependencyInjection\ContainerBuilder;
|
||||
use Symfony\Component\DependencyInjection\Reference;
|
||||
|
||||
/**
|
||||
* Hands DatabaseCacheTables every DoctrineDbalAdapter the container defines.
|
||||
*
|
||||
* Found by class, not injected by pool id. In dev and test the profiler wraps
|
||||
* each pool in a TraceableAdapter under the pool's own id and moves the adapter
|
||||
* itself to `<id>.recorder_inner`, so `cache.messenger.restart_workers_signal`
|
||||
* would hand over a wrapper that has no createTable(). By class, the adapters
|
||||
* are found wherever they ended up — which is how DoctrineBundle finds them
|
||||
* for the schema listener that puts cache_items into the migrations diff — and
|
||||
* a pool added to cache.yaml later is covered without anyone remembering this.
|
||||
*
|
||||
* Only correct once the profiler has done its wrapping; Kernel::build() is
|
||||
* what orders it after that.
|
||||
*/
|
||||
final class DatabaseCachePoolsPass implements CompilerPassInterface
|
||||
{
|
||||
public function process(ContainerBuilder $container): void
|
||||
{
|
||||
$pools = [];
|
||||
|
||||
foreach ($container->getDefinitions() as $id => $definition) {
|
||||
if ($definition->isAbstract() || $definition->isSynthetic()) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if (DoctrineDbalAdapter::class === $definition->getClass()) {
|
||||
$pools[] = new Reference($id);
|
||||
}
|
||||
}
|
||||
|
||||
// Argument 0 is $pools, left abstract in services.yaml so that a
|
||||
// container this pass never ran on fails loudly instead of quietly
|
||||
// creating nothing.
|
||||
$container->getDefinition(DatabaseCacheTables::class)->replaceArgument(0, new IteratorArgument($pools));
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,70 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Infrastructure\Setup;
|
||||
|
||||
use App\Repository\Monitoring\PostgresStatusRepository;
|
||||
use Doctrine\DBAL\Connection;
|
||||
use Doctrine\DBAL\Schema\Schema;
|
||||
use Symfony\Component\Cache\Adapter\DoctrineDbalAdapter;
|
||||
|
||||
/**
|
||||
* Creates the tables the database-backed cache pools keep their items in,
|
||||
* before anything reads from them.
|
||||
*
|
||||
* Symfony's DoctrineDbalAdapter creates its table on the first WRITE, and a
|
||||
* read before that fails and is logged as a warning. On these pools the reads
|
||||
* come first, and constantly: every Messenger consumer checks the
|
||||
* restart-workers signal on every loop (StopWorkerOnRestartSignalListener),
|
||||
* and nothing writes that signal until an administrator presses "Restart
|
||||
* workers". A fresh install logged a warning a second from each consumer until
|
||||
* somebody did.
|
||||
*
|
||||
* The migrations leave the table out on purpose, and this keeps it that way:
|
||||
* the adapter still owns the DDL and is only asked to run it early. A
|
||||
* hand-written CREATE TABLE would be a second copy of Symfony's that nothing
|
||||
* keeps in step.
|
||||
*/
|
||||
final readonly class DatabaseCacheTables
|
||||
{
|
||||
/**
|
||||
* @param iterable<DoctrineDbalAdapter> $pools every one the container defines; see DatabaseCachePoolsPass
|
||||
*/
|
||||
public function __construct(
|
||||
private iterable $pools,
|
||||
private Connection $connection,
|
||||
private PostgresStatusRepository $database,
|
||||
) {
|
||||
}
|
||||
|
||||
/**
|
||||
* @return list<string> the tables this call created; empty when they all existed already
|
||||
*/
|
||||
public function createMissing(): array
|
||||
{
|
||||
$created = [];
|
||||
|
||||
foreach ($this->pools as $pool) {
|
||||
// The adapter is asked for its table rather than the name being
|
||||
// assumed here, so its options stay the one place that decides it.
|
||||
// A pool on another connection answers with nothing: its table is
|
||||
// not in this database, and looking for it here would answer the
|
||||
// wrong question.
|
||||
$wanted = $pool->configureSchema(new Schema(), $this->connection, static fn (): bool => false);
|
||||
|
||||
foreach ($wanted->getTables() as $table) {
|
||||
$name = $table->getObjectName()->toString();
|
||||
|
||||
if ($this->database->hasTable($name)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$pool->createTable();
|
||||
$created[] = $name;
|
||||
}
|
||||
}
|
||||
|
||||
return $created;
|
||||
}
|
||||
}
|
||||
@@ -4,13 +4,27 @@ namespace App;
|
||||
|
||||
use App\Infrastructure\Doctrine\Type\EncryptedStringType;
|
||||
use App\Infrastructure\Encryption\Encryptor;
|
||||
use App\Infrastructure\Setup\DatabaseCachePoolsPass;
|
||||
use Symfony\Bundle\FrameworkBundle\Kernel\MicroKernelTrait;
|
||||
use Symfony\Component\DependencyInjection\Compiler\PassConfig;
|
||||
use Symfony\Component\DependencyInjection\ContainerBuilder;
|
||||
use Symfony\Component\HttpKernel\Kernel as BaseKernel;
|
||||
|
||||
class Kernel extends BaseKernel
|
||||
{
|
||||
use MicroKernelTrait;
|
||||
|
||||
/**
|
||||
* Late on purpose: DatabaseCachePoolsPass has to see the cache pools after
|
||||
* the profiler has wrapped them (before-removing, priority 0), or it would
|
||||
* take a wrapper for an adapter. -10 is the slot DoctrineBundle's own pass
|
||||
* over the same adapters takes, for the same reason.
|
||||
*/
|
||||
protected function build(ContainerBuilder $container): void
|
||||
{
|
||||
$container->addCompilerPass(new DatabaseCachePoolsPass(), PassConfig::TYPE_BEFORE_REMOVING, -10);
|
||||
}
|
||||
|
||||
/**
|
||||
* Doctrine builds its types through a static registry with no access to
|
||||
* the container, so EncryptedStringType is handed its Encryptor here —
|
||||
|
||||
@@ -120,6 +120,20 @@ final readonly class PostgresStatusRepository
|
||||
return $this->hasStatStatements();
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a table exists, for the tables that are not the migrations' to
|
||||
* create. False when the catalogue cannot be read: the caller then tries to
|
||||
* create the table, and that attempt reports the real problem.
|
||||
*/
|
||||
public function hasTable(string $name): bool
|
||||
{
|
||||
try {
|
||||
return $this->connection->createSchemaManager()->tableExists($name);
|
||||
} catch (\Throwable) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Statements whose *average* is slow, worst first — "which queries are
|
||||
* slow". Floored, because a statement that averages a fraction of a
|
||||
|
||||
@@ -29,6 +29,9 @@ use Symfony\Component\Console\Tester\CommandTester;
|
||||
* container that cannot get it refuses to migrate rather than pressing on, and
|
||||
* the lock is handed back afterwards. If any of those three regress, boot goes
|
||||
* back to being a race.
|
||||
*
|
||||
* Also the cache table the migrations leave out, which the command creates
|
||||
* after them; see DatabaseCacheTables.
|
||||
*/
|
||||
final class MigrateCommandTest extends KernelTestCase
|
||||
{
|
||||
@@ -122,6 +125,28 @@ final class MigrateCommandTest extends KernelTestCase
|
||||
$this->otherContainer->executeQuery('SELECT pg_advisory_unlock(?)', [$this->lockKey()]);
|
||||
}
|
||||
|
||||
public function testTheCacheTableExistsBeforeAnyConsumerReadsTheRestartSignal(): void
|
||||
{
|
||||
$schema = $this->connection->createSchemaManager();
|
||||
|
||||
// A fresh install: the migrations leave cache_items to the adapter, and
|
||||
// nothing has written to a pool yet.
|
||||
if ($schema->tableExists('cache_items')) {
|
||||
$schema->dropTable('cache_items');
|
||||
}
|
||||
|
||||
self::assertSame(Command::SUCCESS, $this->command->execute([]));
|
||||
self::assertTrue(
|
||||
$schema->tableExists('cache_items'),
|
||||
'Without it every consumer logs a failed read of the restart signal once a second.',
|
||||
);
|
||||
self::assertStringContainsString('cache_items', $this->command->getDisplay());
|
||||
|
||||
// Every later boot finds it there and leaves it alone.
|
||||
self::assertSame(Command::SUCCESS, $this->command->execute([]));
|
||||
self::assertStringNotContainsString('cache_items', $this->command->getDisplay());
|
||||
}
|
||||
|
||||
/**
|
||||
* A second connection to the same database, which is what makes these
|
||||
* assertions mean anything: the lock is scoped to a session, and the
|
||||
|
||||
Reference in new issue
Block a user