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

Stop firing WP CRON when it's disabled #6282

Closed
wordpressfan opened this issue Nov 22, 2023 · 3 comments · Fixed by #6295
Closed

Stop firing WP CRON when it's disabled #6282

wordpressfan opened this issue Nov 22, 2023 · 3 comments · Fixed by #6295
Labels
effort: [S] 1-2 days of estimated development time needs: documentation Issues which need to create or update a documentation priority: medium Issues which are important, but no one will go out of business. type: enhancement Improvements that slightly enhance existing functionality and are fast to implement
Milestone

Comments

@wordpressfan
Copy link
Contributor

wordpressfan commented Nov 22, 2023

Before submitting an issue please check that you’ve completed the following steps:

  • Made sure you’re on the latest version Yes
  • Used the search feature to ensure that the bug hasn’t been reported before Yes

Describe the bug
When the constant DISABLE_WP_CRON is true, we need to stop calling the method spawn_cron here:

This was a suggestion from Kinsta not to fire the CRON because this may cause CPU issues.

To Reproduce
Steps to reproduce the behavior:

  1. Add the following line in wp-config.php define( 'DISABLE_WP_CRON', true )
  2. Activate RUCSS
  3. Wait in WPR settings page for 90 seconds
  4. Ajax request will be sent to our backend to fire the CRON

Expected behavior
CRON should not be called at all when this constant is set.

Additional context
Slack convos:

https://wp-media.slack.com/archives/CUT7FLHF1/p1700469134274159
https://wp-media.slack.com/archives/C056ZJMHG0P/p1700248478315299

Acceptance Criteria (for WP Media team use only)

  1. When the CRON is disabled, we should run our functions manually to make sure that Used CSS for homepage is generated
  2. We should not spawn CRON if it's disabled
@wordpressfan
Copy link
Contributor Author

Reproduce the issue

Yes

Root cause

As mentioned in the issue, we force the cron to run once the RUCSS option is activated and wait for 90 seconds, this ajax request will be sent to backend to fire the CRON.

Scope a solution

We need to fix this from both sides (the js part and the php part)

JS part:
Here

$data['notice_end_time'] = $transient;

We need to add a new key for cron_disabled which has the value as below:

$data['cron_disabled'] = rocket_get_constant( 'DISABLE_WP_CRON', false );

Then in JS here add new condition to check on this new attribute rocket_ajax_data.cron_disabled

PHP Part:
Here we need to add the following check:

if ( rocket_get_constant( 'DISABLE_WP_CRON', false ) ) {
    return;//Bailout and don't fire the CRON.
}

#Effort
[S]

@wordpressfan wordpressfan added the effort: [S] 1-2 days of estimated development time label Nov 27, 2023
@Miraeld Miraeld removed their assignment Nov 27, 2023
@Miraeld
Copy link
Contributor

Miraeld commented Nov 27, 2023

Seems good to me :)

@piotrbak piotrbak added type: enhancement Improvements that slightly enhance existing functionality and are fast to implement priority: medium Issues which are important, but no one will go out of business. labels Nov 27, 2023
@Miraeld Miraeld self-assigned this Nov 28, 2023
@piotrbak piotrbak modified the milestone: 3.15.6 Nov 29, 2023
@piotrbak
Copy link
Contributor

piotrbak commented Dec 3, 2023

Changed the Acceptance Criteria here.

@Miraeld Miraeld removed their assignment Dec 4, 2023
@Mai-Saad Mai-Saad modified the milestones: 3.15.6, 3.15.5 Dec 4, 2023
@piotrbak piotrbak added this to the 3.15.6 milestone Dec 4, 2023
github-merge-queue bot pushed a commit that referenced this issue Dec 5, 2023
@piotrbak piotrbak added the needs: documentation Issues which need to create or update a documentation label Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort: [S] 1-2 days of estimated development time needs: documentation Issues which need to create or update a documentation priority: medium Issues which are important, but no one will go out of business. type: enhancement Improvements that slightly enhance existing functionality and are fast to implement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants