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

Kill full sitemap cron generation #99

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

mjangda
Copy link
Member

@mjangda mjangda commented Jan 4, 2017

For really large datasets, this ends up in a flood of thousands of cron events that can
slow down or completely disable a site.

Fixes #81

@mjangda
Copy link
Member Author

mjangda commented Jan 4, 2017

This keeps the ongoing cron-based updates (triggered every 15 minutes) although can be replaced with your own system, see https://github.com/Automattic/msm-sitemap/blob/master/wpcom-helper.php#L3

@pkevan
Copy link
Contributor

pkevan commented Jan 9, 2017

Wouldn't this change result in lots of queued crons, but no action to execute them?

msm_cron_generate_sitemap_for_year_month_day should follow something similar to https://github.com/Automattic/msm-sitemap/blob/master/vipgo-helper.php#L8 unless I've misread the changeset.

For really large datasets, this ends up in a flood of thousands of cron events that can
slow down or completely disable a site.

Fixes #81
We no longer need to disable the cron and add a workaround since the
initial cron stuff no longer exists.
We need something to actually trigger the scheduled cron events
@mjangda
Copy link
Member Author

mjangda commented Jan 9, 2017

Wouldn't this change result in lots of queued crons, but no action to execute them?

Yep, great catch. I've restored the relevant code. A few other things we probably need to do:

  • Update tests.
  • Make sure wpcom-helper.php doesn't need any more changes.
  • Remove any related options from other files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants