Skip to content

Commit

Permalink
Closes #6222: Revamp retry process rucss (#6256)
Browse files Browse the repository at this point in the history
Co-authored-by: Vasilis Manthos <vmanthos@gmail.com>
Co-authored-by: COQUARD Cyrille <coquardcyr@gmail.com>
  • Loading branch information
3 people authored Nov 23, 2023
1 parent 01309da commit da05112
Show file tree
Hide file tree
Showing 31 changed files with 1,192 additions and 116 deletions.
27 changes: 27 additions & 0 deletions inc/Engine/Common/Clock/ClockInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

namespace WP_Rocket\Engine\Common\Clock;

interface ClockInterface {
/**
* Retrieves the current time based on specified type.
*
* - The 'mysql' type will return the time in the format for MySQL DATETIME field.
* - The 'timestamp' or 'U' types will return the current timestamp or a sum of timestamp
* and timezone offset, depending on `$gmt`.
* - Other strings will be interpreted as PHP date formats (e.g. 'Y-m-d').
*
* If `$gmt` is a truthy value then both types will use GMT time, otherwise the
* output is adjusted with the GMT offset for the site.
*
* @since 1.0.0
* @since 5.3.0 Now returns an integer if `$type` is 'U'. Previously a string was returned.
*
* @param string $type Type of time to retrieve. Accepts 'mysql', 'timestamp', 'U',
* or PHP date format string (e.g. 'Y-m-d').
* @param int|bool $gmt Optional. Whether to use GMT timezone. Default false.
*
* @return int|string Integer if `$type` is 'timestamp' or 'U', string otherwise.
*/
public function current_time( string $type, $gmt = 0 );
}
35 changes: 35 additions & 0 deletions inc/Engine/Common/Clock/WPRClock.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php

namespace WP_Rocket\Engine\Common\Clock;

class WPRClock implements ClockInterface {

/**
* Retrieves the current time based on specified type.
*
* - The 'mysql' type will return the time in the format for MySQL DATETIME field.
* - The 'timestamp' or 'U' types will return the current timestamp or a sum of timestamp
* and timezone offset, depending on `$gmt`.
* - Other strings will be interpreted as PHP date formats (e.g. 'Y-m-d').
*
* If `$gmt` is a truthy value then both types will use GMT time, otherwise the
* output is adjusted with the GMT offset for the site.
*
* @since 1.0.0
* @since 5.3.0 Now returns an integer if `$type` is 'U'. Previously a string was returned.
*
* @param string $type Type of time to retrieve. Accepts 'mysql', 'timestamp', 'U',
* or PHP date format string (e.g. 'Y-m-d').
* @param int|bool $gmt Optional. Whether to use GMT timezone. Default false.
*
* @return int|string Integer if `$type` is 'timestamp' or 'U', string otherwise.
*/
public function current_time( string $type, $gmt = 0 ) {
$current_time = current_time( $type, $gmt );
$output = apply_filters( 'rocket_current_time', $current_time );
if ( ( is_string( $current_time ) && strtotime( $current_time ) ) || ( is_int( $current_time ) && $current_time >= 0 ) ) {
return $output;
}
return $current_time;
}
}
69 changes: 32 additions & 37 deletions inc/Engine/Optimization/RUCSS/Controller/UsedCSS.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
namespace WP_Rocket\Engine\Optimization\RUCSS\Controller;

use WP_Rocket\Admin\Options_Data;
use WP_Rocket\Engine\Common\Clock\WPRClock;
use WP_Rocket\Engine\Common\Context\ContextInterface;
use WP_Rocket\Engine\Common\Queue\QueueInterface;
use WP_Rocket\Engine\Optimization\CSSTrait;
Expand All @@ -12,6 +13,7 @@
use WP_Rocket\Engine\Optimization\RUCSS\Database\Queries\UsedCSS as UsedCSS_Query;
use WP_Rocket\Engine\Optimization\RUCSS\Frontend\APIClient;
use WP_Admin_Bar;
use WP_Rocket\Engine\Optimization\RUCSS\Strategy\Factory\StrategyFactory;
use WP_Rocket\Logger\LoggerAware;
use WP_Rocket\Logger\LoggerAwareInterface;

Expand Down Expand Up @@ -97,6 +99,20 @@ class UsedCSS implements LoggerAwareInterface {
*/
private $inline_content_exclusions = [];

/**
* Retry Strategy Factory
*
* @var StrategyFactory
*/
protected $strategy_factory;

/**
* Clock instance.
*
* @var WPRClock
*/
protected $wpr_clock;

/**
* Instantiate the class.
*
Expand All @@ -108,6 +124,8 @@ class UsedCSS implements LoggerAwareInterface {
* @param Filesystem $filesystem Filesystem instance.
* @param ContextInterface $context RUCSS context.
* @param ContextInterface $optimize_url_context RUCSS optimize url context.
* @param StrategyFactory $strategy_factory Strategy Factory used for RUCSS retry process.
* @param WPRClock $clock Clock object instance.
*/
public function __construct(
Options_Data $options,
Expand All @@ -117,7 +135,9 @@ public function __construct(
DataManager $data_manager,
Filesystem $filesystem,
ContextInterface $context,
ContextInterface $optimize_url_context
ContextInterface $optimize_url_context,
StrategyFactory $strategy_factory,
WPRClock $clock
) {
$this->options = $options;
$this->used_css_query = $used_css_query;
Expand All @@ -127,6 +147,8 @@ public function __construct(
$this->filesystem = $filesystem;
$this->context = $context;
$this->optimize_url_context = $optimize_url_context;
$this->strategy_factory = $strategy_factory;
$this->wpr_clock = $clock;
}

/**
Expand Down Expand Up @@ -500,12 +522,14 @@ public function process_pending_jobs() {
}

foreach ( $pending_jobs as $used_css_row ) {
$this->logger::debug( "RUCSS: Send the job for url {$used_css_row->url} to Async task to check its job status." );
$current_time = $this->wpr_clock->current_time( 'timestamp', true );
if ( strtotime( $used_css_row->next_retry_time ) < $current_time ) {
$this->logger::debug( "RUCSS: Send the job for url {$used_css_row->url} to Async task to check its job status." );

// Change status to in-progress.
$this->used_css_query->make_status_inprogress( (int) $used_css_row->id );

$this->queue->add_job_status_check_async( (int) $used_css_row->id );
// Change status to in-progress.
$this->used_css_query->make_status_inprogress( (int) $used_css_row->id );
$this->queue->add_job_status_check_async( (int) $used_css_row->id );
}
}
}

Expand All @@ -518,6 +542,7 @@ public function process_pending_jobs() {
*/
public function check_job_status( int $id ) {
$this->logger::debug( 'RUCSS: Start checking job status for row ID: ' . $id );

$row_details = $this->used_css_query->get_item( $id );
if ( ! $row_details ) {
$this->logger::debug( 'RUCSS: Row ID not found ', compact( 'id' ) );
Expand Down Expand Up @@ -550,39 +575,9 @@ public function check_job_status( int $id ) {

if (
200 !== (int) $job_details['code']
||
empty( $job_details['contents'] )
||
! isset( $job_details['contents']['shakedCSS'] )
) {
$this->logger::debug( 'RUCSS: Job status failed for url: ' . $row_details->url, $job_details );

// Failure, check the retries number.
if ( $row_details->retries >= 3 ) {
$this->logger::debug( 'RUCSS: Job failed 3 times for url: ' . $row_details->url );
/**
* Unlock preload URL.
*
* @param string $url URL to unlock
*/
do_action( 'rocket_preload_unlock_url', $row_details->url );

$this->used_css_query->make_status_failed( $id, strval( $job_details['code'] ), $job_details['message'] );

return;
}

// on timeout errors with code 408 create new job.
switch ( $job_details['code'] ) {
case 408:
$this->add_url_to_the_queue( $row_details->url, (bool) $row_details->is_mobile );
return;
}

// Increment the retries number with 1 , Change status to pending again and change job id on timeout.
$this->used_css_query->increment_retries( $id, (int) $job_details['code'], $job_details['message'] );

// @Todo: Maybe we can add this row to the async job to get the status before the next cron
$this->strategy_factory->manage( $row_details, $job_details );

return;
}
Expand Down
45 changes: 45 additions & 0 deletions inc/Engine/Optimization/RUCSS/Database/Queries/UsedCSS.php
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ public function get_pending_jobs( int $count = 100 ) {
'fields' => [
'id',
'url',
'next_retry_time',
],
'job_id__not_in' => [
'not_in' => '',
Expand Down Expand Up @@ -578,6 +579,50 @@ private function table_exists(): bool {
return $exists;
}

/**
* Update the error message.
*
* @param int $job_id Job ID.
* @param int $code Response code.
* @param string $message Response message.
* @param string $previous_message Previous saved message.
*
* @return bool
*/
public function update_message( int $job_id, int $code, string $message, string $previous_message = '' ): bool {
return $this->update_item(
$job_id,
[
'error_message' => $previous_message . ' - ' . current_time( 'mysql', true ) . " {$code}: {$message}",
]
);
}

/**
* Updates the next_retry_time field
*
* @param mixed $job_id the job id.
* @param string|int $next_retry_time timestamp or mysql format date.
*
* @return bool either it is saved or not.
*/
public function update_next_retry_time( $job_id, $next_retry_time ): bool {
if ( is_string( $next_retry_time ) && strtotime( $next_retry_time ) ) {
// If $next_retry_time is a valid date string, convert it to a timestamp.
$next_retry_time = strtotime( $next_retry_time );
} elseif ( ! is_numeric( $next_retry_time ) ) {
// If it's not numeric and not a valid date string, return false.
return false;
}

return $this->update_item(
$job_id,
[
'next_retry_time' => gmdate( 'Y-m-d H:i:s', $next_retry_time ),
]
);
}

/**
* Change the status to be pending.
*
Expand Down
36 changes: 22 additions & 14 deletions inc/Engine/Optimization/RUCSS/Database/Row/UsedCSS.php
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,13 @@ class UsedCSS extends Row {
*/
public $submitted_at;

/**
* Tells when the retry has to be processed
*
* @var int
*/
public $next_retry_time;

/**
* UsedCSS constructor.
*
Expand All @@ -122,19 +129,20 @@ public function __construct( $item ) {
parent::__construct( $item );

// Set the type of each column, and prepare.
$this->id = (int) $this->id;
$this->url = (string) $this->url;
$this->css = (string) $this->css;
$this->hash = (string) $this->hash;
$this->error_code = (string) $this->error_code;
$this->error_message = (string) $this->error_message;
$this->retries = (int) $this->retries;
$this->is_mobile = (bool) $this->is_mobile;
$this->job_id = (string) $this->job_id;
$this->queue_name = (string) $this->queue_name;
$this->status = (string) $this->status;
$this->modified = empty( $this->modified ) ? 0 : strtotime( $this->modified );
$this->last_accessed = empty( $this->last_accessed ) ? 0 : strtotime( $this->last_accessed );
$this->submitted_at = empty( $this->submitted_at ) ? 0 : strtotime( $this->submitted_at );
$this->id = (int) $this->id;
$this->url = (string) $this->url;
$this->css = (string) $this->css;
$this->hash = (string) $this->hash;
$this->error_code = (string) $this->error_code;
$this->error_message = (string) $this->error_message;
$this->retries = (int) $this->retries;
$this->is_mobile = (bool) $this->is_mobile;
$this->job_id = (string) $this->job_id;
$this->queue_name = (string) $this->queue_name;
$this->status = (string) $this->status;
$this->modified = empty( $this->modified ) ? 0 : strtotime( $this->modified );
$this->last_accessed = empty( $this->last_accessed ) ? 0 : strtotime( $this->last_accessed );
$this->submitted_at = empty( $this->submitted_at ) ? 0 : strtotime( $this->submitted_at );
$this->next_retry_time = empty( $this->next_retry_time ) ? 0 : strtotime( $this->next_retry_time );
}
}
10 changes: 10 additions & 0 deletions inc/Engine/Optimization/RUCSS/Database/Schemas/UsedCSS.php
Original file line number Diff line number Diff line change
Expand Up @@ -163,5 +163,15 @@ class UsedCSS extends Schema {
'date_query' => true,
'sortable' => true,
],

// NEXT_RETRY_TIME column.
[
'name' => 'next_retry_time',
'type' => 'timestamp',
'default' => '0000-00-00 00:00:00',
'created' => true,
'date_query' => true,
'sortable' => true,
],
];
}
51 changes: 35 additions & 16 deletions inc/Engine/Optimization/RUCSS/Database/Tables/UsedCSS.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class UsedCSS extends Table {
*
* @var int
*/
protected $version = 20231010;
protected $version = 20231031;

/**
* Key => value array of versions => methods.
Expand All @@ -42,6 +42,7 @@ class UsedCSS extends Table {
20220920 => 'make_status_column_index_instead_queue_name',
20221104 => 'add_error_columns',
20231010 => 'add_submitted_at_column',
20231031 => 'add_next_retry_time_column',
];

/**
Expand All @@ -60,21 +61,22 @@ public function __construct() {
*/
protected function set_schema() {
$this->schema = "
id bigint(20) unsigned NOT NULL AUTO_INCREMENT,
url varchar(2000) NOT NULL default '',
css longtext default NULL,
hash varchar(32) default '',
error_code varchar(32) NULL default NULL,
error_message longtext NULL default NULL,
unprocessedcss longtext NULL,
retries tinyint(1) NOT NULL default 1,
is_mobile tinyint(1) NOT NULL default 0,
job_id varchar(255) NOT NULL default '',
queue_name varchar(255) NOT NULL default '',
status varchar(255) NOT NULL default '',
modified timestamp NOT NULL default '0000-00-00 00:00:00',
last_accessed timestamp NOT NULL default '0000-00-00 00:00:00',
submitted_at timestamp NULL,
id bigint(20) unsigned NOT NULL AUTO_INCREMENT,
url varchar(2000) NOT NULL default '',
css longtext default NULL,
hash varchar(32) default '',
error_code varchar(32) NULL default NULL,
error_message longtext NULL default NULL,
unprocessedcss longtext NULL,
retries tinyint(1) NOT NULL default 1,
is_mobile tinyint(1) NOT NULL default 0,
job_id varchar(255) NOT NULL default '',
queue_name varchar(255) NOT NULL default '',
status varchar(255) NOT NULL default '',
modified timestamp NOT NULL default '0000-00-00 00:00:00',
last_accessed timestamp NOT NULL default '0000-00-00 00:00:00',
submitted_at timestamp NULL,
next_retry_time timestamp NOT NULL default '0000-00-00 00:00:00',
PRIMARY KEY (id),
KEY url (url(150), is_mobile),
KEY modified (modified),
Expand Down Expand Up @@ -342,4 +344,21 @@ protected function add_submitted_at_column() {

return $this->is_success( $created );
}

/**
* Adds the next_retry_time column
*
* @return bool
*/
protected function add_next_retry_time_column() {
$next_retry_time_exists = $this->column_exists( 'next_retry_time' );

$created = true;

if ( ! $next_retry_time_exists ) {
$created &= $this->get_db()->query( "ALTER TABLE `{$this->table_name}` ADD COLUMN next_retry_time timestamp NOT NULL default '0000-00-00 00:00:00' AFTER submitted_at" );
}

return $this->is_success( $created );
}
}
Loading

0 comments on commit da05112

Please sign in to comment.