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

Skip invalidating segments for today if flag is provided #22519

Merged
merged 7 commits into from
Sep 2, 2024

Conversation

sgiehl
Copy link
Member

@sgiehl sgiehl commented Aug 21, 2024

Description:

The core:archive command provides a flag --skip-segments-today, which prevents segments data from being processed for today.

The currently implementation only prevents them from being processed. But invalidation done for today before archiving is started is currently done anyway. This causes also the higher periods (week, month, year) to be invalidated. But as only today is prevented from being archived for segments, those higher periods will be reprocessed anyway, even if there actually were no changes in lower periods.

To prevent this unnecessary archiving of those higher periods, this PR changes the behavior of the initial invalidation. If --skip-segments-today is provided, we will no longer create invalidations for segments for today (and periods including this day).
This shouldn't cause any problems even if the flag is used always when archiving is triggered, as yesterday will still be invalidated for segments.

Review

@sgiehl sgiehl added this to the 5.2.0 milestone Aug 21, 2024
Comment on lines -911 to -926
if ($isYesterday) {
// Skip invalidation for yesterday if archiving for yesterday was already started after midnight in site's timezone
$invalidationsInProgress = $this->model->getInvalidationsInProgress($idSite);
$today = Date::factoryInTimezone('today', $timezone);

foreach ($invalidationsInProgress as $invalidation) {
if (
$invalidation['period'] == Day::PERIOD_ID
&& $date->toString() === $invalidation['date1']
&& Date::factory($invalidation['ts_started'], $timezone)->getTimestamp() >= $today->getTimestamp()
) {
$this->logger->debug(" " . ucfirst($dateStr) . " archive already in process for idSite = $idSite, skipping invalidation...");
return;
}
}
}
Copy link
Member Author

@sgiehl sgiehl Aug 22, 2024

Choose a reason for hiding this comment

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

This check was incorrect here. On the one side it missed to check the done flag, causing any running archive to prevent a new invalidation. On the other side a running invalidation prevented any segment from being invalidated.
Therefor the check was moved to invalidateWithSegments method, where it is done for each segment separately.

@sgiehl sgiehl marked this pull request as ready for review August 23, 2024 08:59
@sgiehl sgiehl added the Needs Review PRs that need a code review label Aug 27, 2024
@sgiehl sgiehl requested a review from a team August 27, 2024 12:22
@mneudert mneudert merged commit c51e175 into fixintersectingcheck Sep 2, 2024
25 checks passed
@mneudert mneudert deleted the dev-18395 branch September 2, 2024 13:49
@sgiehl sgiehl mentioned this pull request Sep 11, 2024
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants