#33386 Fix triggers management issue during setup:upgrade

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,21 +75,24 @@ class Recurring implements InstallSchemaInterface
      * @param EncryptorInterface $encryptor
      * @param EncoderInterface $encoder
      * @param IndexerInterfaceFactory $indexerFactory
+     * @param TriggerCleaner|null $triggerCleaner
      */
     public function __construct(
         CollectionFactory $statesFactory,
         StateFactory $stateFactory,
         ConfigInterface $config,
         EncryptorInterface $encryptor,
         EncoderInterface $encoder,
-        IndexerInterfaceFactory $indexerFactory
+        IndexerInterfaceFactory $indexerFactory,
+        ?TriggerCleaner $triggerCleaner = null
     ) {
         $this->statesFactory = $statesFactory;
         $this->stateFactory = $stateFactory;
         $this->config = $config;
         $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();
     }
 }

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 = [

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));
+    }
 }