Add optimistic pausing to avoid steal reads#1656
Conversation
| while Time.current < started_at + 0.2 # Wait for 200ms max | ||
| return if replica_has_transaction(last_txn) | ||
|
|
||
| sleep 0.02 |
There was a problem hiding this comment.
An alternative to sleeping the request (and wasting Puma worker time) would be to lean on the reproxy feature we built into Kamal Proxy. With that we'd end the request with a header that asks the proxy to present the same request again after a short delay. (Sleeping in the proxy is very cheap because it has massive concurrency.) We could also use that to shunt the request over to the primary if the replica still hasn't caught up after a bit, but that brings the obvious risk that increased replication lag could lead to overloading the primary and become a cascading failure.
In practice I think it'll depend on how much, and how often, we expect to see lag. If it's rare and short, a sleep might be just fine as a way to paper over the occasionally stale read. If it's more common we'll need something more robust; but if it's much more common then writer pinning was probably the better answer in the first place anyway.
|
Nice, it would be great if we can validate this idea. Would the plan be to move this to a Rails database resolver middleware rather than a concern after testing? |
Yes, I think a middleware makes most sense. The concern is just an easy place to test things out first. More generally, the part I was wondering about is whether we should lean on the proxy layer to help with this. If we don't, it's nice that this could be self-contained: we could allow configuring the poll interval and max delay, but otherwise all you'd need to do is enable the middleware and it works. But the downside would be if we had a sustained period of high replica lag, suddenly all the requests that follow a recent write become slower, which could affect capacity. Maybe that's fine though, it's an unusual case and there is an upper limit on how bad it can get. I think it'll be hard to tell without a bit of real-world testing. If we didn't mind involving custom proxy features, I think there could be a version of this where a stale read condition is returned to the proxy, and it could treat that like the writer pinning situation. So, instead of pinning to the writer for 1s after a write, you optimistically send all reads to the secondary site. But if one of those reads returns a special "stale read" header, you treat it like a pinned write: send it to the writer if it was within 1s of an actual write. That way it could degrade to the previous behaviour when necessary without adding delays at the Puma level. But now it's dependent on specific proxy support, which would be fine for Fizzy but becomes harder to make more generally available. I'm inclined to try running Fizzy on this simple version for a bit, and gather some data about how it behaves. What do you think? |
|
Sounds like a good a plan! BTW, did you explore using MySQL's WAIT_FOR_EXECUTED_GTID_SET function (it supports a timeout) instead of polling / sleeping in Ruby? |
Oh that's much better! I'll rework it to use that. (I assume it allows fractional timeouts since 1s feels quite long. I'll check.) |
Looks like it :) |
kevinmcconnell
force-pushed
the
txn-pinning
branch
5 times, most recently
from
0d4dbf1 to
b94fc3f
Compare
kevinmcconnell
force-pushed
the
txn-pinning
branch
5 times, most recently
from
7dd66d1 to
327fac2
Compare
| end.tap do | ||
| replica_metrics["X-Replica-Wait"] = Time.current - started_at | ||
| end |
There was a problem hiding this comment.
I think we can remove the custom headers after a couple of days. They are there now to make it easier to evaluate what's going on. But now that we are tracking aggregate metrics, I don't think we need them long term.
kevinmcconnell
force-pushed
the
txn-pinning
branch
from
327fac2 to
dafceab
Compare
| counter :replica_stale, | ||
| comment: "Number of requests served from a stale replica" |
There was a problem hiding this comment.
I've added this as a separate counter because it's tracking a specific condition -- the timeout expiring while waiting on the gtid.
I think technically this corresponds to the count of measurements in the uppermost histogram bucket, so in that sense maybe redundant. But it seems clearer this way, especially if we make adjustments to the max wait time while tuning this.
Instead of writer pinning, we'll track the last transaction ID of each write in the session. Then on each read we'll wait for the replica to report that this transaction is available. If it doesn't become available within a reasonable timeout, we'll proceed anyway, and accept the possibility of a stale read. The hope here is that most of the time, the replica is caught up in the time between a write request and the following read request. If it's not, we now have a little tolerance to wait for it, which hopefully proves enough to stale reads are not encountered in normal use. We also disable the writer affinity opt-out mechanism that we had before, since we will no longer be using writer affinity at the load balancer.
kevinmcconnell
force-pushed
the
txn-pinning
branch
from
dafceab to
bd259f7
Compare
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
|
Love this. Great one for jobs as well, by passing GTID as a job arg. |
3 participants
Instead of writer pinning, we'll track the last transaction ID of each write. Then on each read we'll check whether the replica already contains this transaction. If it doesn't, there's a chance of a stale read, so we'll briefly wait for it to catch up. If it doesn't catch up quickly enough, just bail and accept the possibility of the read being stale.
The theory being, often after a write the replication logs and the subsequent read requests will be racing each other to get to the new data, but commonly the replication gets there first. And even if a subsequent read request does get there first, the replication data will arrive very shortly after.
If in fact we see that, in practice, requests do commonly have to wait for the replicated data, we could move the delay out to the proxy layer by having it re-proxy the request after a short wait. That could free up Puma to serve another request sooner. But this way is very simple to try, so let's see how it behaves first.
Still validating this idea! I'd like to kick this a bit harder on staging and see if it's worth doing, and also see whether offloading those sleeps to the proxy is worth the complexity. First impressions: when I try this, I'm getting 100% of my reads from the local DC, and no delay is being applied. (So it either works great, or is broken 🤣)