is_cron_disabled doesn’t make sense

  • Resolved Rasso Hilber

    (@nonverbla)


    1 year, 8 months ago

    Hi there! I just noticed one of my sites getting really slow when performing some simple tasks (wp_delete_post for example).

    Turns out it was very slow db queries to the sg-security logs table. Here is an example:

    INSERT INTO wp_sgs_log_events (visitor_id, ts, activity, description, ip, hostname, code, object_id, type, action, visitor_type) VALUES (2, 1679387722, 'Deleted Post', 'Deleted Post - Benchmark for wp_delete_post #1', '127.0.0.1', 'localhost', 200, 'wpcli', 'post', 'delete', 'user')

    That table was huge in my case. After digging through the code, I noticed this function:

    /**
 * Check if wp cron is disabled and send error message.
 *
 * @since  1.0.0
 */
public static function is_cron_disabled() {
	if ( defined( 'DISABLE_WP_CRON' ) && true == DISABLE_WP_CRON ) {
		return 1;
	}

	return 0;
}

    …and this one:

    /**
 * Set the cron job for deleting old logs.
 *
 * @since  1.0.0
 */
public function set_sgs_logs_cron() {
	// Bail if cron is disabled.
	if ( 1 === Helper_Service::is_cron_disabled() ) {
		return;
	}

	if ( ! wp_next_scheduled( 'siteground_security_clear_logs_cron' ) ) {
		wp_schedule_event( time(), 'daily', 'siteground_security_clear_logs_cron' );
	}
}

    The problem with this is, that the constant DISABLE_WP_CRON is only there to disable the WordPress fake cron. On my site I have a real cron job running. So set_sgs_logs_cron should actually always schedule the cleanup job, even if DISABLE_WP_CRON was set to false.

Viewing 4 replies - 1 through 4 (of 4 total)
  • Thread Starter Rasso Hilber

    (@nonverbla)

    This function could still be run if on the plugin page, regardless of the cron job being active:

    /**
 * Delete logs on plugin page if cron is disabled.
 *
 * @since  1.0.0
 */
public function delete_logs_on_admin_page() {
	// Delete if we are on plugin page and cron is disabled.
	if (
		isset( $_GET['page'] ) &&
		'sg-security' === $_GET['page'] &&
		1 === Helper_Service::is_cron_disabled()
	) {
		$this->delete_old_activity_logs();
	}
}

    Better:

    /**
 * Delete logs on plugin page just to be sure
 *
 * @since  1.0.0
 */
public function delete_logs_on_admin_page() {
	// Delete if we are on plugin page
	if (
		isset( $_GET['page'] ) &&
		'sg-security' === $_GET['page']
	) {
		$this->delete_old_activity_logs();
	}
}
    • This reply was modified 1 year, 8 months ago by Rasso Hilber.
    • This reply was modified 1 year, 8 months ago by Rasso Hilber.
    Thread Starter Rasso Hilber

    (@nonverbla)

    As a temporary workaround, users can put this snippet in their functions.php:

    /**
 * Make sure the sg security cleanup job is being scheduled
 */
add_action('init', function () {
    if (!wp_next_scheduled('siteground_security_clear_logs_cron')) {
        wp_schedule_event(time(), 'daily', 'siteground_security_clear_logs_cron');
    }
});
    Plugin Support Gergana Petrova

    (@gpetrova)

    Hello @nonverbla,

    As you already noticed we do have a fallback function for removing logs, in case the native WordPress cron is disabled.

    It is triggered whenever you’re on the SiteGround Security’s page in the Dashboard and it clears logs, which are older than the days set in the Activity tab >> Log Settings.

    We thank you for the feedback on this and the suggestions made. It was already passed on to the plugin’s developers.

    Best Regards,
    Gergana Petrova

    Thread Starter Rasso Hilber

    (@nonverbla)

    Thanks @gpetrova for passing this along! I just noticed that I had something backwards in my first comment: Of course, the cleanup should also be scheduled if DISABLE_WP_CRON was set to true(I wrote false above). In fact, it should simply ignore the DISABLE_WP_CRON constant altogether.

Viewing 4 replies - 1 through 4 (of 4 total)
  • The topic ‘is_cron_disabled doesn’t make sense’ is closed to new replies.