@tsteur opened this Pull Request on July 28th 2021 Member
@tsteur commented on July 29th 2021 Member

Potential patch we could apply to only use this logic when cohorts is not enabled but then the tests etc everything becomes less testable ...

diff --git a/core/ArchiveProcessor.php b/core/ArchiveProcessor.php
index 9365152532..510c418f08 100644
--- a/core/ArchiveProcessor.php
+++ b/core/ArchiveProcessor.php
@@ -528,12 +528,20 @@ class ArchiveProcessor
     {
         $logAggregator = $this->getLogAggregator();
         $sitesBackup = $logAggregator->getSites();
-        $previous = $logAggregator->disallowUsageSegmentCache();
+
+        $hasCohorts = \Piwik\Plugin\Manager::getInstance()->isPluginActivated('Cohorts');
+        if (!$hasCohorts) {
+            // when cohorts is enabled, then it's better to create the segment table as cohorts will also issue a
+            // unique query on log data for weeks etc. This way the segment query is only executed once and in most
+            // cases things are faster that way. Vs when cohorts is disabled, then we don't want to first create a
+            // temporary segment table for the segment query and then execute the count.
+            $previous = $logAggregator->disallowUsageSegmentCache();
+        }
         $logAggregator->setSites($sites);
         try {
             $query = $logAggregator->queryVisitsByDimension(array(), false, array(), $metrics);
         } finally {
-            if ($previous) {
+            if (!empty($previous)) {
                 $logAggregator->allowUsageSegmentCache();
             }
             $logAggregator->setSites($sitesBackup);
This Pull Request was closed on July 29th 2021
Powered by GitHub Issue Mirror