Do not process archives already in progress and conditionally keep certain duplicate invalidations. Also fix issue w/ done flag detection, should not process done flag types together otherwise segment can be done before day.archive-invalidations-remove

3 files changed, 39 insertions, 35 deletions

diff --git a/core/CronArchive/QueueConsumer.php b/core/CronArchive/QueueConsumer.php
index 4af3afbb7c..16193fa322 100644
--- a/core/CronArchive/QueueConsumer.php
+++ b/core/CronArchive/QueueConsumer.php
@@ -186,15 +186,6 @@ class QueueConsumer
                 continue;
             }
 
-            if ($this->hasDifferentDoneFlagType($archivesToProcess, $invalidatedArchive['name'])) {
-                $this->logger->debug("Found archive with different done flag type (segment vs. no segment) in concurrent batch, skipping until next batch: $invalidationDesc");
-
-                $idinvalidation = $invalidatedArchive['idinvalidation'];
-                $invalidationsToExcludeInBatch[$idinvalidation] = true;
-
-                continue;
-            }
-
             if ($invalidatedArchive['segment'] === null) {
                 $this->logger->debug("Found archive for segment that is not auto archived, ignoring: $invalidationDesc");
                 $this->addInvalidationToExclude($invalidatedArchive);
@@ -229,6 +220,18 @@ class QueueConsumer
                 continue;
             }
 
+            $alreadyInProgressId = $this->model->isArchiveAlreadyInProgress($invalidatedArchive);
+            if ($alreadyInProgressId) {
+                $this->addInvalidationToExclude($invalidatedArchive);
+                if ($alreadyInProgressId < $invalidatedArchive['idinvalidation']) {
+                    $this->logger->debug("Skipping invalidated archive {$invalidatedArchive['idinvalidation']}, invalidation already in progress. Since in progress is older, not removing invalidation.");
+               } else if ($alreadyInProgressId > $invalidatedArchive['idinvalidation']) {
+                    $this->logger->debug("Skipping invalidated archive {$invalidatedArchive['idinvalidation']}, invalidation already in progress. Since in progress is newer, will remove invalidation.");
+                    $this->model->deleteInvalidations([$invalidatedArchive['idinvalidation']]);
+                }
+                continue;
+            }
+
             if ($this->canSkipArchiveBecauseNoPoint($invalidatedArchive)) {
                 $this->logger->debug("Found invalidated archive we can skip (no visits): $invalidationDesc");
                 $this->addInvalidationToExclude($invalidatedArchive);
@@ -464,18 +467,6 @@ class QueueConsumer
         return $this->segmentArchiving->isAutoArchivingEnabledFor($storedSegment);
     }
 
-    private function hasDifferentDoneFlagType(array $archivesToProcess, $name)
-    {
-        if (empty($archivesToProcess)) {
-            return false;
-        }
-
-        $existingDoneFlagType = $this->getDoneFlagType($archivesToProcess[0]['name']);
-        $newArchiveDoneFlagType = $this->getDoneFlagType($name);
-
-        return $existingDoneFlagType != $newArchiveDoneFlagType;
-    }
-
     private function getPluginNameForArchiveIfAny($archive)
     {
         $name = $archive['name'];
diff --git a/core/DataAccess/Model.php b/core/DataAccess/Model.php
index b623d7fc24..118772c6cf 100644
--- a/core/DataAccess/Model.php
+++ b/core/DataAccess/Model.php
@@ -715,6 +715,30 @@ class Model
         Db::query($sql, [$idSite, 'done.' . $plugin, $report]);
     }
 
+    public function isArchiveAlreadyInProgress($invalidatedArchive)
+    {
+        $table = Common::prefixTable('archive_invalidations');
+
+        $bind = [
+            $invalidatedArchive['idsite'],
+            $invalidatedArchive['date1'],
+            $invalidatedArchive['date2'],
+            $invalidatedArchive['period'],
+            $invalidatedArchive['name'],
+        ];
+
+        $reportClause = "(report = '' OR report IS NULL)";
+        if (!empty($invalidatedArchive['report'])) {
+            $reportClause = "report = ?";
+            $bind[] = $invalidatedArchive['report'];
+        }
+
+        $sql = "SELECT MAX(idinvalidation) FROM `$table` WHERE idsite = ? AND date1 = ? AND date2 = ? AND `period` = ? AND `name` = ? AND status = 1 AND $reportClause";
+
+        $inProgressInvalidation = Db::fetchOne($sql, $bind);
+        return $inProgressInvalidation;
+    }
+
     /**
      * Returns true if there is an archive that exists that can be used when aggregating an archive for $period.
      *
diff --git a/tests/PHPUnit/Integration/CronArchive/QueueConsumerTest.php b/tests/PHPUnit/Integration/CronArchive/QueueConsumerTest.php
index 1d14e457b9..50d824bfdb 100644
--- a/tests/PHPUnit/Integration/CronArchive/QueueConsumerTest.php
+++ b/tests/PHPUnit/Integration/CronArchive/QueueConsumerTest.php
@@ -187,16 +187,16 @@ class QueueConsumerTest extends IntegrationTestCase
                 ),
             ),
             array (
-                array ( // duplicate, processed but if in progress or recent should be skipped
+                array (
                     'idarchive' => '1',
                     'idsite' => '1',
                     'date1' => '2018-03-06',
                     'date2' => '2018-03-06',
                     'period' => '1',
-                    'name' => 'done',
+                    'name' => 'done5f4f9bafeda3443c3c2d4b2ef4dffadc',
                     'report' => NULL,
                     'plugin' => NULL,
-                    'segment' => '',
+                    'segment' => 'browserCode==IE',
                 ),
                 array (
                     'idarchive' => '1',
@@ -214,17 +214,6 @@ class QueueConsumerTest extends IntegrationTestCase
                 array (
                     'idarchive' => '1',
                     'idsite' => '1',
-                    'date1' => '2018-03-06',
-                    'date2' => '2018-03-06',
-                    'period' => '1',
-                    'name' => 'done5f4f9bafeda3443c3c2d4b2ef4dffadc',
-                    'report' => NULL,
-                    'plugin' => NULL,
-                    'segment' => 'browserCode==IE',
-                ),
-                array (
-                    'idarchive' => '1',
-                    'idsite' => '1',
                     'date1' => '2018-03-04',
                     'date2' => '2018-03-04',
                     'period' => '1',