When applications request a "write" connection, we must return a writable connection.
When the application requests a "read" connection, we're free to return a readable or a writable connection.
If the application requests a "read" connection, we currently return a writable connection if we already have one, and establish and return a readable connection otherwise.
It's desirable that the connection we return is labeled as "read" so we can throw if the application runs a write over it, like an INSERT. However, if an existing "w" connection exists we could reasonably return it labeled as an "r" connection. This would also improve "r" vs "w" safety.
That is, these are current bad behaviors:
$conn_w = $dao->establishConnection('w');
$conn_r = $dao->establishConnection('r');
// BAD: This works because $conn_r silently "upgraded" and returned
// the existing writable connection.
// Instead, it should throw: no writes allowed on a read connection.
queryfx($conn_r, 'INSERT ...');$conn_r = $dao->establishConnection('r');
// BAD: Always establishes a new connection, but could sometimes return
// the existing connection. Should return existing connection if the
// database on the other end is writable.
$conn_w = $dao->establishConnection('w');The impact of both is likely quite small. Case (A) is just minor application bugs around 'r' vs 'w' that will manifest once we start sending traffic to read replicas more regularly (see T11056). Case (B) mostly impacts the daemons. These would be good to clean up when T11908 happens, though.
PHI916 notes a likely-adjacent issue with enabling connection persistence and getting connection mode errors:
Exception: Attempting to issue a write query on a read-only connection (to database "phabricator_conduit")!