Merge pull request #1990 from Automattic/add/validate_content_lock

Adds validate content lock
Automattic · Mar 18, 2021 · 957b95f · 957b95f
2 parents 2a90db3 + e3e4c85
commit 957b95f
Show file tree

Hide file tree

Showing 3 changed files with 94 additions and 12 deletions.
diff --git a/search/includes/classes/class-health.php b/search/includes/classes/class-health.php
@@ -12,6 +12,8 @@
 class Health {
 	const CONTENT_VALIDATION_BATCH_SIZE    = 500;
 	const CONTENT_VALIDATION_MAX_DIFF_SIZE = 1000;
+	const CONTENT_VALIDATION_LOCK_NAME = 'vip_search_content_validation_lock';
+	const CONTENT_VALIDATION_LOCK_TIMEOUT = 900; // 15 min
 	const DOCUMENT_IGNORED_KEYS            = array(
 		// This field is proving problematic to reliably diff due to differences in the filters
 		// that run during normal indexing and this validator
@@ -34,13 +36,14 @@ class Health {
 
 	/**
 	 * Instance of Search class
-	 * 
+	 *
 	 * Useful for overriding (dependency injection) for tests
 	 */
 	public $search;
 
 	public function __construct( \Automattic\VIP\Search\Search $search ) {
 		$this->search = $search;
+		$this->indexables = \ElasticPress\Indexables::factory();
 	}
 
 	/**
@@ -253,7 +256,12 @@ public static function validate_index_posts_count( $options = array() ) {
 	 *
 	 * @return array Array containing counts and ids of posts with inconsistent content
 	 */
-	public static function validate_index_posts_content( $start_post_id, $last_post_id, $batch_size, $max_diff_size, $silent, $inspect, $do_not_heal ) {
+	public function validate_index_posts_content( $start_post_id, $last_post_id, $batch_size, $max_diff_size, $silent, $inspect, $do_not_heal, $force_parallel_execution = false ) {
+		$process_parallel_execution_lock = ! $force_parallel_execution;
+		if ( $process_parallel_execution_lock && $this->is_validate_content_ongoing() ) {
+			return new WP_Error( 'content_validation_already_ongoing', 'Content validation is already ongoing' );
+		}
+
 		// If batch size value NOT a numeric value over 0 but less than or equal to PHP_INT_MAX, reset to default
 		//     Otherwise, turn it into an int
 		if ( ! is_numeric( $batch_size ) || 0 >= $batch_size || $batch_size > PHP_INT_MAX ) {
@@ -271,9 +279,9 @@ public static function validate_index_posts_content( $start_post_id, $last_post_
 		}
 
 		// Get indexable objects
-		$indexable = Indexables::factory()->get( 'post' );
+		$indexable = $this->indexables->get( 'post' );
 
-		// Indexables::factory()->get() returns boolean|array
+		// $this->indexables->get() returns boolean|array
 		// False is returned in case of error
 		if ( ! $indexable ) {
 			return new WP_Error( 'es_posts_query_error', 'Failure retrieving post indexable #vip-search' );
@@ -296,6 +304,10 @@ public static function validate_index_posts_content( $start_post_id, $last_post_
 		}
 
 		do {
+			if ( $process_parallel_execution_lock ) {
+				$this->set_validate_content_lock();
+			}
+
 			$next_batch_post_id = $start_post_id + $batch_size;
 
 			if ( $last_post_id < $next_batch_post_id ) {
@@ -306,7 +318,7 @@ public static function validate_index_posts_content( $start_post_id, $last_post_
 				echo sprintf( 'Validating posts %d - %d', $start_post_id, $next_batch_post_id - 1 ); // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped
 			}
 
-			$result = self::validate_index_posts_content_batch( $indexable, $start_post_id, $next_batch_post_id, $inspect );
+			$result = $this->validate_index_posts_content_batch( $indexable, $start_post_id, $next_batch_post_id, $inspect );
 
 			if ( is_wp_error( $result ) ) {
 				$result['errors'] = array( sprintf( 'batch %d - %d (entity: %s) error: %s', $start_post_id, $next_batch_post_id - 1, $indexable->slug, $result->get_error_message() ) );
@@ -324,6 +336,10 @@ public static function validate_index_posts_content( $start_post_id, $last_post_
 
 				$error->add_data( $results, 'diff' );
 
+				if ( $process_parallel_execution_lock ) {
+					$this->remove_validate_content_lock();
+				}
+
 				return $error;
 			}
 
@@ -343,10 +359,28 @@ public static function validate_index_posts_content( $start_post_id, $last_post_
 			}
 		} while ( $start_post_id <= $last_post_id );
 
+		if ( $process_parallel_execution_lock ) {
+			$this->remove_validate_content_lock();
+		}
+
 		return $results;
 	}
 
-	public static function validate_index_posts_content_batch( $indexable, $start_post_id, $next_batch_post_id, $inspect ) {
+	public function is_validate_content_ongoing(): bool {
+		$is_locked = get_transient( self::CONTENT_VALIDATION_LOCK_NAME, false );
+
+		return (bool) $is_locked;
+	}
+
+	public function set_validate_content_lock() {
+		set_transient( self::CONTENT_VALIDATION_LOCK_NAME, true, self::CONTENT_VALIDATION_LOCK_TIMEOUT );
+	}
+
+	public function remove_validate_content_lock() {
+		delete_transient( self::CONTENT_VALIDATION_LOCK_NAME );
+	}
+
+	public function validate_index_posts_content_batch( $indexable, $start_post_id, $next_batch_post_id, $inspect ) {
 		global $wpdb;
 
 		$rows = $wpdb->get_results( $wpdb->prepare( "SELECT ID, post_type, post_status FROM $wpdb->posts WHERE ID >= %d AND ID < %d", $start_post_id, $next_batch_post_id ) );
@@ -604,10 +638,10 @@ public function get_index_settings_health_for_all_indexables() {
 
 			if ( is_wp_error( $diff ) ) {
 				$unhealthy[ $indexable->slug ] = $diff;
-				
+
 				continue;
 			}
-			
+
 			if ( empty( $diff ) ) {
 				continue;
 			}

diff --git a/search/includes/classes/commands/class-healthcommand.php b/search/includes/classes/commands/class-healthcommand.php
@@ -262,20 +262,26 @@ private function render_results( array $results ) {
 	 * [--silent]
 	 * : Optional silences all non-error output except for the final results
 	 *
+	 * [--force-parallel-execution]
+	 * : Optional Force execution even if the process is already ongoing
+	 *
 	 * ## EXAMPLES
 	 *     wp vip-search health validate-contents
 	 *
 	 * @subcommand validate-contents
 	 */
 	public function validate_contents( $args, $assoc_args ) {
-		$results = \Automattic\VIP\Search\Health::validate_index_posts_content(
+		$health = new \Automattic\VIP\Search\Health( \Automattic\VIP\Search\Search::instance() );
+
+		$results = $health->validate_index_posts_content(
 			$assoc_args['start_post_id'] ?? 1,
 			$assoc_args['last_post_id'] ?? null,
 			$assoc_args['batch_size'] ?? null,
 			$assoc_args['max_diff_size'] ?? null,
 			isset( $assoc_args['silent'] ),
 			isset( $assoc_args['inspect'] ),
-			isset( $assoc_args['do-not-heal'] )
+			isset( $assoc_args['do-not-heal'] ),
+			isset( $assoc_args['force-parallel-execution'] )
 		);
 
 		if ( is_wp_error( $results ) ) {
@@ -285,7 +291,11 @@ public function validate_contents( $args, $assoc_args ) {
 				$this->render_contents_diff( $diff, $assoc_args['format'], $assoc_args['max_diff_size'] );
 			}
 
-			WP_CLI::error( $results->get_error_message() );
+			$message = $results->get_error_message();
+			if ( 'content_validation_already_ongoing' === $results->get_error_code() ) {
+				$message .= "\n\nYou can use --force-parallel-execution to run the command even with the lock in place";
+			}
+			WP_CLI::error( $message );
 		}
 
 		if ( empty( $results ) ) {

diff --git a/tests/search/includes/classes/test-class-health.php b/tests/search/includes/classes/test-class-health.php
@@ -442,6 +442,44 @@ public function test_validate_index_entity_count__skipping_non_initialized_index
 		$this->assertEquals( $result, $expected_result );
 	}
 
+	public function test_validate_index_posts_content__ongoing_results_in_error() {
+		$patrtially_mocked_health = $this->getMockBuilder( \Automattic\VIP\Search\Health::class )
+			->setMethods( [ 'is_validate_content_ongoing' ] )
+			->disableOriginalConstructor()
+			->getMock();
+
+		$patrtially_mocked_health->method( 'is_validate_content_ongoing' )
+			->willReturn( true );
+
+		$result = $patrtially_mocked_health->validate_index_posts_content( 1, null, null, null, false, false, false );
+
+		$this->assertTrue( is_wp_error( $result ) );
+	}
+
+	public function test_validate_index_posts_content__should_set_and_clear_lock() {
+		$patrtially_mocked_health = $this->getMockBuilder( \Automattic\VIP\Search\Health::class )
+			->setMethods( [ 'set_validate_content_lock', 'remove_validate_content_lock', 'validate_index_posts_content_batch' ] )
+			->disableOriginalConstructor()
+			->getMock();
+		$patrtially_mocked_health->method( 'validate_index_posts_content_batch' )->willReturn( [] );
+
+		$mocked_indexables = $this->getMockBuilder( \ElasticPress\Indexables::class )
+			->setMethods( [ 'get' ] )
+			->getMock();
+		$patrtially_mocked_health->indexables = $mocked_indexables;
+
+		$mocked_indexable = $this->getMockBuilder( \ElasticPress\Indexable::class )
+			->setMethods( [ 'query_db', 'prepare_document', 'put_mapping', 'build_mapping', 'build_settings' ] )
+			->getMock();
+
+		$mocked_indexables->method( 'get' )->willReturn( $mocked_indexable );
+
+		$patrtially_mocked_health->expects( $this->once() )->method( 'set_validate_content_lock' );
+		$patrtially_mocked_health->expects( $this->once() )->method( 'remove_validate_content_lock' );
+
+		$patrtially_mocked_health->validate_index_posts_content( 1, null, null, null, false, false, false );
+	}
+
 	public function get_index_settings_diff_for_indexable_data() {
 		return array(
 			// No diff expected, empty arrays
@@ -679,7 +717,7 @@ public function test_heal_index_settings_for_indexable( $desired_settings, $opti
 		$mocked_indexable->expects( $this->once() )
 			->method( 'update_index_settings' )
 			->with( $expected_updated_settings );
-	
+
 		$result = $health->heal_index_settings_for_indexable( $mocked_indexable, $options );
 
 		$expected_result = array(