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.

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

Allow invalidated archive data to be used until they are re-processed #6116

Merged

Conversation

mkurzeja
Copy link
Contributor

@mkurzeja mkurzeja commented Sep 3, 2014

Added DONE_INVALIDATED flag to archiving and considering it when fetching archives

@mkurzeja
Copy link
Contributor Author

mkurzeja commented Sep 3, 2014

ref #5932

@mkurzeja
Copy link
Contributor Author

mkurzeja commented Sep 3, 2014

This is a small part of the code that shows how I plan to achieve the goal:

ArchiveSelector loads the invalidated data only if archiver will not be triggered in the current request. Thanks to that if archiver can be run it re-processes the data and if not it loads the invalidated ones.

Left to be done:

  1. Mark archive as invalid instead of deleting when CoreAdminHome.invalidateArchivedReports is run
  2. Force core:archive method to detect if site has invalid archives (right now --force-idsites and --force-date-range need to be added).
  3. Add invalid archive deletion in core:archive command

@mkurzeja
Copy link
Contributor Author

mkurzeja commented Sep 8, 2014

@mattab please check this PR. It should be ready to merge

foreach ($archiveTables as $archiveTable) {
$query = '
SELECT t1.idarchive FROM `' . $archiveTable . '` t1
INNER JOIN `piwik_archive_numeric_2014_09` t2
Copy link
Member

Choose a reason for hiding this comment

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

table month is hardcoded here: piwik_archive_numeric_2014_09

@mattab
Copy link
Member

mattab commented Sep 9, 2014

@mkurzeja Nice work, I put feedback inline, looking forward to being able to merge it!

@mattab mattab added Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. labels Sep 9, 2014
@mattab mattab added this to the Piwik 2.7.0 milestone Sep 9, 2014
@mattab mattab added the Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. label Sep 9, 2014
@mkurzeja
Copy link
Contributor Author

@mattab Thanks for the review. I've introduced some changes and updated to the current master.

INNER JOIN `' . $archiveTable . '` t2
ON t1.name = t2.name AND t1.idsite=t2.idsite
AND t1.date1=t2.date1 AND t1.date2=t2.date2 AND t1.period=t2.period
WHERE t1.value = 4 AND t2.value IN(' . ArchiveWriter::DONE_OK . ', ' . ArchiveWriter::DONE_OK_TEMPORARY . ') AND t1.name LIKE \'done%\'';
Copy link
Member

Choose a reason for hiding this comment

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

magic number 4 should be using the const

public function testAnotherApi($api, $params)
{
$this->setBrowserArchivingTriggering(1);
$this->invalidateTestArchives();
Copy link
Member

Choose a reason for hiding this comment

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

maybe this line can be removed as it's already called before in testSameApi

@mattab
Copy link
Member

mattab commented Sep 17, 2014

Nice. Besides review above, the tests are easy to read and understand!

Well done, looking forward to merge it.

@mattab mattab added the Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. label Sep 17, 2014
@mkurzeja
Copy link
Contributor Author

@mattab Thanks for the review. I've added all changes and updated to the current master

mattab pushed a commit that referenced this pull request Sep 21, 2014
…ed-reports

fixes #5932 Allow invalidated archive data to be used until they are re-processed
@mattab mattab merged commit 0fac10d into matomo-org:master Sep 21, 2014
@mattab
Copy link
Member

mattab commented Sep 21, 2014

Kuddos @mkurzeja for this nicely done Pull request and big usability improvement to Piwik users who will now enjoy reports filled with data until the reports will be re-processed. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants