diff --git a/cli/class-tmdo-cli.php b/cli/class-tmdo-cli.php index d87c79c..b44bd5b 100644 --- a/cli/class-tmdo-cli.php +++ b/cli/class-tmdo-cli.php @@ -142,6 +142,10 @@ class TMDO_CLI { foreach ( $violations as $entity_type => $integrations ) { WP_CLI::warning( " [GUARD] {$entity_type} is aeav_only while " . implode( ', ', $integrations ) . " reads native meta — write flat back with `wp tmdo bridge-restore-native {$entity_type} --demote`." ); } + $pending = TMDO_Schema_Manager::pending_indexes(); + if ( $pending ) { + WP_CLI::warning( ' [INDEX] declared indexes not added yet: ' . implode( ', ', $pending ) . ' — still missing after this run: another process is adding them, or the ALTER failed (see the `schema.index_failed` log).' ); + } $status = get_option( TMDO_Drift_Check::OPTION, array() ); $status = is_array( $status ) ? $status : array(); diff --git a/includes/class-tmdo-core.php b/includes/class-tmdo-core.php index fa8fb2e..3a116bb 100644 --- a/includes/class-tmdo-core.php +++ b/includes/class-tmdo-core.php @@ -306,6 +306,9 @@ class TMDO_Core { add_action( 'add_option_' . TMDO_Hook_Bus_Bridge::OPTION, array( 'TMDO_Drift_Check', 'on_hook_bus_added' ), 10, 2 ); add_action( TMDO_Drift_Check::RESYNC_HOOK, array( 'TMDO_Drift_Check', 'run_scheduled_fix' ) ); + // 1.3.0: declared indexes a visitor request left for WP-Cron. + add_action( TMDO_Schema_Manager::INDEXES_HOOK, array( 'TMDO_Schema_Manager', 'ensure_pending_indexes' ) ); + // v2.3.0 M6: daily snapshot prune cron (paired with health check). add_action( 'wpdo_snapshot_prune_daily', array( 'TMDO_Snapshot_Pruner', 'cron_run' ) ); diff --git a/includes/class-tmdo-installer.php b/includes/class-tmdo-installer.php index 7dd5fe8..da1244a 100644 --- a/includes/class-tmdo-installer.php +++ b/includes/class-tmdo-installer.php @@ -789,6 +789,7 @@ class TMDO_Installer { wp_clear_scheduled_hook( 'wpdo_post_shadow_verify' ); wp_clear_scheduled_hook( 'wpdo_term_comment_shadow_verify' ); wp_clear_scheduled_hook( 'tmdo_drift_resync' ); + wp_clear_scheduled_hook( 'tmdo_ensure_group_indexes' ); wp_clear_scheduled_hook( 'wpdo_post_stress_test_batch' ); wp_clear_scheduled_hook( 'wpdo_user_stress_test_batch' ); wp_clear_scheduled_hook( 'wpdo_term_stress_test_batch' ); @@ -993,6 +994,7 @@ class TMDO_Installer { 'wpdo_post_shadow_verify', 'wpdo_term_comment_shadow_verify', 'tmdo_drift_resync', + 'tmdo_ensure_group_indexes', 'wpdo_post_stress_test_batch', 'wpdo_user_stress_test_batch', 'wpdo_term_stress_test_batch', diff --git a/includes/engine/class-tmdo-schema-manager.php b/includes/engine/class-tmdo-schema-manager.php index 448c21e..88ee68b 100644 --- a/includes/engine/class-tmdo-schema-manager.php +++ b/includes/engine/class-tmdo-schema-manager.php @@ -23,6 +23,9 @@ final class TMDO_Schema_Manager { /** Option: "entity/group" → md5 of the composite index definition already applied. */ public const INDEXES_OPTION = 'tmdo_entity_indexes'; + /** Single cron event that adds declared indexes a visitor request had to leave out. */ + public const INDEXES_HOOK = 'tmdo_ensure_group_indexes'; + /** Request-scoped cache for table_exists() — prevents repeated SHOW TABLES per request. */ private static array $table_exists_cache = array(); @@ -144,6 +147,11 @@ final class TMDO_Schema_Manager { } } + // A new table gets its declared composite indexes while it is empty. + foreach ( TMDO_Entity_Registry::get_group_indexes( $entity_type, $group_name ) as $name => $keys ) { + $sql_indexes[] = 'KEY `cidx_' . self::sanitize_column_name( (string) $name ) . '` (' . implode( ', ', array_map( static fn( $key ) => '`' . self::sanitize_column_name( $key ) . '`', $keys ) ) . ')'; + } + $columns_sql = implode( ",\n ", $sql_columns ); $indexes_sql = implode( ",\n ", $sql_indexes ); @@ -176,6 +184,12 @@ final class TMDO_Schema_Manager { * applied definition is remembered per group (one autoloaded option), so * requests only run SHOW INDEX / ALTER when the declaration changes. * + * On a large table the ALTER takes seconds, so it runs only from WP-CLI or + * WP-Cron (ddl_allowed(); other requests schedule INDEXES_HOOK) and in one + * process at a time (MySQL GET_LOCK on index_lock_name()). Until every + * ALTER succeeds the group stays in pending_indexes() and the table works + * without the index. + * * @param string $entity_type Entity type. * @param string $group_name Group name. * @return void @@ -193,23 +207,112 @@ final class TMDO_Schema_Manager { if ( ( $applied[ $id ] ?? '' ) === $signature ) { return; } + if ( ! self::ddl_allowed() ) { + if ( ! wp_next_scheduled( self::INDEXES_HOOK ) ) { + wp_schedule_single_event( time(), self::INDEXES_HOOK ); + } + return; + } $table = self::get_table_name( $entity_type, $group_name ); if ( ! self::table_exists( $table ) ) { return; } - $existing = array_unique( array_column( (array) $wpdb->get_results( "SHOW INDEX FROM `{$table}`", ARRAY_A ), 'Key_name' ) ); - foreach ( $declared as $name => $keys ) { - $index = 'cidx_' . self::sanitize_column_name( $name ); - if ( in_array( $index, $existing, true ) ) { - continue; - } - $columns = implode( ', ', array_map( static fn( $key ) => '`' . self::sanitize_column_name( $key ) . '`', $keys ) ); - $wpdb->query( "ALTER TABLE `{$table}` ADD INDEX `{$index}` ({$columns})" ); + // add_option() is INSERT ... ON DUPLICATE KEY UPDATE and would let two + // processes both "acquire" an option lock; GET_LOCK is atomic and is + // released by MySQL if this process dies. + $lock = self::index_lock_name(); + if ( '1' !== (string) $wpdb->get_var( $wpdb->prepare( 'SELECT GET_LOCK(MD5(CONCAT(DATABASE(), %s)), 0)', $lock ) ) ) { + return; // Another process is adding indexes. } + try { + // SHOW INDEX (not the option, cached for this request) tells what another process already added. + $existing = array_unique( array_column( (array) $wpdb->get_results( "SHOW INDEX FROM `{$table}`", ARRAY_A ), 'Key_name' ) ); + $failed = false; + foreach ( $declared as $name => $keys ) { + $index = 'cidx_' . self::sanitize_column_name( $name ); + if ( in_array( $index, $existing, true ) ) { + continue; + } + $columns = implode( ', ', array_map( static fn( $key ) => '`' . self::sanitize_column_name( $key ) . '`', $keys ) ); + if ( false === $wpdb->query( "ALTER TABLE `{$table}` ADD INDEX `{$index}` ({$columns})" ) ) { + $failed = true; + TMDO_Logger::warning( + 'schema.index_failed', + array( + 'table' => $table, + 'index' => $index, + 'error' => $wpdb->last_error, + ) + ); + } + } - $applied[ $id ] = $signature; - update_option( self::INDEXES_OPTION, $applied, true ); + if ( ! $failed ) { + $applied[ $id ] = $signature; + update_option( self::INDEXES_OPTION, $applied, true ); + } + } finally { + $wpdb->query( $wpdb->prepare( 'SELECT RELEASE_LOCK(MD5(CONCAT(DATABASE(), %s)))', $lock ) ); + } + } + + /** + * INDEXES_HOOK handler: add the indexes a visitor request could not. + * + * @return void + */ + public static function ensure_pending_indexes(): void { + foreach ( self::pending_indexes() as $id ) { + list( $entity_type, $group_name ) = explode( '/', $id, 2 ); + self::ensure_group_indexes( $entity_type, $group_name ); + } + } + + /** + * Key of the MySQL lock held while indexes are added. Lock names are + * server-wide, so SQL hashes it with DATABASE() to keep sites apart. + * + * @return string + */ + public static function index_lock_name(): string { + global $wpdb; + return $wpdb->prefix . self::INDEXES_OPTION; + } + + /** + * Whether this request may alter flat tables: WP-CLI or WP-Cron only. + * Front-end, REST, admin-ajax, admin-post and even /wp-admin/ requests + * (init runs before auth_redirect()) can be a visitor's, who would wait + * for the ALTER. Filter: `tmdo_schema_ddl_allowed`. + * + * @return bool + */ + public static function ddl_allowed(): bool { + $allowed = ( defined( 'WP_CLI' ) && WP_CLI ) + || ( function_exists( 'wp_doing_cron' ) && wp_doing_cron() ); + return (bool) apply_filters( 'tmdo_schema_ddl_allowed', $allowed ); + } + + /** + * Registered groups whose declared indexes are not on their table yet. + * + * @return string[] "entity/group" + */ + public static function pending_indexes(): array { + $applied = get_option( self::INDEXES_OPTION, array() ); + $applied = is_array( $applied ) ? $applied : array(); + $pending = array(); + foreach ( TMDO_Entity_Registry::VALID_ENTITY_TYPES as $entity_type ) { + foreach ( TMDO_Entity_Registry::get_groups_for_type( $entity_type ) as $group_name ) { + $declared = TMDO_Entity_Registry::get_group_indexes( $entity_type, $group_name ); + if ( $declared && ( $applied[ "{$entity_type}/{$group_name}" ] ?? '' ) !== md5( (string) wp_json_encode( $declared ) ) + && self::table_exists( self::get_table_name( $entity_type, $group_name ) ) ) { + $pending[] = "{$entity_type}/{$group_name}"; + } + } + } + return $pending; } /** diff --git a/tests/integration/GroupCompositeIndexTest.php b/tests/integration/GroupCompositeIndexTest.php index 4e4d4f5..e8d6e3e 100644 --- a/tests/integration/GroupCompositeIndexTest.php +++ b/tests/integration/GroupCompositeIndexTest.php @@ -31,6 +31,10 @@ class GroupCompositeIndexTest extends TestCase { $wpdb->query( 'CREATE TABLE `' . self::TABLE . '` ( id BIGINT NOT NULL AUTO_INCREMENT, post_id BIGINT NOT NULL, ci_flag BIGINT(20) DEFAULT NULL, ci_price DECIMAL(18,6) DEFAULT NULL, PRIMARY KEY (id), UNIQUE KEY uk_entity (post_id) ) ENGINE=InnoDB' ); $wpdb->query( 'INSERT INTO `' . self::TABLE . '` (post_id, ci_flag, ci_price) VALUES (1, 1, 10), (2, 0, 20)' ); $GLOBALS['_wp_options'] = array(); + // Index DDL runs from WP-CLI / WP-Cron only (2.3 PR-090a); these tests act as one. + $GLOBALS['_wp_filter_callbacks']['tmdo_schema_ddl_allowed'] = array(); + add_filter( 'tmdo_schema_ddl_allowed', static fn() => true ); + $GLOBALS['_wp_scheduled_single'] = array(); $ref = new ReflectionProperty( WPDO_Schema_Manager::class, 'table_exists_cache' ); $ref->setAccessible( true ); @@ -53,6 +57,81 @@ class GroupCompositeIndexTest extends TestCase { global $wpdb; $wpdb->query( 'DROP TABLE IF EXISTS `' . self::TABLE . '`' ); WPDO_Entity_Registry::init(); + $GLOBALS['_wp_filter_callbacks']['tmdo_schema_ddl_allowed'] = array(); + unset( $GLOBALS['_wp_is_admin'], $GLOBALS['_wp_scheduled_single'] ); + } + + public function test_a_web_request_adds_no_index_and_leaves_it_to_wp_cron(): void { + $GLOBALS['_wp_filter_callbacks']['tmdo_schema_ddl_allowed'] = array(); + foreach ( array( false, true ) as $is_admin ) { // /wp-admin/ runs init before auth_redirect(). + $GLOBALS['_wp_is_admin'] = $is_admin; + $this->assertFalse( TMDO_Schema_Manager::ddl_allowed() ); + + TMDO_Schema_Manager::ensure_group_indexes( 'post', 'ci_core' ); + + $this->assertArrayNotHasKey( 'cidx_flag_price', $this->indexes(), 'no ALTER in a web request' ); + } + $this->assertSame( array( 'post/ci_core' ), TMDO_Schema_Manager::pending_indexes() ); + $this->assertContains( TMDO_Schema_Manager::INDEXES_HOOK, $GLOBALS['_wp_scheduled_single'] ); + + add_filter( 'tmdo_schema_ddl_allowed', static fn() => true ); // The cron run. + TMDO_Schema_Manager::ensure_pending_indexes(); + + $this->assertArrayHasKey( 'cidx_flag_price', $this->indexes() ); + $this->assertSame( array(), TMDO_Schema_Manager::pending_indexes() ); + } + + public function test_the_ddl_decision_can_be_overridden(): void { + add_filter( 'tmdo_schema_ddl_allowed', static fn() => false ); + + TMDO_Schema_Manager::ensure_group_indexes( 'post', 'ci_core' ); + + $this->assertArrayNotHasKey( 'cidx_flag_price', $this->indexes() ); + } + + public function test_a_failed_alter_is_not_recorded_as_applied(): void { + global $wpdb; + $wpdb->query( 'ALTER TABLE `' . self::TABLE . '` ADD COLUMN ci_note TEXT DEFAULT NULL' ); + WPDO_Entity_Registry::init(); + WPDO_Entity_Registry::register_adapter( 'post', new WPDO_Adapter_Post() ); + WPDO_Entity_Registry::register_group( + 'post', + 'ci_core', + array( + array( 'key' => 'ci_flag', 'type' => 'integer' ), + array( 'key' => 'ci_note', 'type' => 'textarea' ), + ), + array( 'indexes' => array( 'flag_note' => array( 'ci_flag', 'ci_note' ) ) ) // TEXT without a prefix length: MySQL 1170. + ); + + TMDO_Schema_Manager::ensure_group_indexes( 'post', 'ci_core' ); + + $this->assertArrayNotHasKey( 'cidx_flag_note', $this->indexes() ); + $this->assertFalse( get_option( TMDO_Schema_Manager::INDEXES_OPTION ), 'not recorded, so it is retried' ); + $this->assertSame( array( 'post/ci_core' ), TMDO_Schema_Manager::pending_indexes() ); + } + + public function test_a_lock_held_by_another_process_skips_the_ddl_until_it_is_released(): void { + $other = new mysqli( + getenv( 'TMDO_TEST_DB_HOST' ) ?: '127.0.0.1', + getenv( 'TMDO_TEST_DB_USER' ) ?: 'dbo', + (string) getenv( 'TMDO_TEST_DB_PASS' ), + getenv( 'TMDO_TEST_DB_NAME' ) ?: 'wp_wpdo_test' + ); + $name = $other->real_escape_string( TMDO_Schema_Manager::index_lock_name() ); + $this->assertSame( '1', (string) $other->query( "SELECT GET_LOCK(MD5(CONCAT(DATABASE(), '{$name}')), 0)" )->fetch_row()[0] ); + + TMDO_Schema_Manager::ensure_group_indexes( 'post', 'ci_core' ); + $this->assertArrayNotHasKey( 'cidx_flag_price', $this->indexes(), 'another process holds the lock' ); + + $other->query( "SELECT RELEASE_LOCK(MD5(CONCAT(DATABASE(), '{$name}')))" ); + $other->close(); + TMDO_Schema_Manager::ensure_group_indexes( 'post', 'ci_core' ); + + $this->assertArrayHasKey( 'cidx_flag_price', $this->indexes() ); + $this->assertSame( array(), TMDO_Schema_Manager::pending_indexes() ); + global $wpdb; + $this->assertSame( '1', (string) $wpdb->get_var( $wpdb->prepare( 'SELECT IS_FREE_LOCK(MD5(CONCAT(DATABASE(), %s)))', TMDO_Schema_Manager::index_lock_name() ) ), 'lock released' ); } /** @return array index name → ordered columns */ diff --git a/tests/integration/bootstrap.php b/tests/integration/bootstrap.php index 6d3dad8..1c9cbcc 100644 --- a/tests/integration/bootstrap.php +++ b/tests/integration/bootstrap.php @@ -168,7 +168,12 @@ $wpdb = new class( $_mysqli ) { fn( $v ) => $v === null ? 'NULL' : "'" . $this->db->real_escape_string( (string) $v ) . "'", array_values( $data ) ) ); - $ok = $this->db->query( "INSERT INTO `{$table}` ({$cols}) VALUES ({$vals})" ); + try { + $ok = $this->db->query( "INSERT INTO `{$table}` ({$cols}) VALUES ({$vals})" ); + } catch ( mysqli_sql_exception $e ) { // Like wpdb::insert(): an SQL error returns false. + $this->last_error = $e->getMessage(); + return false; + } if ( $ok ) { $this->insert_id = (int) $this->db->insert_id; return $this->insert_id; } $this->last_error = (string) $this->db->error; return false; @@ -200,7 +205,12 @@ $wpdb = new class( $_mysqli ) { public function flush(): void {} public function query( string $sql ): int|bool { - $result = $this->db->query( $sql ); + try { + $result = $this->db->query( $sql ); + } catch ( mysqli_sql_exception $e ) { // Like wpdb::query(): an SQL error returns false. + $this->last_error = $e->getMessage(); + return false; + } if ( $result instanceof mysqli_result ) { $result->free(); return true; } if ( false === $result ) { return false; } return $this->db->affected_rows; @@ -291,7 +301,7 @@ if ( ! function_exists( 'is_singular' ) ) { function is_singular( $t = '' ): if ( ! function_exists( 'wp_doing_ajax' ) ) { function wp_doing_ajax(): bool { return false; } } if ( ! function_exists( 'wp_next_scheduled' ) ) { function wp_next_scheduled( string $hook ): int|false { return false; } } if ( ! function_exists( 'wp_schedule_event' ) ) { function wp_schedule_event( int $t, string $r, string $h ): bool { return true; } } -if ( ! function_exists( 'wp_schedule_single_event' ) ) { function wp_schedule_single_event( int $ts, string $hook ): bool { return true; } } +if ( ! function_exists( 'wp_schedule_single_event' ) ) { function wp_schedule_single_event( int $ts, string $hook ): bool { $GLOBALS['_wp_scheduled_single'][] = $hook; return true; } } if ( ! function_exists( 'wp_clear_scheduled_hook' ) ) { function wp_clear_scheduled_hook( string $hook ): int|false { return 0; } } if ( ! function_exists( 'sanitize_text_field' ) ) { function sanitize_text_field( string $s ): string { return trim( strip_tags( $s ) ); } } if ( ! function_exists( '__' ) ) { function __( string $text, string $domain = 'default' ): string { return $text; } }