Rdbms library is too aggressive in changing to read-only
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Ladsgroup
	Aug 11 2022, 5:46 AM

Description

We are doing way more master switchovers and part of the switchover is the replicas topology changes. Basically move all of the replicas (except the-to-be-master) under the-to-be-master. For example:

(and during the actual switchover, you just bring the old master under the new one)

This happens around half an hour before the actual switchover (taking around ten to twenty minutes) and only deals with replicas. Yet MediaWiki is acting too smart and decides to go read-only a lot during that time, causing read-only errors way more than the actual switchover: https://logstash.wikimedia.org/goto/580e0362614b1ffd4df5d71749a681a6

This needs fixing.

Details

Other Assignee: Krinkle

Project	Branch	Lines +/-	Subject
mediawiki/core	master	+25 -149	Get rid of concept of lagged replica protection
mediawiki/core	master	+9 -19	rdbms: Remove getLaggedReplicaMode() in favour of laggedReplicaUsed()
mediawiki/core	master	+6 -36	rdbms: Remove "read-only primary" and restore "short cache" in lagged mode
mediawiki/core	master	+23 -34	rdbms: Stop going read-only if all replicas are lagged

Customize query in gerrit

Related Objects

Mentioned In: T292543: Improve the community relations process for data center switchover
Mentioned Here: T141968: Display lag on grafana (prometheus) from pt-heartbeat instead (or in addition) of Seconds_Behind_Master

Event Timeline

Ladsgroup created this task.Aug 11 2022, 5:46 AM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 11 2022, 5:46 AM

Ladsgroup updated the task description. (Show Details)Aug 11 2022, 5:47 AM

Aklapper renamed this task from MediaWiki is too aggersive in changing to read-only to MediaWiki is too aggressive in changing to read-only.Aug 11 2022, 7:43 AM

Krinkle assigned this task to Ladsgroup.Aug 15 2022, 7:37 PM

Krinkle edited projects, added Performance-Team (Radar); removed Performance-Team.

I think the easiest would be to increase the max lag before topology change and reset it right after.

I don't know where this comment is coming from though:

// should be safely less than $wgCdnReboundPurgeDelay
'max lag'            => 6,

We have semi-sync enabled in production, there is not much point in going read-only when replication is lagging behind.

I don't see enough lag to hit "max lag" read-only mode for s2 in https://grafana-rw.wikimedia.org/d/G9kbQdRVz/mediawiki-loadbalancer?orgId=1&from=1660194000000&to=1660197600000&kiosk . I see the "all replicas lagged" logs though (I guess the statsd gauge wasn't updated frequently enough to see >6s). The tree diagram shows 0s lag and MW showed ~4s. Wait positions timed out too. Is the diagram just using seconds_behind_master? Is there really high lag during those 20min (pt-heartbeat)? The "Mysql Replication" dashboard on Grafana just uses seconds_behind_master too.

I think it's pt-heartbeat. Again, I said it in the patch and I say it here again. MediaWiki should not do the job replication management. It's job of mysql cluster/semi-sync/pt-heartbeat/alerting/etc. The whole concept of going read-only if there is lag > 6s in all replicas is useless and harmful.

Change 832270 had a related patch set uploaded (by Ladsgroup; author: Amir Sarabadani):

[mediawiki/core@master] Get rid of concept of lagged replica protection

https://gerrit.wikimedia.org/r/832270

gerritbot added a project: Patch-For-Review.Sep 14 2022, 2:35 PM

Krinkle moved this task from Limbo to Perf recommendation on the Performance-Team (Radar) board.Sep 26 2022, 8:23 PM

Krinkle edited projects, added Performance-Team; removed Performance-Team (Radar).Oct 3 2022, 8:09 PM

Krinkle moved this task from Untriaged to Rdbms library on the MediaWiki-libs-Rdbms board.Oct 15 2022, 7:28 PM

Krinkle renamed this task from MediaWiki is too aggressive in changing to read-only to Rdbms library is too aggressive in changing to read-only.Oct 17 2022, 7:47 PM

Krinkle moved this task from Inbox, needs triage to Backlog: Maintenance, non-prioritized on the Performance-Team board.Oct 17 2022, 7:51 PM

This happens in codfw switchovers too, had 30,000 errors in an hour ago: https://logstash.wikimedia.org/app/discover#/doc/logstash-*/logstash-mediawiki-1-7.0.0-1-2022.12.07?id=jgGm7YQBIFwrQt9ttHqb

Change 874940 had a related patch set uploaded (by Ladsgroup; author: Amir Sarabadani):

[mediawiki/core@master] rdbms: Stop going read-only if all replicas are lagged

https://gerrit.wikimedia.org/r/874940

Ladsgroup edited projects, added DBA; removed Data-Persistence (work done).Jan 17 2023, 6:32 AM

Ladsgroup moved this task from Triage to In progress on the DBA board.

Change 874940 merged by jenkins-bot:

[mediawiki/core@master] rdbms: Stop going read-only if all replicas are lagged

https://gerrit.wikimedia.org/r/874940

ReleaseTaggerBot added a project: MW-1.40-notes (1.40.0-wmf.20; 2023-01-23).Jan 17 2023, 5:00 PM

Ladsgroup mentioned this in T292543: Improve the community relations process for data center switchover.Jan 17 2023, 5:30 PM

Change 880995 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] rdbms: Remove "read-only primary" and restore "short cache" in lagged mode

https://gerrit.wikimedia.org/r/880995

Change 880996 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] rdbms: Remove getLaggedReplicaMode() in favour of laggedReplicaUsed()

https://gerrit.wikimedia.org/r/880996

We still don't know why the pt-heartbeat would show high lag in this scenario. Is pt-heartbeat stopped during this time on the primary?

In T314975#8531645, @gerritbot wrote:

Change 874940 merged by jenkins-bot:

[mediawiki/core@master] rdbms: Stop going read-only if all replicas are lagged

https://gerrit.wikimedia.org/r/874940

I did several switchovers and can confirm this fixes the issue.

In T314975#8544823, @aaron wrote:

We still don't know why the pt-heartbeat would show high lag in this scenario. Is pt-heartbeat stopped during this time on the primary?

We only kill pt-hearbeat for like 2 seconds when we do the primary switchover itself (that is: changing the master, the last step of the switchover).
When we move the replicas around, we stop replication on two hosts:

The future primary master
The replica that is going to be moved under it.

We only move one replica at the time, so if anything, we should only show lag on two hosts (the future master and the replica that is being moved around). Obviously, as more and more replicas get moved under the future new master, this pt-heartbeat lag might show up on more hosts).
For instance, say we have 10 replicas, once 9 of them are already under the new master, when moving the last one, stopping replication on the future new master will make those 9 replicas to show pt-heartbeat-lag.

Hope this helps

Thanks @Marostegui. Some stuff to add:

The all replicas are lagged happens actually quite early during the topology changes, e.g. when only two replicas are actually lagged.
- So I think there might be something with how mw reads lag, orchestrator doesn't show any lag during the time the mw goes read-only.
The whole operation is quite quick (in total one minute per moving one replica and it includes a lot of operations that are outside of the stopped replication timespan), the actual time replicas are lagged is quite short.
Generally, the LB/LBF going read-only here is quite an overreaction to lagged replicas. MW shouldn't do flow control for databases.

I spent most of yesterday and today reading code and looking at pt-hearbeat and I'm fairly certain it's not an issue with pt-heartbeat (I also found an issue with handling of pt-heartbeat in mw, ticket coming). Otherwise it would show up in grafana which it doesn't as you said in T314975#8209689.

My bet would be on LoadMonitor and how it stores and retrieves cached values. I think something makes it add lags to the cached value instead of replacing it, it's an issue with our caching that I have seen with Special:LintErrors too. If you refresh the page in certain times (not back to back), all the values it in the page would be doubled (these values are coming from a cache, look at TotalsLookup class), probably the eager recalculate is buggy.

In T314975#8555910, @Ladsgroup wrote:

I spent most of yesterday and today reading code and looking at pt-hearbeat and I'm fairly certain it's not an issue with pt-heartbeat (I also found an issue with handling of pt-heartbeat in mw, ticket coming). Otherwise it would show up in grafana which it doesn't as you said in T314975#8209689.

Also T141968: Display lag on grafana (prometheus) from pt-heartbeat instead (or in addition) of Seconds_Behind_Master

Change 880995 merged by jenkins-bot:

[mediawiki/core@master] rdbms: Remove "read-only primary" and restore "short cache" in lagged mode

https://gerrit.wikimedia.org/r/880995

Change 880996 merged by jenkins-bot:

[mediawiki/core@master] rdbms: Remove getLaggedReplicaMode() in favour of laggedReplicaUsed()

https://gerrit.wikimedia.org/r/880996

ReleaseTaggerBot edited projects, added MW-1.40-notes (1.40.0-wmf.21; 2023-01-30); removed MW-1.40-notes (1.40.0-wmf.20; 2023-01-23).Jan 30 2023, 11:00 PM

Ladsgroup closed this task as Resolved.Jan 31 2023, 4:59 AM

Ladsgroup updated Other Assignee, added: Krinkle.

Maintenance_bot moved this task from In progress to Done on the DBA board.Jan 31 2023, 5:15 AM

Change 832270 abandoned by Krinkle:

[mediawiki/core@master] Get rid of concept of lagged replica protection

Reason:

Superseded by alternate patches that trim down the feature a lot, but keeps some of it.