Automattic / Automattic/HyperDB

SELECT ... FOR UPDATE is routed to a read-only replica unless send_reads_to_masters() is called manually

Open
#191 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
PHP
Stars
150
Forks
28
PR merge metrics
No merged PRs in 30d

Description

Hi,

We are using HyperDB with a standard primary/replica configuration:

```php
$wpdb->add_database(
array(
'host' => 'primary.example.com',
'write' => 1,
'read' => 0,
)
);

$wpdb->add_database(
array(
'host' => 'replica.example.com',
'write' => 0,
'read' => 1,
)
);
```

We found that a transaction containing `SELECT ... FOR UPDATE` can route the locking `SELECT` to the read-only replica.

The relevant classification currently treats every `SELECT` statement as read-only:

```php
public function is_write_query( $q ) {
$q = ltrim( $q, "\r\n\t (" );
return ! preg_match( '/^(?:SELECT|SHOW|DESCRIBE|DESC|EXPLAIN)\s/i', $q );
}
```

As a result, `SELECT ... FOR UPDATE`, despite requiring a writable connection and being part of a transaction, is classified as a read query.

### Reproduction

```php
global $wpdb;

$wpdb->query( 'START TRANSACTION' );

$row = $wpdb->get_row(
"SELECT id, balance
FROM wp_credit_balances
WHERE id = 1
FOR UPDATE"
);

$wpdb->query( 'COMMIT' );
```

With a write-only primary and a read-only replica, the `SELECT ... FOR UPDATE` may be sent to the replica and fail with an error similar to:

```text
Cannot execute statement in a READ ONLY transaction
```

We observed the transaction start on the writer, followed by the locking `SELECT` on the replica.

### Expected behavior

Queries that require row locks should always use the writer connection, including at least:

```sql
SELECT ... FOR UPDATE
SELECT ... FOR SHARE
SELECT ... LOCK IN SHARE MODE
```

Once a transaction begins on the writer, all queries belonging to that transaction should ideally stay on the same writer connection until `COMMIT` or `ROLLBACK`.

### Current workaround

We currently force all subsequent reads to the writer before starting the transaction:

```php
if ( method_exists( $wpdb, 'send_reads_to_masters' ) ) {
$wpdb->send_reads_to_masters();
}

$wpdb->query(
'START TRANSACTION /* IN_TABLE=wp_credit_balances */'
);
```

This works, but `send_reads_to_masters()` applies to the rest of the PHP request and requires application code to know about HyperDB-specific routing.

The `IN_TABLE` hint helps HyperDB identify the dataset, but by itself it does not solve transactions that lock or update rows across multiple tables.

### Suggested improvement

Would it make sense for `is_write_query()` to identify locking reads explicitly?

For example, conceptually:

```php
if (
preg_match(
'/^\s*SELECT\b.*\b(?:FOR\s+UPDATE|FOR\s+SHARE|LOCK\s+IN\s+SHARE\s+MODE)\b/is',
$q
)
) {
return true;
}
```

Alternatively, HyperDB could track an active transaction per dataset/connection and pin all its queries to the writer until `COMMIT` or `ROLLBACK`.

Automatic transaction pinning would also prevent a transaction from accidentally spanning different database connections.

### Additional concern

MySQL advisory locks have the same connection-scope requirement:

```sql
SELECT GET_LOCK(...);
SELECT RELEASE_LOCK(...);
```

If application code calls `send_reads_to_masters()` between those queries, `GET_LOCK()` can run on a replica and `RELEASE_LOCK()` on the writer. Therefore, connection affinity during consistency-sensitive operations would be useful beyond `SELECT ... FOR UPDATE`.

Thanks for maintaining HyperDB. I would be happy to provide additional logs or test a proposed change.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start by tracing is_write_query() and reproducing the transaction with a writable primary and read-only replica. Check how SELECT ... FOR UPDATE, SELECT ... FOR SHARE, and LOCK IN SHARE MODE are classified and routed. Done means locking reads use the writer, with transaction queries remaining on the same connection until COMMIT or ROLLBACK.

Written by the indexing model from the issue text.

Assessment

Tech stack
mysql, php
Domain
databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.