diff --git a/app/code/Magento/Indexer/Setup/Recurring.php b/app/code/Magento/Indexer/Setup/Recurring.php index d95df217fad26..87c6186b1bd1d 100644 --- a/app/code/Magento/Indexer/Setup/Recurring.php +++ b/app/code/Magento/Indexer/Setup/Recurring.php @@ -6,7 +6,9 @@ namespace Magento\Indexer\Setup; +use Magento\Framework\App\ObjectManager; use Magento\Framework\Encryption\Encryptor; +use Magento\Framework\Mview\TriggerCleaner; use Magento\Framework\Encryption\EncryptorInterface; use Magento\Framework\Indexer\StateInterface; use Magento\Framework\Json\EncoderInterface; @@ -59,6 +61,11 @@ class Recurring implements InstallSchemaInterface */ private $indexerFactory; + /** + * @var TriggerCleaner + */ + private $triggerCleaner; + /** * Init * @@ -68,6 +75,7 @@ class Recurring implements InstallSchemaInterface * @param EncryptorInterface $encryptor * @param EncoderInterface $encoder * @param IndexerInterfaceFactory $indexerFactory + * @param TriggerCleaner|null $triggerCleaner */ public function __construct( CollectionFactory $statesFactory, @@ -75,7 +83,8 @@ public function __construct( ConfigInterface $config, EncryptorInterface $encryptor, EncoderInterface $encoder, - IndexerInterfaceFactory $indexerFactory + IndexerInterfaceFactory $indexerFactory, + ?TriggerCleaner $triggerCleaner = null ) { $this->statesFactory = $statesFactory; $this->stateFactory = $stateFactory; @@ -83,6 +92,7 @@ public function __construct( $this->encryptor = $encryptor; $this->encoder = $encoder; $this->indexerFactory = $indexerFactory; + $this->triggerCleaner = $triggerCleaner ?? ObjectManager::getInstance()->get(TriggerCleaner::class); } /** @@ -132,15 +142,15 @@ public function install(SchemaSetupInterface $setup, ModuleContextInterface $con $indexer = $this->indexerFactory->create()->load($indexerId); if ($indexer->isScheduled()) { - // The purpose of the following two lines is to ensure that any - // database triggers are correctly set up for this indexer. We - // are calling methods on the view directly because we want to - // choose to not drop the changelog tables at this time. This - // also intentionally bypasses the $indexer->invalidate() call - // within $indexer->setScheduled(). - $indexer->getView()->unsubscribe(false); - $indexer->getView()->subscribe(); + // Migrate existing changelog tables (version_id int→bigint, charset→utf8mb4) + // without unsubscribing/subscribing, which would unconditionally rebuild triggers. + $indexer->getView()->getChangelog()->create(); } } + + // Use TriggerCleaner to only recreate triggers whose statements actually changed, + // instead of unconditionally dropping and recreating all triggers for every indexer. + // This avoids acquiring exclusive table locks when no trigger changes are needed. + $this->triggerCleaner->removeTriggers(); } } diff --git a/lib/internal/Magento/Framework/Mview/Test/Unit/TriggerCleanerTest.php b/lib/internal/Magento/Framework/Mview/Test/Unit/TriggerCleanerTest.php index 0d4a8616a9831..985ba2c4e8e58 100644 --- a/lib/internal/Magento/Framework/Mview/Test/Unit/TriggerCleanerTest.php +++ b/lib/internal/Magento/Framework/Mview/Test/Unit/TriggerCleanerTest.php @@ -8,16 +8,16 @@ namespace Magento\Framework\Mview\Test\Unit; +use Magento\Framework\App\ResourceConnection; use Magento\Framework\DB\Adapter\AdapterInterface; -use Magento\Framework\DB\Select; use Magento\Framework\DB\Ddl\Trigger; -use Magento\Framework\App\ResourceConnection; +use Magento\Framework\DB\Select; +use Magento\Framework\Mview\TriggerCleaner; +use Magento\Framework\Mview\View; use Magento\Framework\Mview\View\CollectionFactory; use Magento\Framework\Mview\View\CollectionInterface; use Magento\Framework\Mview\View\Subscription; -use Magento\Framework\Mview\View; use Magento\Framework\Mview\ViewFactory; -use Magento\Framework\Mview\TriggerCleaner; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; @@ -52,7 +52,7 @@ class TriggerCleanerTest extends TestCase * @inheritdoc */ protected function setUp(): void - { + { $this->resource = $this->createMock(ResourceConnection::class); $this->viewCollectionFactory = $this->getMockBuilder(CollectionFactory::class) ->disableOriginalConstructor() @@ -117,6 +117,52 @@ public function testRemoveTriggersNoChanges(): void $this->model->removeTriggers(); } + public function testRemoveTriggersWithStaleStatements(): void + { + $DBTriggers = [ + 'trg_catalog_category_entity_int_after_insert' => [ + 'TRIGGER_NAME' => 'trg_catalog_category_entity_int_after_insert', + 'ACTION_STATEMENT' => 'BEGIN statement; stale_statement; END', + 'EVENT_OBJECT_TABLE' => 'catalog_category_entity_int' + ] + ]; + + $connectionMock = $this->getConnectionMock(); + $connectionMock->expects($this->once()) + ->method('fetchAssoc') + ->willReturn($DBTriggers); + + $this->resource->expects($this->once())->method('getConnection')->willReturn($connectionMock); + + $triggerMock = $this->getMockBuilder(Trigger::class) + ->disableOriginalConstructor() + ->onlyMethods(['getName', 'getStatements']) + ->getMock(); + $triggerMock->expects($this->atLeastOnce()) + ->method('getName') + ->willReturn('trg_catalog_category_entity_int_after_insert'); + $triggerMock->expects($this->once())->method('getStatements')->willReturn(['statement;']); + + $subscriptionMock = $this->createMock(Subscription::class); + $subscriptionMock->expects($this->once())->method('getTriggers')->willReturn([$triggerMock]); + $subscriptionMock->expects($this->once())->method('create')->willReturn($subscriptionMock); + $subscriptionMock->expects($this->once())->method('saveTrigger')->with($triggerMock); + + $viewMock = $this->createMock(View::class); + $viewMock->expects($this->once()) + ->method('getSubscriptions') + ->willReturn(['subscriptionConfig' => []]); + $viewMock->expects($this->once())->method('initSubscriptionInstance')->willReturn($subscriptionMock); + $viewMock->expects($this->never())->method('unsubscribe'); + + $viewCollectionMock = $this->createMock(CollectionInterface::class); + $viewCollectionMock->expects($this->once())->method('getViewsByStateMode')->willReturn([$viewMock]); + + $this->viewCollectionFactory->expects($this->once())->method('create')->willReturn($viewCollectionMock); + + $this->model->removeTriggers(); + } + public function testRemoveTriggersNotLinked(): void { $DBTriggers = [ diff --git a/lib/internal/Magento/Framework/Mview/TriggerCleaner.php b/lib/internal/Magento/Framework/Mview/TriggerCleaner.php index 764f72e5c3911..f0983b488cfe5 100644 --- a/lib/internal/Magento/Framework/Mview/TriggerCleaner.php +++ b/lib/internal/Magento/Framework/Mview/TriggerCleaner.php @@ -8,10 +8,10 @@ namespace Magento\Framework\Mview; use Magento\Framework\App\ResourceConnection; +use Magento\Framework\DB\Ddl\Trigger; use Magento\Framework\Mview\View\CollectionFactory; use Magento\Framework\Mview\View\StateInterface; use Magento\Framework\Mview\View\Subscription; -use Magento\Framework\DB\Ddl\Trigger; /** * Class for removing old triggers that were created by mview @@ -106,22 +106,35 @@ public function removeTriggers(): bool private function processViewTriggers(array $viewTriggers, Subscription $subscription): void { foreach ($viewTriggers as $viewTrigger) { - if (array_key_exists($viewTrigger->getName(), $this->DbTriggers)) { - foreach ($this->getStatementsFromViewTrigger($viewTrigger) as $statement) { - if (!empty($statement) && - !str_contains($this->DbTriggers[$viewTrigger->getName()]['ACTION_STATEMENT'], $statement) - ) { - $subscription->saveTrigger($viewTrigger); - break; - } - } - } else { + if ($this->shouldUpdateTrigger($viewTrigger)) { $subscription->saveTrigger($viewTrigger); } $this->processedTriggers[$viewTrigger->getName()] = true; } } + /** + * Determine whether an existing DB trigger needs to be recreated. + * + * @param Trigger $viewTrigger + * @return bool + */ + private function shouldUpdateTrigger(Trigger $viewTrigger): bool + { + if (!array_key_exists($viewTrigger->getName(), $this->DbTriggers)) { + return true; + } + + $expectedStatement = $this->normalizeTriggerStatement( + implode(PHP_EOL, $this->getStatementsFromViewTrigger($viewTrigger)) + ); + $dbStatement = $this->normalizeTriggerStatement( + $this->DbTriggers[$viewTrigger->getName()]['ACTION_STATEMENT'] + ); + + return $expectedStatement !== $dbStatement; + } + /** * Retrieve list of all triggers from DB * @@ -188,4 +201,23 @@ private function getStatementsFromViewTrigger(Trigger $trigger): array return $statements; } + + /** + * Normalize trigger statements so equivalent SQL can be compared reliably. + * + * @param string $statement + * @return string + */ + private function normalizeTriggerStatement(string $statement): string + { + $statement = trim($statement); + + if (preg_match('/^BEGIN\s+(.*)\s+END;?$/is', $statement, $matches)) { + $statement = $matches[1]; + } + + $statement = preg_replace('/\s*;\s*/', ';', $statement); + + return trim((string)preg_replace('/\s+/', ' ', $statement)); + } }