Archiving job prioritization and safety precautions #15991
Conversation
diosmosis
added
not-in-changelog
| @@ -235,6 +240,8 @@ public function __construct($processNewSegmentsFrom = null, LoggerInterface $log | |||
| $this->periodIdsToLabels = array_flip(Piwik::$idPeriods); | |||
|
|
|||
| $this->rawLogDao = new RawLogDao(); | |||
|
|
|||
| $this->cliMultiRequestParser = new RequestParser($this->makeCliMulti()->supportsAsync()); | |||
There was a problem hiding this comment.
@diosmosis did you maybe forget to commit this file?
|
@diosmosis now getting an exception |
|
Oh, huh, I forgot to implement that, but the tests were passing for me... interesting, will fix. |
|
@tsteur pushed |
There was a problem hiding this comment.
@diosmosis found one more notice. Also not sure if I missed it or if this is covered by a lock. Should the very same archive already be in process (same site, period, date and segment (if given)) to then skip as well?
| } | ||
|
|
||
| if ($this->isArchiveNonSegmentAndInProgressArchiveSegment($archiveToProcess, $archiveBeingProcessed)) { | ||
| return "segment archive in progress for same site/period ({$archiveBeingProcessed['segment']})"; |
There was a problem hiding this comment.
fyi segment might not be defined here
got Notice - Undefined index: segment
| if (empty($invalidatedArchive)) { | ||
| $this->logger->debug("No next invalidated archive."); | ||
| break; | ||
| } | ||
|
|
||
| if ($this->hasDifferentPeriod($archivesToProcess, $invalidatedArchive['period'])) { |
Covered by two things, first is that if the archive is DONE_IN_PROGRESS we shouldn't handle it. Second is if it is locked and not expired, we shouldn't handle it. We could also just do |
|
FYI, I'm planning on refactoring/testing all of CronArchive in #11974 |
|
@diosmosis don't have to add it unless it's easy. The DONE IN PROGRESS should be fine too |
|
@tsteur single character change, so definitely easy |
|
👍 feel free to change it and I have another look |
|
@tsteur you can take another look |
|
@diosmosis some tests are failing now. Otherwise looks good to merge |
Refs #15117