@tsteur opened this Pull Request on August 23rd 2017 Owner

When browser archiving is disabled, and it is eg shortly after midnight but no new period archive has been processed yet, the reports for week / month / year will all show zeros. This is because we only accept temporary archives from the same day.

However, if it is just after midnight, or an hour after midnight, any archive that is like only a few hours old is still accurate enough and potentially even more accurate then any archive that was fetched eg at 4pm and generated at 8am (which we would still consider acceptable currently as it is generated on the same day).

I went with 12 hours for now. Could also only allow up to 6 hours back but someone might actually configure larger processing intervals via #11965 in the future and therefore 12 should be better. Ideally in #11965, if someone sets eg 24 hours for "yearly periods", then we would adjust this number to 24 hours back.

I tested it locally and worked for me but not sure re any possible side effects.

@tsteur commented on August 23rd 2017 Owner

Moved it into 3.1.0 but feel free to move it to 3.2.0

@tsteur commented on August 23rd 2017 Owner

Looking at the code again it should be quite a save merge as it is only executed when archiving is disabled anyway.

@mattab commented on September 5th 2017 Owner

-> I cannot reproduce the issue locally, in my tests a 2 day old temporary month archive is loaded anyway. Wondering if i'm doing something wrong or if the issue we experienced of a "no data report" after midnight is due to some other problem?

It would have made total sense that this should work but somehow I couldn't figure it out and then realised that this getMinTimeProcessedForTemporaryArchive function is only called for a process whcih can trigger archiving:

getMinTimeProcessedForTemporaryArchive called by
getMinTimeArchiveProcessed called by
loadExistingArchiveIdFromDb called by
prepareArchive called by
cacheArchiveIdsAfterLaunching
which is only called when archiving is enabled for current request:

            if (!Rules::isArchivingDisabledFor($this->params->getIdSites(), $this->params->getSegment(), $this->getPeriodLabel())) {
                $this->cacheArchiveIdsAfterLaunching($archiveGroups, $plugins);
            } else {
                $this->cacheArchiveIdsWithoutLaunching($plugins);
            }

In the code path for "Browser archiving is disabled", then the code already loads the last available archive and it could even be a few days old it seems?!

@tsteur commented on September 5th 2017 Owner

I totally get what you mean. It seems to never even go into that part of the method. I think when I tested it, I set in that method archivingDisabled=false and then it worked but when disabling archiving properly it does not even go in there.

I wonder if the merge still makes sense. I tested also with browser archiving enabled and then it doesn't go into that path either because archiving needs to be first enabled. I think it may go in there though if somehow:

  • Archiving is disabled but a range is requested and archiving_range_force_on_browser_request is enabled. (couldn't reproduce that though)
  • Archiving is enabled but a range is requested and archiving_range_force_on_browser_request is disabled. (couldn't reproduce that though)
  • If somehow for some periods archiving is enabled but not for others eg in here: https://github.com/piwik/piwik/blob/3.1.0-rc1/core/Archive.php#L669-L674 as we loop through all sites and all periods

Probs be save to just close the issue for now.

FYI: Tried to find out why it was initially added but couldn't find it in eg https://github.com/piwik/piwik/commit/dd2140c8c3faf2cfca30fd6fea0350ca953695da

Powered by GitHub Issue Mirror