Skip to content
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.

Sign up for GitHub

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

Start storing the processing host and process id for started invalidations #22981

Open
wants to merge 2 commits into
base: 5.x-dev
Choose a base branch
from
+176 −45
Open

Start storing the processing host and process id for started invalidations #22981

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1fd9e83
Start storing the processing host and process id for started invalida…
sgiehl Jan 24, 2025
407d1a8
remove unreachable code and add test
sgiehl Jan 24, 2025
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
48 changes: 5 additions & 43 deletions core/DataAccess/Model.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -713,54 +713,16 @@ public function startArchive($invalidation)
$table = Common::prefixTable('archive_invalidations');

// set archive value to in progress if not set already
$statement = Db::query("UPDATE `$table` SET `status` = ?, ts_started = NOW() WHERE idinvalidation = ? AND status = ?", [
$statement = Db::query("UPDATE `$table` SET `status` = ?, `processing_host` = ?, `process_id` = ?, `ts_started` = NOW() WHERE `idinvalidation` = ? AND `status` = ?", [
ArchiveInvalidator::INVALIDATION_STATUS_IN_PROGRESS,
gethostname() ?: null,
Common::getProcessId(),
$invalidation['idinvalidation'],
ArchiveInvalidator::INVALIDATION_STATUS_QUEUED,
]);

if ($statement->rowCount() > 0) { // if we updated, then we've marked the archive as started
return true;
}

// archive was not originally started or was started within the expected time, so we assume it's ongoing and another process
// (on this machine or another) is actively archiving it.
$archiveFailureRecoveryTimeout = GeneralConfig::getConfigValue('archive_failure_recovery_timeout', $invalidation['idsite']);
if (
empty($invalidation['ts_started'])
|| $invalidation['ts_started'] > Date::now()->subSeconds($archiveFailureRecoveryTimeout)->getTimestamp()
Copy link
Member Author

@sgiehl sgiehl Jan 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check never worked properly as ts_archived is provided from the database as datetime like 2025-01-01 12:00:00. Comparing that this way with a timestamp always results in true, causing the code below the check to never get executed.
As we are resetting stale invalidations in the begin of the archiving process, that shouldn't be required here anyway.

) {
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'],
$invalidation['period'],
$invalidation['date1'],
$invalidation['date2'],
$invalidation['name'],
ArchiveInvalidator::INVALIDATION_STATUS_IN_PROGRESS,
];

if (empty($invalidation['report'])) {
$reportClause = "(report IS NULL OR report = '')";
} else {
$reportClause = "report = ?";
$bind[] = $invalidation['report'];
}

$sql = "DELETE FROM " . Common::prefixTable('archive_invalidations') . " WHERE idinvalidation < ? AND idsite = ? AND "
. "date1 = ? AND date2 = ? AND `period` = ? AND `name` = ? AND $reportClause";
Db::query($sql, $bind);

return true;
// if we updated, then we've marked the archive as started
return $statement->rowCount() > 0;
}

public function isSimilarArchiveInProgress($invalidation)
Expand Down
2 changes: 2 additions & 0 deletions core/Db/Schema/Mysql.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,8 @@ public function getTablesCreateSql()
ts_started DATETIME NULL,
status TINYINT(1) UNSIGNED DEFAULT 0,
`report` VARCHAR(255) NULL,
processing_host VARCHAR(100) NULL DEFAULT NULL,
process_id VARCHAR(15) NULL DEFAULT NULL,
PRIMARY KEY(idinvalidation),
INDEX index_idsite_dates_period_name(idsite, date1, period)
) $tableOptions
Expand Down
42 changes: 42 additions & 0 deletions core/Updates/5.3.0-b1.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<?php

/**
* Matomo - free/libre analytics platform
*
* @link https://matomo.org
* @license https://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
*/

namespace Piwik\Updates;

use Piwik\Updater;
use Piwik\Updater\Migration\Factory as MigrationFactory;
use Piwik\Updates;

class Updates_5_3_0_b1 extends Updates
{
/**
* @var MigrationFactory
*/
private $migration;

public function __construct(MigrationFactory $factory)
{
$this->migration = $factory;
}

public function getMigrations(Updater $updater)
{
return [
$this->migration->db->addColumns('archive_invalidations', [
'processing_host' => 'VARCHAR(100) NULL DEFAULT NULL',
'process_id' => 'VARCHAR(15) NULL DEFAULT NULL',
])
];
}

public function doUpdate(Updater $updater)
{
$updater->executeMigrations(__FILE__, $this->getMigrations($updater));
}
}
2 changes: 1 addition & 1 deletion core/Version.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ final class Version
* The current Matomo version.
* @var string
*/
public const VERSION = '5.3.0-alpha';
public const VERSION = '5.3.0-b1';

public const MAJOR_VERSION = 5;

Expand Down
50 changes: 49 additions & 1 deletion tests/PHPUnit/Integration/CronArchive/QueueConsumerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ public function testConsumerIgnoresPeriodsThatHaveBeenDisabledInApi()
'report' => null,
'plugin' => null,
'segment' => '',
'processing_host' => null,
'process_id' => null,
],
],
[
Expand All @@ -132,6 +134,8 @@ public function testConsumerIgnoresPeriodsThatHaveBeenDisabledInApi()
'report' => null,
'plugin' => null,
'segment' => '',
'processing_host' => null,
'process_id' => null,
],
],
[],
Expand Down Expand Up @@ -260,6 +264,8 @@ public function testInvalidateConsumeOrder()
'segment' => '',
'ts_started' => null,
'status' => '0',
'processing_host' => null,
'process_id' => null,
),
array (
'idarchive' => '1',
Expand All @@ -273,6 +279,8 @@ public function testInvalidateConsumeOrder()
'segment' => '',
'ts_started' => null,
'status' => '0',
'processing_host' => null,
'process_id' => null,
),
array (
'idarchive' => '1',
Expand All @@ -286,6 +294,8 @@ public function testInvalidateConsumeOrder()
'segment' => '',
'ts_started' => null,
'status' => '0',
'processing_host' => null,
'process_id' => null,
),
),
array (
Expand All @@ -301,6 +311,8 @@ public function testInvalidateConsumeOrder()
'segment' => 'browserCode==IE;dimension1==val',
'ts_started' => null,
'status' => '0',
'processing_host' => null,
'process_id' => null,
),
array (
'idarchive' => '1',
Expand All @@ -314,6 +326,8 @@ public function testInvalidateConsumeOrder()
'segment' => 'browserCode==IE;dimension1==val',
'ts_started' => null,
'status' => '0',
'processing_host' => null,
'process_id' => null,
),
array (
'idarchive' => '1',
Expand All @@ -327,6 +341,8 @@ public function testInvalidateConsumeOrder()
'segment' => '',
'ts_started' => null,
'status' => '0',
'processing_host' => null,
'process_id' => null,
),
),
array (
Expand All @@ -342,6 +358,8 @@ public function testInvalidateConsumeOrder()
'segment' => 'browserCode==IE;dimension1==val',
'ts_started' => null,
'status' => '0',
'processing_host' => null,
'process_id' => null,
),
array (
'idarchive' => '1',
Expand All @@ -355,6 +373,8 @@ public function testInvalidateConsumeOrder()
'segment' => '',
'ts_started' => null,
'status' => '0',
'processing_host' => null,
'process_id' => null,
),
),
array (
Expand All @@ -370,6 +390,8 @@ public function testInvalidateConsumeOrder()
'segment' => 'browserCode==IE;dimension1==val',
'ts_started' => null,
'status' => '0',
'processing_host' => null,
'process_id' => null,
),
),
array (
Expand All @@ -385,6 +407,8 @@ public function testInvalidateConsumeOrder()
'segment' => '',
'ts_started' => null,
'status' => '0',
'processing_host' => null,
'process_id' => null,
),
),
array (
Expand All @@ -400,6 +424,8 @@ public function testInvalidateConsumeOrder()
'segment' => '',
'ts_started' => null,
'status' => '0',
'processing_host' => null,
'process_id' => null,
),
),
array (
Expand All @@ -415,6 +441,8 @@ public function testInvalidateConsumeOrder()
'segment' => 'browserCode==IE;dimension1==val',
'ts_started' => null,
'status' => '0',
'processing_host' => null,
'process_id' => null,
),
),
array (
Expand All @@ -430,6 +458,8 @@ public function testInvalidateConsumeOrder()
'segment' => '',
'ts_started' => null,
'status' => '0',
'processing_host' => null,
'process_id' => null,
),
),
array (
Expand All @@ -445,6 +475,8 @@ public function testInvalidateConsumeOrder()
'segment' => '',
'ts_started' => null,
'status' => '0',
'processing_host' => null,
'process_id' => null,
),
),
array (
Expand All @@ -460,6 +492,8 @@ public function testInvalidateConsumeOrder()
'segment' => 'browserCode==IE;dimension1==val',
'ts_started' => null,
'status' => '0',
'processing_host' => null,
'process_id' => null,
),
),
array ( // end of idsite=1
Expand All @@ -477,6 +511,8 @@ public function testInvalidateConsumeOrder()
'segment' => '',
'ts_started' => null,
'status' => '0',
'processing_host' => null,
'process_id' => null,
),
),
array ( // end of idsite=2
Expand Down Expand Up @@ -568,7 +604,7 @@ public function testPluginInvalidationDeletedIfUsableArchiveExists()

$expectedInvalidationsFound = [
array(
['idarchive' => '1', 'idsite' => '1', 'date1' => '2018-03-04', 'date2' => '2018-03-04', 'period' => '3', 'name' => 'done', 'report' => null, 'plugin' => null, 'segment' => '', 'ts_started' => null, 'status' => '0']
['idarchive' => '1', 'idsite' => '1', 'date1' => '2018-03-04', 'date2' => '2018-03-04', 'period' => '3', 'name' => 'done', 'report' => null, 'plugin' => null, 'segment' => '', 'ts_started' => null, 'status' => '0', 'processing_host' => null, 'process_id' => null]
),
array()
];
Expand Down Expand Up @@ -665,6 +701,8 @@ public function testSkipSegmentsToday()
'segment' => '',
'ts_started' => null,
'status' => '0',
'processing_host' => null,
'process_id' => null,
],
[
'idarchive' => '3',
Expand All @@ -678,6 +716,8 @@ public function testSkipSegmentsToday()
'segment' => 'browserCode==IE',
'ts_started' => null,
'status' => '0',
'processing_host' => null,
'process_id' => null,
],
],
[
Expand All @@ -693,6 +733,8 @@ public function testSkipSegmentsToday()
'segment' => 'browserCode==IE',
'ts_started' => null,
'status' => '0',
'processing_host' => null,
'process_id' => null,
],
],
[
Expand All @@ -708,6 +750,8 @@ public function testSkipSegmentsToday()
'report' => null,
'plugin' => null,
'segment' => '',
'processing_host' => null,
'process_id' => null,
],
],
[
Expand All @@ -723,6 +767,8 @@ public function testSkipSegmentsToday()
'report' => null,
'plugin' => null,
'segment' => 'browserCode==FF',
'processing_host' => null,
'process_id' => null,
],
],
[// end of idsite=1
Expand Down Expand Up @@ -884,6 +930,8 @@ public function testMaxWebsitesToProcess()
'report' => null,
'plugin' => null,
'segment' => '',
'processing_host' => null,
'process_id' => null,
],
]
];
Expand Down
Loading
Loading