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

Scheduled tasks should not launch archiving #17976

Closed
tsteur opened this issue Sep 8, 2021 · 4 comments · Fixed by #18101
Closed

Scheduled tasks should not launch archiving #17976

tsteur opened this issue Sep 8, 2021 · 4 comments · Fixed by #18101
Assignees
Labels
Bug For errors / faults / flaws / inconsistencies etc. c: Performance For when we could improve the performance / speed of Matomo. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical.
Milestone

Comments

@tsteur
Copy link
Member

tsteur commented Sep 8, 2021

See af44ee1#diff-f41c275e883996b8b69cb765b1818e12e82cbe034e648bc3f7d03610cc7abad1R449-R456

The trigger=archivephp flag will cause scheduled tasks to allow archiving. Seems this hack was needed for purgeOutdatedArchives to work. We should fix this issue so that scheduled tasks won't launch archiving when browser archiving is disabled.

This can otherwise cause many tasks to become very slow (including scheduled reports, custom alerts, some on the cloud). And it can cause issues with other archives that were invalidated in the sense that for example some reports might be archived multiple times or that they are archived in the wrong order and therefore causes results to be wrong under circumstances. Because usually the archiver makes sure to first run all daily archives, then the weekly, then ... vs this way there might be race conditions or other problems maybe.

@tsteur tsteur added Bug For errors / faults / flaws / inconsistencies etc. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. c: Performance For when we could improve the performance / speed of Matomo. labels Sep 8, 2021
@tsteur tsteur added this to the 4.6.0 milestone Sep 8, 2021
@peterhashair peterhashair self-assigned this Oct 5, 2021
@peterhashair
Copy link
Contributor

@tsteur sorry to bother, If I understand correctly, this issue is differentiating the CLI and the browser request on Archives jobs. When the browser triggers it, it should pass the setting already set by the admin. But through CLI it won't need that check?

@tsteur
Copy link
Member Author

tsteur commented Oct 6, 2021

I'm not sure if it will need that check or not on CLI. That would be part of the issue to figure that out. Generally it be great if we could remove this line $_GET['trigger'] = 'archivephp'; completely. Or if it was added for purgeOutdatedArchives, maybe it could be set only in that task and then unset again if it did not already exist before that.

I would probably start with removing $_GET['trigger'] = 'archivephp'; completely and checking if tests still pass. If that's the case, I would look into why purgeOutdatedArchives could possibly need that flag to trigger archiving and whether it might be safe to just completely remove it.

The goal will be to not have trigger=archivephp during scheduled tasks as this can have unexpected consequences like all tasks triggering archiving etc.

@Starker3
Copy link
Contributor

@tsteur Would this completely prevent scheduled tasks from triggering archiving for specific plugins? For example a user recently had an issue where the IP2Company plugin was launching archiving when the scheduled tasks executed. Due to the configuration of the plugin it seems to rely only on archiving triggered by the scheduled task.

@tsteur
Copy link
Member Author

tsteur commented Nov 30, 2021

Would this completely prevent scheduled tasks from triggering archiving for specific plugins

AFAIK yes it would stop that. Plugins shouldn't archive data during scheduled task as this can have big performance and memory implications. The plugin could probably workaround it by setting $_GET['trigger'] = 'archivephp'; in the scheduled task (and ideally unset it after it's finished)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc. c: Performance For when we could improve the performance / speed of Matomo. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants