Skip to content

Remove archive status locking since it is not needed anymore #17657

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 1 addition & 10 deletions core/Archive/ArchiveInvalidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,7 @@

namespace Piwik\Archive;

use Piwik\Access;
use Piwik\Archive\ArchiveInvalidator\InvalidationResult;
use Piwik\ArchiveProcessor\ArchivingStatus;
use Piwik\ArchiveProcessor\Loader;
use Piwik\ArchiveProcessor\Rules;
use Piwik\Config;
use Piwik\Container\StaticContainer;
Expand Down Expand Up @@ -73,11 +70,6 @@ class ArchiveInvalidator
*/
private $model;

/**
* @var ArchivingStatus
*/
private $archivingStatus;

/**
* @var SegmentArchiving
*/
Expand All @@ -93,10 +85,9 @@ class ArchiveInvalidator
*/
private $allIdSitesCache;

public function __construct(Model $model, ArchivingStatus $archivingStatus, LoggerInterface $logger)
public function __construct(Model $model, LoggerInterface $logger)
{
$this->model = $model;
$this->archivingStatus = $archivingStatus;
$this->segmentArchiving = null;
$this->logger = $logger;
}
Expand Down
14 changes: 2 additions & 12 deletions core/ArchiveProcessor/Loader.php
Original file line number Diff line number Diff line change
Expand Up @@ -154,18 +154,8 @@ private function prepareArchiveImpl($pluginName)
$this->logger->info("initiating archiving via core:archive for " . $this->params);
}

/** @var ArchivingStatus $archivingStatus */
$archivingStatus = StaticContainer::get(ArchivingStatus::class);
$locked = $archivingStatus->archiveStarted($this->params);

try {
list($visits, $visitsConverted) = $this->prepareCoreMetricsArchive($visits, $visitsConverted);
list($idArchive, $visits) = $this->prepareAllPluginsArchive($visits, $visitsConverted);
} finally {
if ($locked) {
$archivingStatus->archiveFinished();
}
}
list($visits, $visitsConverted) = $this->prepareCoreMetricsArchive($visits, $visitsConverted);
list($idArchive, $visits) = $this->prepareAllPluginsArchive($visits, $visitsConverted);

if ($this->isThereSomeVisits($visits) || PluginsArchiver::doesAnyPluginArchiveWithoutVisits()) {
return [[$idArchive], $visits];
Expand Down
1 change: 0 additions & 1 deletion core/Concurrency/Lock.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
*/
namespace Piwik\Concurrency;

use Piwik\ArchiveProcessor\ArchivingStatus;
use Piwik\Common;
use Piwik\Date;

Expand Down
23 changes: 1 addition & 22 deletions core/DataAccess/ArchivingDbAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@

namespace Piwik\DataAccess;

use Piwik\ArchiveProcessor\ArchivingStatus;
use Piwik\Concurrency\Lock;
use Piwik\Config;
use Piwik\Db\AdapterInterface;
use Piwik\DbHelper;
Expand All @@ -23,11 +21,6 @@ class ArchivingDbAdapter
*/
private $wrapped;

/**
* @var Lock
*/
private $archivingLock;

/**
* @var LoggerInterface
*/
Expand All @@ -38,10 +31,9 @@ class ArchivingDbAdapter
*/
private $maxExecutionTime;

public function __construct($wrapped, Lock $archivingLock = null, LoggerInterface $logger = null)
public function __construct($wrapped, LoggerInterface $logger = null)
{
$this->wrapped = $wrapped;
$this->archivingLock = $archivingLock;
$this->logger = $logger;
$this->maxExecutionTime = (float) Config::getInstance()->General['archiving_query_max_execution_time'];
}
Expand All @@ -53,7 +45,6 @@ public function __call($name, $arguments)

public function exec($sql)
{
$this->reexpireLock();
$sql = DbHelper::addMaxExecutionTimeHintToQuery($sql, $this->maxExecutionTime);
$this->logSql($sql);

Expand All @@ -62,7 +53,6 @@ public function exec($sql)

public function query($sql)
{
$this->reexpireLock();
$sql = DbHelper::addMaxExecutionTimeHintToQuery($sql, $this->maxExecutionTime);
$this->logSql($sql);

Expand All @@ -71,7 +61,6 @@ public function query($sql)

public function fetchAll($sql)
{
$this->reexpireLock();
$sql = DbHelper::addMaxExecutionTimeHintToQuery($sql, $this->maxExecutionTime);
$this->logSql($sql);

Expand All @@ -80,7 +69,6 @@ public function fetchAll($sql)

public function fetchRow($sql)
{
$this->reexpireLock();
$sql = DbHelper::addMaxExecutionTimeHintToQuery($sql, $this->maxExecutionTime);
$this->logSql($sql);

Expand All @@ -89,7 +77,6 @@ public function fetchRow($sql)

public function fetchOne($sql)
{
$this->reexpireLock();
$sql = DbHelper::addMaxExecutionTimeHintToQuery($sql, $this->maxExecutionTime);
$this->logSql($sql);

Expand All @@ -98,7 +85,6 @@ public function fetchOne($sql)

public function fetchAssoc($sql)
{
$this->reexpireLock();
$sql = DbHelper::addMaxExecutionTimeHintToQuery($sql, $this->maxExecutionTime);
$this->logSql($sql);

Expand All @@ -112,11 +98,4 @@ private function logSql($sql)
$this->logger->debug($sql);
}
}

private function reexpireLock()
{
if ($this->archivingLock) {
$this->archivingLock->reexpireLock();
}
}
}
6 changes: 1 addition & 5 deletions core/DataAccess/LogAggregator.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
*/
namespace Piwik\DataAccess;

use Piwik\ArchiveProcessor\ArchivingStatus;
use Piwik\ArchiveProcessor\Parameters;
use Piwik\Common;
use Piwik\Config;
Expand Down Expand Up @@ -1240,9 +1239,6 @@ public static function makeArrayOneColumn($row, $columnName, $lookForThisPrefix

public function getDb()
{
/** @var ArchivingStatus $archivingStatus */
$archivingStatus = StaticContainer::get(ArchivingStatus::class);
$archivingLock = $archivingStatus->getCurrentArchivingLock();
return new ArchivingDbAdapter(Db::getReader(), $archivingLock, $this->logger);
return new ArchivingDbAdapter(Db::getReader(), $this->logger);
}
}
29 changes: 13 additions & 16 deletions core/DataAccess/Model.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

use Exception;
use Piwik\Archive\ArchiveInvalidator;
use Piwik\ArchiveProcessor\ArchivingStatus;
use Piwik\ArchiveProcessor\Parameters;
use Piwik\ArchiveProcessor\Rules;
use Piwik\Common;
Expand All @@ -35,15 +34,9 @@ class Model
*/
private $logger;

/**
* @var ArchivingStatus
*/
private $archivingStatus;

public function __construct(LoggerInterface $logger = null)
{
$this->logger = $logger ?: StaticContainer::get('Psr\Log\LoggerInterface');
$this->archivingStatus = StaticContainer::get(ArchivingStatus::class);
}

/**
Expand Down Expand Up @@ -696,16 +689,20 @@ public function startArchive($invalidation)
return true;
}

// if we didn't get anything, some process either got there first, OR
// the archive was started previously and failed in a way that kept it's done value
// set to DONE_IN_PROGRESS. try to acquire the lock and if acquired, archiving isn' in process
// so we can claim it.
$lock = $this->archivingStatus->acquireArchiveInProgressLock($invalidation['idsite'], $invalidation['date1'],
$invalidation['date2'], $invalidation['period'], $invalidation['name']);
if (!$lock->isLocked()) {
return false; // we couldn't claim the lock, archive is in progress
// archive was not originally started or was started within 24 hours, we assume it's ongoing and another process
// (on this machine or another) is actively archiving it.
if (empty($invalidation['ts_started'])
|| $invalidation['ts_started'] > Date::now()->subDay(1)->getTimestamp()
) {
return false;
}

// archive was started over 24 hours ago, we assume it failed and take it over
Db::query("UPDATE `$table` SET `status` = ?, ts_started = NOW() WHERE idinvalidation = ?", [
ArchiveInvalidator::INVALIDATION_STATUS_IN_PROGRESS,
$invalidation['idinvalidation'],
]);

// remove similar invalidations w/ lesser idinvalidation values
$bind = [
$invalidation['idsite'],
Expand Down Expand Up @@ -767,7 +764,7 @@ public function isSimilarArchiveInProgress($invalidation)
public function getNextInvalidatedArchive($idSite, $archivingStartTime, $idInvalidationsToExclude = null, $useLimit = true)
{
$table = Common::prefixTable('archive_invalidations');
$sql = "SELECT idinvalidation, idarchive, idsite, date1, date2, period, `name`, report, ts_invalidated
$sql = "SELECT *
FROM `$table`
WHERE idsite = ? AND status != ? AND ts_invalidated <= ?";
$bind = [
Expand Down
87 changes: 0 additions & 87 deletions tests/PHPUnit/Integration/ArchiveProcessor/ArchivingStatusTest.php

This file was deleted.

Loading