[codex] Use writer-consistent reads after WordPress writes#30
Closed
alexstandiford wants to merge 1 commit into
Closed
[codex] Use writer-consistent reads after WordPress writes#30alexstandiford wants to merge 1 commit into
alexstandiford wants to merge 1 commit into
Conversation
6a6fd39 to
7507701
Compare
7507701 to
23dc3df
Compare
23dc3df to
7a5157a
Compare
Contributor
Author
|
Withdrawing this PR after further reproduction work. We provisioned a Cloudways Autonomous trial app, imported Darren's database/files/plugins/theme, matched PHP 8.2.9, preserved the Cloudways Redis/Object Cache Pro layer, and ran the Stripe/WooCommerce paths again. The original failure did not reproduce:
Given that evidence, this writer-consistent read change is no longer justified for this incident. Closing rather than merging. |
alexstandiford
added a commit
to phpnomad/db
that referenced
this pull request
Apr 30, 2026
…#25) The post-insert read in WithDatastoreHandlerMethods::create() races read-replicas on hosts with write/read-split routers (ProxySQL, MaxScale, RDS Proxy, Aurora, Cloudways Autonomous, etc.). The router routes the autocommit INSERT to master and the immediately-following autocommit SELECT to a replica that hasn't yet received the write, producing RecordNotFoundException after a successful insert. Reproduced live on a customer environment: 14/2200 sequential creates failed (~0.6%), all on the same client connection id, all with LAST_INSERT_ID() returning the right id, all recovering within 10-50ms. Wrapping in an explicit transaction eliminated the failure entirely (because routers pin transactional clients to one backend), but adding transactions to every create has its own trade-offs - and the underlying problem shows up on any backend with eventual consistency (Postgres+pgpool, MongoDB read preferences, Cassandra, search indexes, REST APIs behind CDNs). The cleanest fix is to remove the post-insert read entirely. We already have the data the caller passed in, plus the new identity from the insert response. Construct the model from those in PHP and skip the round-trip. What changed: - Column gains an optional withPhpDefault(callable) setter. The callable is invoked at create time for any column the caller didn't provide, so the in-memory model carries the same value the DB would have defaulted in. Backward compatible: existing columns without a phpDefault behave identically. - DateCreatedFactory and DateModifiedFactory now provide phpDefaults that return the current MySQL-format timestamp. The DB-side DEFAULT CURRENT_TIMESTAMP stays in place as belt-and-suspenders coverage for inserts that bypass the framework. - WithDatastoreHandlerMethods::create() applies phpDefaults to the attributes array, runs the insert, and hydrates the model via modelAdapter->toModel( array_merge(attributes, ids)). No second SELECT. Cache pre-warm via the existing cacheItems() so subsequent reads short-circuit through the cache the same way they do today. The new contract: any column whose value should appear in the post-create() model must either be passed by the caller or come from a column factory that provides a phpDefault. Trigger-set values, generated columns, and DB defaults without a matching phpDefault won't be reflected in the in-memory model - they'll need an explicit refresh by the caller if they're load-bearing. Tests cover: - Column.withPhpDefault / getPhpDefault contract - DateCreatedFactory and DateModifiedFactory produce a working phpDefault - create() hydrates without calling query() (no readback) - create() applies phpDefaults for missing columns - Caller-provided values win over phpDefaults Closes the underlying race that motivated phpnomad/wordpress-integration#30 and phpnomad/mysql-db-integration#12 - both can stay closed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
This keeps the read-after-write consistency fix entirely inside the WordPress integration.
The behavior is dynamic:
$wpdb->get_results()path.insert()resolves$wpdb->insert_idinside the same explicit transaction as the insert.insert(),update(), ordelete(), the strategy marks that a write occurred.send_reads_to_masters()exists, it is used.No datastore-layer interface was added. No public API changed.
Why
PHPNomad's datastore create flow inserts a row, receives an identity, and then reads the model back. On managed WordPress hosts with read/write splitting, that immediate read can hit a stale replica even though the insert succeeded.
This fix keeps normal reads cheap while ensuring reads that happen after a write can see the write.
Production Evidence
Darren's live SmartWorkflowGuru database was pulled into a disposable test install with WP Migrate DB Pro on April 28, 2026.
Observed production state from the failed Stripe/WooCommerce order:
8870completed at2026-04-27 16:27:03UTC with Stripe/Link payment.19and20were created at2026-04-27 16:26:49and2026-04-27 16:26:57UTC for engagement81.pendingwithtransactionId = NULLandobligationId = NULL.29and30existed with matching timestamps, proving the inserts did commit.30also had a transaction detail row, but neither29nor30had awc_ordermapping for order8870.RecordNotFoundExceptionfromCanQueryWordPressDatabase.php:38duringIdentifiableDatabaseDatastoreHandler->create()after inserting opportunities.That shape is the failure this PR targets: the write succeeds, then PHPNomad's immediate post-write hydration/read path fails to see the row and callers treat creation as failed.
Cloudways Test Notes
A standard Cloudways test app was also loaded with the live DB. It reported:
$wpdbclass:wpdbwp-content/db.php: absentDB_HOST: local socket (localhost:/run/mysqld/mysqld.sock)On that standard Cloudways app, the issue did not reproduce:
$wpdbinsert/select cycles outside a transaction: 0 failures.$wpdbinsert/select cycles inside a transaction: 0 failures.woocommerce_order_status_completedhook for order8870outside a transaction: all created complete conversions with transaction mappings.This indicates the standard Cloudways test app is not equivalent to the customer's Cloudways Autonomous database topology. It does not remove the production evidence above; it only explains why the generic Cloudways app did not reproduce the managed-read failure.
Impact
This should prevent a successful insert from being followed by a stale read that makes PHPNomad behave as if the inserted row does not exist.
The extra transaction work only happens on insert identity resolution and on reads after a write has occurred in the same query strategy instance.
Validation
vendor/bin/phpunit --testdox