This addresses the various dead lock issues when using transactions a…

…nd multiple recorders. (matomo-org#12733)

* visitorGeolocator: output actual changes in debug mode (matomo-org#12478)

closes matomo-org#12477

* Revert "visitorGeolocator: output actual changes in debug mode (matomo-org#12478)" (matomo-org#12480)

This reverts commit 19a7654.

* Fix SystemIntegration test (matomo-org#12726)

Found `piwik`

* This addresses the various dead lock issues when using transactions and bulk load.  Issue matomo-org#6398

* Fix archive test to work with multi recorder fix.

* Minor changes and renames

core/Archive/ArchiveInvalidator.php

@@ -60,15 +60,26 @@ public function __construct(Model $model)
 
     public function rememberToInvalidateArchivedReportsLater($idSite, Date $date)
     {
-        $key   = $this->buildRememberArchivedReportId($idSite, $date->toString());
-        $value = Option::get($key);
+        // To support multiple transactions at once, look for any other process to have set (and committed)
+        // this report to be invalidated.
+        $key   = $this->buildRememberArchivedReportIdForSiteAndDate($idSite, $date->toString());
 
         // we do not really have to get the value first. we could simply always try to call set() and it would update or
         // insert the record if needed but we do not want to lock the table (especially since there are still some
         // MyISAM installations)
-
-        if (false === $value) {
-            Option::set($key, '1');
+        $value = Option::getLike($key . '%');
+
+        // In order to support multiple concurrent transactions, add our pid to the end of the key so that it will just insert
+        // rather than waiting on some other process to commit before proceeding.The issue is that with out this, more than
+        // one process is trying to add the exact same value to the table, which causes contention. With the pid suffixed to
+        // the value, each process can successfully enter its own row in the table. The net result will be the same. We could
+        // always just set this, but it would result in a lot of rows in the options table.. more than needed.  With this
+        // change you'll have at most N rows per date/site, where N is the number of parallel requests on this same idsite/date
+        // that happen to run in overlapping transactions.
+        $mykey = $this->buildRememberArchivedReportIdProcessSafe($idSite, $date->toString());
+        // getLike() returns an empty array rather than 'false'
+        if (empty($value)) {
+            Option::set($mykey, '1');
         }
     }
 
@@ -94,17 +105,25 @@ public function getRememberedArchivedReportsThatShouldBeInvalidated()
         return $sitesPerDay;
     }
 
-    private function buildRememberArchivedReportId($idSite, $date)
+    private function buildRememberArchivedReportIdForSite($idSite)
+    {
+        return $this->rememberArchivedReportIdStart . (int) $idSite;
+    }
+
+    private function buildRememberArchivedReportIdForSiteAndDate($idSite, $date)
     {
         $id  = $this->buildRememberArchivedReportIdForSite($idSite);
         $id .= '_' . trim($date);
 
         return $id;
     }
 
-    private function buildRememberArchivedReportIdForSite($idSite)
+    // This version is multi process safe on the insert of a new date to invalidate.
+    private function buildRememberArchivedReportIdProcessSafe($idSite, $date)
     {
-        return $this->rememberArchivedReportIdStart . (int) $idSite;
+        $id  = $this->buildRememberArchivedReportIdForSiteAndDate($idSite, $date);
+        $id .= '_' . getmypid();
+        return $id;
     }
 
     public function forgetRememberedArchivedReportsToInvalidateForSite($idSite)
@@ -118,9 +137,11 @@ public function forgetRememberedArchivedReportsToInvalidateForSite($idSite)
      */
     public function forgetRememberedArchivedReportsToInvalidate($idSite, Date $date)
     {
-        $id = $this->buildRememberArchivedReportId($idSite, $date->toString());
+        $id = $this->buildRememberArchivedReportIdForSiteAndDate($idSite, $date->toString());
 
-        Option::delete($id);
+        // The process pid is added to the end of the entry in order to support multiple concurrent transactions.
+        //  So this must be a deleteLike call to get all the entries, where there used to only be one.
+        Option::deleteLike($id . '%');
     }
 
     /**

core/CronArchive.php

@@ -1151,7 +1151,8 @@ public function invalidateArchivedReportsForSitesThatNeedToBeArchivedAgain()
         $sitesPerDays = $this->invalidator->getRememberedArchivedReportsThatShouldBeInvalidated();
 
         foreach ($sitesPerDays as $date => $siteIds) {
-            $listSiteIds = implode(',', $siteIds);
+            //Concurrent transaction logic will end up with duplicates set.  Adding array_unique to the siteIds.
+            $listSiteIds = implode(',', array_unique($siteIds ));
 
             try {
                 $this->logger->info('- Will invalidate archived reports for ' . $date . ' for following websites ids: ' . $listSiteIds);

tests/PHPUnit/Integration/DataAccess/ArchiveInvalidatorTest.php

@@ -66,7 +66,8 @@ public function setUp()
 
     public function test_rememberToInvalidateArchivedReportsLater_shouldCreateAnEntryInCaseThereIsNoneYet()
     {
-        $key = 'report_to_invalidate_2_2014-04-05';
+        //Updated for change to allow for multiple transactions to invalidate the same report without deadlock.
+        $key = 'report_to_invalidate_2_2014-04-05' . '_' . getmypid();
         $this->assertFalse(Option::get($key));
 
         $this->rememberReport(2, '2014-04-05');