disable cron task when segment is deleted #18383

Description:

Fixes:#16152
disable cron task when segment is deleted

Review

• [ ] Functional review done
• [ ] Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
• [ ] Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
• [ ] Security review done
• [ ] Wording review done
• [ ] Code review done
• [ ] Tests were added if useful/possible
• [ ] Reviewed for breaking changes
• [ ] Developer changelog updated if needed
• [ ] Documentation added if needed
• [ ] Existing documentation updated if needed

not sure that's the right solution. but it should skip the segment mark as deleted.

@sgiehl right, that makes more sense now. Update a little bit, hopeful I am on the right direction :)

This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers

@sgiehl Have a deep look at this one, if I am right I think a new function doesn't even needed, it seems like already checked in here. I guess it's somewhere else.

https://github.com/matomo-org/matomo/blob/8e1aabfc68ad16ea8af059a9108a54e6053aa414/core/CronArchive/SegmentArchiving.php#L83-L104

@peterhashair Were you actually able to reproduce the error that is described in the issue in any way?
The only way I was able to reproduce the exact same error message was when I disabled a plugin while archiving was running. So the archiving requests then failed due to the missing segments. If the segment is already invalid before the archiving starts it seems to be skipped without resulting in an error.

I have created a test case, that reflects the way I was able to reproduce. So the process that starts the archiving receives a segment from UserLanguage plugin. But for the archiving requests that are sent the plugin is disabled, causing the segment to be invalid.
The test currently fails. I guess the target would be to adjust the archiving, so such requests with an invalid segment won't throw an error or that specific error is handled properly, without resulting in a failure.

If you were able to reproduce that another way, feel free to add an additional test case for that as well.

@sgiehl I saw the new test, reproduced it. Should be fixed now.

@peterhashair Actually you have adjusted the test case so it covers another problem. Disabling the plugin before the archiver runs, let's the archiver skip the segments. That actually was already the case before, even though the message was not that clear.
But the error message of the original issue is

ERROR [2018-03-12 00:12:06] Got invalid response from API request: ?module=API&method=API.get&idSite=2&period=day&date=last52&format=php&trigger=archivephp. Response was 'a:2:{s:6:"result";s:5:"error";s:7:"message";s:81:"Segment 'dimension7' is not a supported segment. - caused by plugin CustomReports";}'

That actually happens if the archiver already started, but while looping over the archiving requests the plugin gets disabled. So a request for archiving an invalid segment is sent, causing the error above.

I guess we need to ignore if a archiving request results in that specific error. And log an info about that instead.

@sgiehl right, that makes more sense. Update a little. Hopefully is correct :)

@sgiehl update the log info. 😀