Harden sync_all() for future live-site use: per-row savepoints, scoped cache invalidation

Ahead of running this against a live site (not just staging bulk-imports),
addressed the two risks that only matter under concurrent traffic:

- One bad product no longer wastes a whole batch: each product save is
  wrapped in its own SAVEPOINT within the batch transaction, so a failure
  rolls back just that row (counted in the new failed[] result, surfaced
  via WP_CLI::warning like import-gutenberg already does) instead of
  discarding up to 200 already-good rows. Verified SAVEPOINT/ROLLBACK TO
  SAVEPOINT actually works on this MariaDB instance via a direct $wpdb test
  before relying on it.

- Cache invalidation suspension is now scoped per-batch instead of the
  whole run, with clean_post_cache() called explicitly per product once
  each batch commits, replacing the single end-of-run wp_cache_flush().
  This site runs a shared Redis object cache across PHP-FPM workers, so a
  whole-run suspension + full flush would let a concurrent visitor see
  stale cached product data for the run's duration, and the flush itself
  would evict sessions and everything else site-wide. Scoping bounds
  staleness to one batch and targets only the products actually touched.

Also moved term resolution (term_exists()/wp_insert_term()) out of the
per-product savepoint entirely: warm_term_cache() now resolves every
distinct subject for a batch before its transaction opens, so a term
created for one product can never get silently rolled back by a *different*
product's savepoint failure while a stale term_id sits cached in memory
and gets attached to a later product (an orphaned term_relationships row,
checked for directly post-fix: 0 found).

Verified after: 2000-item sync still ~90s (savepoints add no measurable
overhead), 0 lookup-table mismatches, 0 term-count mismatches, 0 orphaned
term_relationships, idempotent re-run with 0 duplicates.

wp-content/plugins/bookstore-core/includes/Cli/Commands.php

@@ -96,7 +96,17 @@ class Commands
         $result = ProductSync::sync_all(200, function (int $processed, int $created, int $updated) {
             \WP_CLI::log("  processed {$processed} (created {$created}, updated {$updated})...");
         });
-        \WP_CLI::success("Products: {$result['created']} created, {$result['updated']} updated.");
+
+        foreach ($result['failed'] as $failure) {
+            \WP_CLI::warning($failure);
+        }
+
+        \WP_CLI::success(sprintf(
+            'Products: %d created, %d updated, %d failed.',
+            $result['created'],
+            $result['updated'],
+            count($result['failed'])
+        ));
     }
 
     /**

wp-content/plugins/bookstore-core/includes/Product/ProductSync.php

@@ -19,28 +19,53 @@ defined('ABSPATH') || exit;
  * 1. wp_defer_term_counting() — batches product_cat term-count recalculation
  *    to once at the end instead of once per wp_set_object_terms() call.
  * 2. wp_suspend_cache_invalidation() — skips clean_post_cache()'s cache
- *    clearing during the run. Verified this does NOT risk stale reads in
+ *    clearing while a batch's transaction is open. Verified this does NOT
- *    WooCommerce's own wc_product_meta_lookup sync: that sync reads back
+ *    risk stale reads in WooCommerce's own wc_product_meta_lookup sync: that
- *    ~14 postmeta values via get_post_meta() immediately after save(), but
+ *    sync reads back ~14 postmeta values via get_post_meta() immediately
- *    update_post_meta()/add_post_meta() write straight into the postmeta
+ *    after save(), but update_post_meta()/add_post_meta() write straight
- *    object cache on write (not merely invalidate-then-refetch), and this
+ *    into the postmeta object cache on write (not merely
- *    flag only affects clean_post_cache()'s handling of the *posts* cache
+ *    invalidate-then-refetch), and this flag only affects
- *    group — postmeta cache correctness is unaffected either way.
+ *    clean_post_cache()'s handling of the *posts* cache group — postmeta
- * 3. Static in-memory subject→term_id cache in sync_categories() — avoids
+ *    cache correctness is unaffected either way.
+ *
+ *    Scoped to one batch at a time (suspended/lifted around each batch,
+ *    not the whole run), with clean_post_cache() called explicitly per
+ *    product once lifted. This site runs a shared Redis object cache
+ *    (persistent across PHP-FPM workers, not per-request) — a concurrent
+ *    visitor loading a product page mid-batch could otherwise see a stale
+ *    cached price/title until invalidation resumes. Per-batch scoping
+ *    bounds that staleness window to one batch's duration instead of the
+ *    whole run, and targets just the touched products instead of a
+ *    whole-cache wp_cache_flush() (which would evict sessions and every
+ *    other cached object site-wide — a real concern once this runs
+ *    against a live site with real traffic, not just staging).
+ * 3. Static in-memory subject→term_id cache, warmed once per batch via
+ *    warm_term_cache() *before* the batch's transaction opens — avoids
  *    repeat term_exists()/wp_insert_term() round-trips for subjects that
  *    recur across many Works (common — PG subject strings repeat a lot).
- * 4. Each batch runs inside one explicit DB transaction. WordPress/WooCommerce
+ *    Deliberately resolved outside the transaction: term_exists()/
- *    writes run in MySQL autocommit by default, so every post/postmeta/term-
+ *    wp_insert_term() commit immediately in their own autocommit write, so
- *    relationship/wc_product_meta_lookup write in sync_one() is its own
+ *    a later SAVEPOINT rollback for some other product in the batch can
- *    implicit transaction — each one an InnoDB commit, each commit an fsync
+ *    never leave a cached term_id pointing at a term that got rolled back
+ *    out of the DB. sync_categories() itself is then a pure cache lookup —
+ *    no term creation happens inside the per-product savepoint at all.
+ * 4. Each batch runs inside one explicit DB transaction, with a SAVEPOINT
+ *    around each individual product. WordPress/WooCommerce writes run in
+ *    MySQL autocommit by default, so every post/postmeta/term-relationship/
+ *    wc_product_meta_lookup write in sync_one() is its own implicit
+ *    transaction — each one an InnoDB commit, each commit an fsync
  *    (confirmed: wc_product_meta_lookup sync is synchronous inside
  *    WC_Product::save(), not deferred via Action Scheduler). Wrapping a
  *    batch in one transaction turns ~batch_size fsyncs into one, which is
- *    where most of the win is on slow/networked storage. Caveat: this
+ *    where most of the win is on slow/networked storage. The per-product
- *    assumes nothing reads mid-batch data from a separate connection before
+ *    SAVEPOINT means one bad row rolls back to just before itself (counted
- *    COMMIT (true today — this site has no webhooks or other Action
+ *    in $failed, run continues) instead of discarding the rest of an
- *    Scheduler consumers hooked on product save; would need re-checking if
+ *    otherwise-good batch. Caveat: batching still assumes nothing reads
- *    that changes).
+ *    mid-batch data from a separate connection before COMMIT (true today —
+ *    this site has no webhooks or other Action Scheduler consumers hooked
+ *    on product save; would need re-checking if that changes). For a run
+ *    that has to share the DB with live traffic, pass a smaller
+ *    $batch_size — shorter transactions, shorter lock windows.
  */
 class ProductSync
 {
@@ -49,7 +74,7 @@ class ProductSync
 
     /**
      * @param callable|null $on_progress function(int $processed, int $created, int $updated): void
-     * @return array{created:int, updated:int}
+     * @return array{created:int, updated:int, failed:string[]}
      */
     public static function sync_all(int $batch_size = 200, ?callable $on_progress = null): array
     {
@@ -58,10 +83,10 @@ class ProductSync
         $created = 0;
         $updated = 0;
         $processed = 0;
+        $failed = [];
         $offset = 0;
 
         wp_defer_term_counting(true);
-        wp_suspend_cache_invalidation(true);
 
         try {
             while (true) {
@@ -70,18 +95,36 @@ class ProductSync
                     break;
                 }
 
+                $batch_product_ids = [];
+
+                self::warm_term_cache($works);
+
+                wp_suspend_cache_invalidation(true);
                 $wpdb->query('START TRANSACTION');
-                try {
+
-                    foreach ($works as $work) {
+                foreach ($works as $work) {
+                    $wpdb->query('SAVEPOINT sp_sync_one');
+                    try {
                         $is_new = empty($work->wc_product_id) || !wc_get_product((int) $work->wc_product_id);
-                        self::sync_one($work);
+                        $product_id = self::sync_one($work);
+                        $wpdb->query('RELEASE SAVEPOINT sp_sync_one');
+                        $batch_product_ids[] = $product_id;
                         $is_new ? $created++ : $updated++;
-                        $processed++;
+                    } catch (\Throwable $e) {
+                        $wpdb->query('ROLLBACK TO SAVEPOINT sp_sync_one');
+                        $failed[] = "work_id={$work->work_id} ({$work->title}): " . $e->getMessage();
                     }
-                    $wpdb->query('COMMIT');
+                    $processed++;
-                } catch (\Throwable $e) {
+                }
-                    $wpdb->query('ROLLBACK');
+
-                    throw $e;
+                $wpdb->query('COMMIT');
+                wp_suspend_cache_invalidation(false);
+
+                // Cache invalidation was suspended for the batch above —
+                // fire it now, scoped to just the products this batch
+                // actually touched, instead of a whole-cache flush.
+                foreach ($batch_product_ids as $product_id) {
+                    clean_post_cache($product_id);
                 }
 
                 if ($on_progress) {
@@ -90,15 +133,13 @@ class ProductSync
                 $offset += $batch_size;
             }
         } finally {
-            wp_suspend_cache_invalidation(false);
             wp_defer_term_counting(false);
-            wp_cache_flush();
         }
 
-        return ['created' => $created, 'updated' => $updated];
+        return ['created' => $created, 'updated' => $updated, 'failed' => $failed];
     }
 
-    private static function sync_one(object $work): void
+    private static function sync_one(object $work): int
     {
         $existing_id = (int) ($work->wc_product_id ?? 0);
         $product = $existing_id ? wc_get_product($existing_id) : false;
@@ -129,8 +170,46 @@ class ProductSync
         }
 
         self::sync_categories($product_id, $work->subjects ?? null);
+
+        return (int) $product_id;
     }
 
+    /**
+     * Resolves (creating if needed) a product_cat term for every distinct
+     * subject across this batch's Works, populating self::$term_cache.
+     * Runs before the batch's transaction opens — see class docblock §3
+     * for why that ordering matters.
+     *
+     * @param object[] $works
+     */
+    private static function warm_term_cache(array $works): void
+    {
+        $new_subjects = [];
+        foreach ($works as $work) {
+            $subjects = !empty($work->subjects) ? json_decode($work->subjects, true) : [];
+            if (!is_array($subjects)) {
+                continue;
+            }
+            foreach ($subjects as $subject) {
+                $subject = trim((string) $subject);
+                if ($subject !== '' && !isset(self::$term_cache[$subject])) {
+                    $new_subjects[$subject] = true;
+                }
+            }
+        }
+
+        foreach (array_keys($new_subjects) as $subject) {
+            $term = term_exists($subject, 'product_cat');
+            if (!$term) {
+                $term = wp_insert_term($subject, 'product_cat');
+            }
+            if (!is_wp_error($term) && isset($term['term_id'])) {
+                self::$term_cache[$subject] = (int) $term['term_id'];
+            }
+        }
+    }
+
+    /** Pure cache lookup — see warm_term_cache(), which populates it ahead of time. */
     private static function sync_categories(int $product_id, ?string $subjects_json): void
     {
         $subjects = $subjects_json ? json_decode($subjects_json, true) : [];
@@ -141,21 +220,8 @@ class ProductSync
         $term_ids = [];
         foreach ($subjects as $subject) {
             $subject = trim((string) $subject);
-            if ($subject === '') {
+            if ($subject !== '' && isset(self::$term_cache[$subject])) {
-                continue;
-            }
-            if (isset(self::$term_cache[$subject])) {
                 $term_ids[] = self::$term_cache[$subject];
-                continue;
-            }
-            $term = term_exists($subject, 'product_cat');
-            if (!$term) {
-                $term = wp_insert_term($subject, 'product_cat');
-            }
-            if (!is_wp_error($term) && isset($term['term_id'])) {
-                $term_id = (int) $term['term_id'];
-                self::$term_cache[$subject] = $term_id;
-                $term_ids[] = $term_id;
             }
         }