Move Archive.php archive invalidation to Loader… (matomo-org#15616)

* Move Archive.php archive invalidation to Loader so we only invalidate when about to launch archiving. * Attempt to handle more cases when invalidating before launching archiving. * fix possible sql error * fix possible error * fixing some tests * remove test code * Only invalidate specific archive being requested. * Do not invalidate on today in tracker and avoid existing valid archive check in CronArchive. * more test fixes * Attempt to fix more tests. * Fixing last tests. * another test fix * Invalidate in scheduled task if browser triggered archiving is enabled. * deal with TODO * Get ArchiveSelectorTest to pass. * applying review feedback including new tests * apply review feedback & fix tests * fix couple more tests Co-authored-by: Thomas Steur <[email protected]>
1024pix · Sep 22, 2020 · 1cc7f1d · 1cc7f1d
1 parent 7e7ae31
commit 1cc7f1d
Show file tree

Hide file tree

Showing 25 changed files with 879 additions and 650 deletions.
diff --git a/core/Archive.php b/core/Archive.php
@@ -12,7 +12,6 @@
 use Piwik\Archive\ArchiveQueryFactory;
 use Piwik\Archive\Parameters;
 use Piwik\ArchiveProcessor\Rules;
-use Piwik\Archive\ArchiveInvalidator;
 use Piwik\Container\StaticContainer;
 use Piwik\DataAccess\ArchiveSelector;
 
@@ -167,11 +166,6 @@ class Archive implements ArchiveQuery
      */
     private static $cache;
 
-    /**
-     * @var ArchiveInvalidator
-     */
-    private $invalidator;
-
     /**
      * @param Parameters $params
      * @param bool $forceIndexedBySite Whether to force index the result of a query by site ID.
@@ -183,8 +177,6 @@ public function __construct(Parameters $params, $forceIndexedBySite = false,
         $this->params = $params;
         $this->forceIndexedBySite = $forceIndexedBySite;
         $this->forceIndexedByDate = $forceIndexedByDate;
-
-        $this->invalidator = StaticContainer::get('Piwik\Archive\ArchiveInvalidator');
     }
 
     /**
@@ -453,67 +445,6 @@ public static function createDataTableFromArchive($recordName, $idSite, $period,
         return $dataTable;
     }
 
-    private function getSiteIdsThatAreRequestedInThisArchiveButWereNotInvalidatedYet()
-    {
-        if (is_null(self::$cache)) {
-            self::$cache = Cache::getTransientCache();
-        }
-
-        $id = 'Archive.SiteIdsOfRememberedReportsInvalidated';
-
-        if (!self::$cache->contains($id)) {
-            self::$cache->save($id, array());
-        }
-
-        $siteIdsAlreadyHandled = self::$cache->fetch($id);
-        $siteIdsRequested      = $this->params->getIdSites();
-
-        foreach ($siteIdsRequested as $index => $siteIdRequested) {
-            $siteIdRequested = (int) $siteIdRequested;
-
-            if (in_array($siteIdRequested, $siteIdsAlreadyHandled)) {
-                unset($siteIdsRequested[$index]); // was already handled previously, do not do it again
-            } else {
-                $siteIdsAlreadyHandled[] = $siteIdRequested; // we will handle this id this time
-            }
-        }
-
-        self::$cache->save($id, $siteIdsAlreadyHandled);
-
-        return $siteIdsRequested;
-    }
-
-    private function invalidatedReportsIfNeeded()
-    {
-        $siteIdsRequested = $this->getSiteIdsThatAreRequestedInThisArchiveButWereNotInvalidatedYet();
-
-        if (empty($siteIdsRequested)) {
-            return; // all requested site ids were already handled
-        }
-
-        $sitesPerDays = $this->invalidator->getRememberedArchivedReportsThatShouldBeInvalidated();
-
-        foreach ($sitesPerDays as $date => $siteIds) {
-            if (empty($siteIds)) {
-                continue;
-            }
-
-            $siteIdsToActuallyInvalidate = array_intersect($siteIds, $siteIdsRequested);
-
-            if (empty($siteIdsToActuallyInvalidate)) {
-                continue; // all site ids that should be handled are already handled
-            }
-
-            try {
-                $this->invalidator->markArchivesAsInvalidated($siteIdsToActuallyInvalidate, array(Date::factory($date)), false);
-            } catch (\Exception $e) {
-                Site::clearCache();
-                throw $e;
-            }
-        }
-
-        Site::clearCache();
-    }
 
     /**
      * Queries archive tables for data and returns the result.
@@ -638,8 +569,6 @@ private function getArchiveIds($archiveNames)
      */
     private function cacheArchiveIdsAfterLaunching($archiveGroups, $plugins)
     {
-        $this->invalidatedReportsIfNeeded();
-
         $today = Date::today();
 
         foreach ($this->params->getPeriods() as $period) {
@@ -856,8 +785,11 @@ public static function getPluginForReport($report)
      */
     private function prepareArchive(array $archiveGroups, Site $site, Period $period)
     {
+        // if cron archiving is running, we will invalidate in CronArchive, not here
+        $invalidateBeforeArchiving = !SettingsServer::isArchivePhpTriggered();
+
         $parameters = new ArchiveProcessor\Parameters($site, $period, $this->params->getSegment());
-        $archiveLoader = new ArchiveProcessor\Loader($parameters);
+        $archiveLoader = new ArchiveProcessor\Loader($parameters, $invalidateBeforeArchiving);
 
         $periodString = $period->getRangeString();
 

diff --git a/core/Archive/ArchiveInvalidator.php b/core/Archive/ArchiveInvalidator.php
@@ -147,6 +147,10 @@ public function getRememberedArchivedReportsThatShouldBeInvalidated()
             $siteId = (int) $report[0];
             $date   = $report[1];
 
+            if (empty($siteId)) {
+                continue;
+            }
+
             if (empty($sitesPerDay[$date])) {
                 $sitesPerDay[$date] = array();
             }

diff --git a/core/ArchiveProcessor/Loader.php b/core/ArchiveProcessor/Loader.php
@@ -8,13 +8,18 @@
  */
 namespace Piwik\ArchiveProcessor;
 
+use Piwik\Archive\ArchiveInvalidator;
 use Piwik\Cache;
 use Piwik\Config;
 use Piwik\Container\StaticContainer;
 use Piwik\Context;
 use Piwik\DataAccess\ArchiveSelector;
+use Piwik\DataAccess\ArchiveTableCreator;
 use Piwik\Date;
+use Piwik\Db;
 use Piwik\Piwik;
+use Piwik\Site;
+use Psr\Log\LoggerInterface;
 
 /**
  * This class uses PluginsArchiver class to trigger data aggregation and create archives.
@@ -33,9 +38,28 @@ class Loader
      */
     protected $params;
 
-    public function __construct(Parameters $params)
+    /**
+     * @var ArchiveInvalidator
+     */
+    private $invalidator;
+
+    /**
+     * @var \Piwik\Cache\Cache
+     */
+    private $cache;
+
+    /**
+     * @var LoggerInterface
+     */
+    private $logger;
+
+    public function __construct(Parameters $params, $invalidateBeforeArchiving = false)
     {
         $this->params = $params;
+        $this->invalidateBeforeArchiving = $invalidateBeforeArchiving;
+        $this->invalidator = StaticContainer::get(ArchiveInvalidator::class);
+        $this->cache = Cache::getTransientCache();
+        $this->logger = StaticContainer::get(LoggerInterface::class);
     }
 
     /**
@@ -65,11 +89,19 @@ private function prepareArchiveImpl($pluginName)
     {
         $this->params->setRequestedPlugin($pluginName);
 
-        list($idArchive, $visits, $visitsConverted) = $this->loadExistingArchiveIdFromDb();
-        if (!empty($idArchive)) {
+        list($idArchive, $visits, $visitsConverted, $isAnyArchiveExists) = $this->loadExistingArchiveIdFromDb();
+        if (!empty($idArchive)) { // we have a usable idarchive (it's not invalidated and it's new enough)
             return $idArchive;
         }
 
+        // if there is an archive, but we can't use it for some reason, invalidate existing archives before
+        // we start archiving. if the archive is made invalid, we will correctly re-archive below.
+        if ($this->invalidateBeforeArchiving
+            && $isAnyArchiveExists
+        ) {
+            $this->invalidatedReportsIfNeeded();
+        }
+
         /** @var ArchivingStatus $archivingStatus */
         $archivingStatus = StaticContainer::get(ArchivingStatus::class);
         $archivingStatus->archiveStarted($this->params);
@@ -170,20 +202,17 @@ protected function isArchivingForcedToTrigger()
      */
     public function loadExistingArchiveIdFromDb()
     {
-        $noArchiveFound = array(false, false, false);
-
         if ($this->isArchivingForcedToTrigger()) {
-            return $noArchiveFound;
-        }
+            $this->logger->debug("Archiving forced to trigger for {$this->params}.");
 
-        $minDatetimeArchiveProcessedUTC = $this->getMinTimeArchiveProcessed();
-        $idAndVisits = ArchiveSelector::getArchiveIdAndVisits($this->params, $minDatetimeArchiveProcessedUTC);
-
-        if (!$idAndVisits) {
-            return $noArchiveFound;
+            // return no usable archive found, and no existing archive. this will skip invalidation, which should
+            // be fine since we just force archiving.
+            return [false, false, false, false];
         }
 
-        return $idAndVisits;
+        $minDatetimeArchiveProcessedUTC = $this->getMinTimeArchiveProcessed();
+        $result = ArchiveSelector::getArchiveIdAndVisits($this->params, $minDatetimeArchiveProcessedUTC);
+        return $result;
     }
 
     /**
@@ -242,4 +271,46 @@ private function getIdSitesToArchiveWhenNoVisits()
 
         return $cache->fetch($cacheKey);
     }
+
+    // public for tests
+    public function getReportsToInvalidate()
+    {
+        $sitesPerDays = $this->invalidator->getRememberedArchivedReportsThatShouldBeInvalidated();
+
+        foreach ($sitesPerDays as $dateStr => $siteIds) {
+            if (empty($siteIds)
+                || !in_array($this->params->getSite()->getId(), $siteIds)
+            ) {
+                unset($sitesPerDays[$dateStr]);
+            }
+
+            $date = Date::factory($dateStr);
+            if ($date->isEarlier($this->params->getPeriod()->getDateStart())
+                || $date->isLater($this->params->getPeriod()->getDateEnd())
+            ) { // date in list is not the current date, so ignore it
+                unset($sitesPerDays[$dateStr]);
+            }
+        }
+
+        return $sitesPerDays;
+    }
+
+    private function invalidatedReportsIfNeeded()
+    {
+        $sitesPerDays = $this->getReportsToInvalidate();
+        if (empty($sitesPerDays)) {
+            return;
+        }
+
+        foreach ($sitesPerDays as $date => $siteIds) {
+            try {
+                $this->invalidator->markArchivesAsInvalidated([$this->params->getSite()->getId()], array(Date::factory($date)), false, $this->params->getSegment());
+            } catch (\Exception $e) {
+                Site::clearCache();
+                throw $e;
+            }
+        }
+
+        Site::clearCache();
+    }
 }
diff --git a/core/ArchiveProcessor/Parameters.php b/core/ArchiveProcessor/Parameters.php
@@ -258,4 +258,9 @@ public function setIsRootArchiveRequest($isRootArchiveRequest)
     {
         $this->isRootArchiveRequest = $isRootArchiveRequest;
     }
+
+    public function __toString()
+    {
+        return "[idSite = {$this->getSite()->getId()}, period = {$this->getPeriod()->getLabel()} {$this->getPeriod()->getRangeString()}, segment = {$this->getSegment()->getString()}]";
+    }
 }