Page MenuHomePhabricator

Create tool to handle the state of database configuration in MediaWiki in etcd
Closed, ResolvedPublic

Description

Most of the daily toil on our DBA team is caused by repetitive actions to change the state of individual databases or of entire sections. This right now is done via code deploys; a typical maintenance window on a single server can require at least 3 code deployments. This is not acceptable, and we decided to move the storage of such state from the code repository to our consistent datastore, etcd.

Given how critical and complex those data are, our standard confctl tool won't be enough to ensure data are correctly stored and to minimize the risk of errors and the strain on DBAs from using the tool.

This CLI tool, that I propose to call dbctl, will still be part of conftool, and will have the following functionalities:

  • Depool/warmup/repool fully a database server
  • Change weights per server
  • Set which server is the master for each section
  • Set read-only per-section or globally

I also have a basic proposal for a UI, and the corresponding safety checks.

Get the current configuration

  • dbctl instance NAME get gets you all the current configuration of a mysql instance (so either host:port or host only if it's the default port)
  • dbctl section [SECTION|all] get shows what is configured for that specific section, or for all of them as a well formatted yaml
  • dbctl config get will get all the configuration, in the form of a data structure that will be read by MediaWiki

Depool one instance

First depool the resources we want on the instance

  • dbctl instance [NAME] depool [--section SECTION] [--group GROUP] , where both section and groups are optional. If none is specified, it's assumed we will act on all of them

Then we commit the changes to the MediaWiki config

  • dbctl config commit (means the instance is removed from sectionLoads and groupLoadsBySection in mediawiki-config)

Before we perform any write actions, we perform two basic sanity checks:

  • we're not removing the master
  • we're not leaving a section with less than N slaves (configurable)

further sanity checks might be added in the future.

Pool/warmup one instance

First pool the resources we want on the instance. If we add a `percentage

  • dbctl instance [NAME] pool [--section SECTION] [--group GROUP] [--percentage 100], where both section and groups are optional. If none is specified, it's assumed we will act on all of them. Percentage indicates what percentage of the original weight should be used when repooling. This allows to perform warmups by increasing the percentage parameter from 0 to 100 in steps.

Then we commit the changes to the MediaWiki config

  • dbctl config commit (means the instance is added back to sectionLoads and groupLoadsBySection in mediawiki-config, with the previously-stored weights * percentage * 0.01)
  1. Change the weights
  2. dbctl instance NAME set-weight [--section SECTION] [--group GROUP] WEIGHT

Then commit the configuration as shown before.

Set master for a section

  • dbctl section SECTION set-master NAME sets the master for that specific section to NAME
  • Will verify that the server is pooled for that section
  • Should verify that the new master is not a sanitarium master [NOT IMPLEMENTED]

Then commit the configuration as shown before.

Set a section read-only/read-write

  • dbctl section SECTION [ro|rw] REASON will set the section to readonly with the specified reason

Then commit the configuration as shown before.

Bootstrapping a new instance/section

As usual for conftool entities, the entities are created from yaml files, but once created they're uninitialized and won't appear in the configuration for mediawiki.

In order to initialize them, you need to edit the new object:

dbconfig [instance|section] NAME edit

Details

Related Gerrit Patches:
operations/mediawiki-config : masternoc: Add cross-dc navigation links to db.php footer
operations/puppet : productiondbctl_client.pp: Remove dbctl diff alerts
operations/puppet : productiondbctl: monitor for uncommitted changes
operations/puppet : productionconftool: update schemata for dbctl
operations/mediawiki-config : masternoc: allow db.php?dc=codfw instead of just eqiad
operations/software/conftool : masterAdd a WMF-specific tool for managing db config in MediaWiki

Event Timeline

Joe created this task.Jun 13 2018, 3:26 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 13 2018, 3:26 PM

Quick first feedback/questions on the proposal:

dbconfig get NAME gets you all the current configuration of a mysql instance (so either host:port or host only if it's the default port)

I think that if NAME is just the hostname, it should show the config for all the configured HOST:PORT combinations in that host.

dbconfig list [SECTION|all] shows what is configured for that specific section, or for all of them

It seems to me that show might be a better verb here, from list I would expect a list of things, not their full config. Also doesn't seem to match the actions below dbconfig section [command], so maybe could be revisited or considered a shortcut, for example of dbconfig section SECTION show.

dbconfig [depool|warmup|pool] [NAME] [selector tags]

If no selector tags are specified, does it apply to all? All configured? It's not clear.
If I do pool after a depool and a warmup with different values (0.1, 0.3, 0.6), what it does? I would expect it to reset the weight to it's configured value.

dbconfig warmup NAME 0.1 pools a depooled database with weights set to ceil(weights*0.1)

Following the above question, without specifying selector tags, how this applies to a db that is both in sectionLoads with wight 100 and in groupLoadsBySection, in multiple subsections, with different weights? Would the warmup factor apply to all of them?
I guess that if applied to an already pooled host it will just change it's wight accordingly, is that correct?

dbconfig edit LABEL

What is a LABEL here?

dbconfig section [command] SECTION [params]

Consider also the alternative dbconfig section SECTION [command] [params], it should reduce the horizontal scrolling when doing maintenance (check current value, change it, verify it, etc..). Also the values are close to their label (section SECTION, ro REASON, etc..)

This is great! Thanks for getting on with this - these are my first thoughts!

Depool/pool/warmup
dbconfig [depool|warmup|pool] [NAME] [selector tags], where selector tags are on function (main, vslow, rc, etc.)
depool means removing from sectionLoads and groupLoadsBySection
Sanity check 2: we're not leaving a section with less than N slaves (configurable)

There should be an emergency mode like --emergency
Some of our sections are a bit over provisioned (like s5), and we can probably serve them (in a degraded state) with 1-2 slaves (as was shown during https://wikitech.wikimedia.org/wiki/Incident_documentation/20171116-s5-dewiki-wikidata where most of our slaves broke).
There should be a quick way to by pass the "N min amount of slaves" if required during an emergency. Some sort of: "I know what I am doing" mode.

dbconfig section set-master SECTION NAME sets the master for that specific section to NAME
Sanity check 1: Will verify that the server is pooled for that section

This is not likely to happen in a near future, but as we are starting from 0....I would suggest that the sanity check will consider the fact that a master can be in one or multiple sections (it is unlikely that we will decide to run multi-instance on masters, but I don't think we can completely discard it).

There should also be another sanity check, which is: a sanitarium master cannot be pooled as master.
However, the only way to identify a sanitarium master right now is to scan db-eqiad.php or db-codfw.php - we do not have them anywhere else.
The reason we cannot get them as masters is because they run ROW based replication and not STATEMENT.
We actually ran ROW based replication on s5 a year ago (by mistake) and despite the fact that it worked fine, we are not completely ready for it at this point as doing schema changes the way we do them now (slave by slave) is incompatible (and caused an outage - https://wikitech.wikimedia.org/wiki/Incident_documentation/20171116-s5-dewiki-wikidata)

Marostegui triaged this task as Normal priority.Jun 14 2018, 5:54 AM

I think that if NAME is just the hostname, it should show the config for all the configured HOST:PORT combinations in that host.

No, NAME should be instances only, this tools should not know anything about servers. If you run something on HOST and it is not on the default port, it should give out an error and fail quickly.

After thinking for a while, pool|depool|warmup (as an interface, not as the idea) looks confusing and not adjusted to real usage, several people have objected to it- plus if you have a host fully pooled and run warmup 0.5, you are actually depooling it partially. What would happen after doing a warmup depending on the initial state is missleading. My proposal is to get rid of the warmup verb, which is confusing, and have only 2 verbs:

  • pool [fraction]
  • depool

depool removes an instance from a service (or from all services), pool, without parameters re-adds it with the default, pool with a parameter re-adds it with the given percentage pool == pool 1. That will make completely sure what you are doing when you are running those actions (pooling or depooling). If someone puts a fraction on depooling, it should error out directly.

Please let's use the terminology mediawiki uses:

  • Section is 's1', 's2', 'm1', 'pc1', 'es1' (some of those sections are shards, some are not)
  • subtypes of traffic are called 'groups' (watchlist, vslow, dump)
  • Load or section load is value assigned to a combination of section + instance name. Group load is the value for a section, instance and group

Also, load 0 + pooled is a valid state, and it is different from the "depooled" state.

Sanity checks are a complex issue and should not be discussed in long here until we have something to work with. Note that when we depool a server, it is likely we reassign a host to a different group temporarily, rather than having multiple (or at least 1) host all the time. For example, s3 has no groups defined except vslow and dump. When a group doesn't exist, it uses the general/main load.

Joe added a comment.Jun 14 2018, 8:26 AM

Quick first feedback/questions on the proposal:

dbconfig get NAME gets you all the current configuration of a mysql instance (so either host:port or host only if it's the default port)

I think that if NAME is just the hostname, it should show the config for all the configured HOST:PORT combinations in that host.

NAME is generic as a label and typically will be HOST:PORT or HOST if the port is 3306; like we do currently in mediawiki-config; I was not planning on doing "all instances on a host" automagically.

dbconfig list [SECTION|all] shows what is configured for that specific section, or for all of them

It seems to me that show might be a better verb here, from list I would expect a list of things, not their full config. Also doesn't seem to match the actions below dbconfig section [command], so maybe could be revisited or considered a shortcut, for example of dbconfig section SECTION show

yeah, let's go that way. dbconfig secition SECTION show it is.

dbconfig [depool|warmup|pool] [NAME] [selector tags]

If no selector tags are specified, does it apply to all? All configured? It's not clear.
If I do pool after a depool and a warmup with different values (0.1, 0.3, 0.6), what it does? I would expect it to reset the weight to it's configured value.

I'm not sure I get what scenario you mean, but to be more specific: let's say you have a db with weights of 100 in main and 10 in vslow. dbconfig warmup mydb 0.1 will set the weights in mediawiki to 1/10th of the weight you stored, so respectively 10 and 1. If you then do dbconfig warmup mydb 0.6, the weights will now be 60 and 6 respectively.

dbconfig warmup NAME 0.1 pools a depooled database with weights set to ceil(weights*0.1)

Following the above question, without specifying selector tags, how this applies to a db that is both in sectionLoads with wight 100 and in groupLoadsBySection, in multiple subsections, with different weights? Would the warmup factor apply to all of them?
I guess that if applied to an already pooled host it will just change it's wight accordingly, is that correct?

Yes to all, see above

dbconfig edit LABEL

What is a LABEL here?

Should've been NAME. Amended.

dbconfig section [command] SECTION [params]

Consider also the alternative dbconfig section SECTION [command] [params], it should reduce the horizontal scrolling when doing maintenance (check current value, change it, verify it, etc..). Also the values are close to their label (section SECTION, ro REASON, etc..)

ok, amended.

Joe updated the task description. (Show Details)Jun 14 2018, 8:26 AM

@Joe ack to all your replies, thanks for integrating the suggestions!

Joe added a comment.Jun 14 2018, 8:31 AM

After thinking for a while, pool|depool|warmup (as an interface, not as the idea) looks confusing and not adjusted to real usage, several people have objected to it- plus if you have a host fully pooled and run warmup 0.5, you are actually depooling it partially. What would happen after doing a warmup depending on the initial state is missleading. My proposal is to get rid of the warmup verb, which is confusing, and have only 2 verbs:

  • pool [fraction]
  • depool

depool removes an instance from a service (or from all services), pool, without parameters re-adds it with the default, pool with a parameter re-adds it with the given percentage pool == pool 1. That will make completely sure what you are doing when you are running those actions (pooling or depooling). If someone puts a fraction on depooling, it should error out directly.

Perfect, this is much better indeed. Amending the UI specs.

Please let's use the terminology mediawiki uses:

  • Section is 's1', 's2', 'm1', 'pc1', 'es1' (some of those sections are shards, some are not)
  • subtypes of traffic are called 'groups' (watchlist, vslow, dump)
  • Load or section load is value assigned to a combination of section + instance name. Group load is the value for a section, instance and group

Also, load 0 + pooled is a valid state, and it is different from the "depooled" state.

Yes, pooled/depooled and weight are going to be different and distinct metrics.

Sanity checks are a complex issue and should not be discussed in long here until we have something to work with. Note that when we depool a server, it is likely we reassign a host to a different group temporarily, rather than having multiple (or at least 1) host all the time. For example, s3 has no groups defined except vslow and dump. When a group doesn't exist, it uses the general/main load.

Joe updated the task description. (Show Details)Jun 14 2018, 8:32 AM
Joe added a comment.Jun 14 2018, 8:35 AM

This is great! Thanks for getting on with this - these are my first thoughts!

Depool/pool/warmup
dbconfig [depool|warmup|pool] [NAME] [selector tags], where selector tags are on function (main, vslow, rc, etc.)
depool means removing from sectionLoads and groupLoadsBySection
Sanity check 2: we're not leaving a section with less than N slaves (configurable)

There should be an emergency mode like --emergency
Some of our sections are a bit over provisioned (like s5), and we can probably serve them (in a degraded state) with 1-2 slaves (as was shown during https://wikitech.wikimedia.org/wiki/Incident_documentation/20171116-s5-dewiki-wikidata where most of our slaves broke).
There should be a quick way to by pass the "N min amount of slaves" if required during an emergency. Some sort of: "I know what I am doing" mode.

The famous --bblack switch. Yes, there will be multiple ways to work around the safety checks, the easiest one will be a --unsafe switch to the dbconfig cli.

dbconfig section set-master SECTION NAME sets the master for that specific section to NAME
Sanity check 1: Will verify that the server is pooled for that section

This is not likely to happen in a near future, but as we are starting from 0....I would suggest that the sanity check will consider the fact that a master can be in one or multiple sections (it is unlikely that we will decide to run multi-instance on masters, but I don't think we can completely discard it).

Not sure I understand what I should check here: if the instance is in multiple sections, refuse to promote it to master?

There should also be another sanity check, which is: a sanitarium master cannot be pooled as master.
However, the only way to identify a sanitarium master right now is to scan db-eqiad.php or db-codfw.php - we do not have them anywhere else.

Well we can move that information to the section metadata on etcd.

Adding this.

Joe updated the task description. (Show Details)Jun 14 2018, 8:36 AM

This is not likely to happen in a near future, but as we are starting from 0....I would suggest that the sanity check will consider the fact that a master can be in one or multiple sections (it is unlikely that we will decide to run multi-instance on masters, but I don't think we can completely discard it).

Not sure I understand what I should check here: if the instance is in multiple sections, refuse to promote it to master?

No, just saying that when checking if the master is in an specific section, you might want to consider the fact that it could be appearing in multiple sections and act accordingly.
"I want to promote this server to master on s4 even if it is pooled as master in some other section". Again, this is unlikely to happen in a near future, but just saying that it is not a completely crazy idea.
So long story short, consider that masters can be pooled in more than one section.

jcrespo moved this task from Triage to Meta/Epic on the DBA board.Jun 15 2018, 11:30 AM
Joe claimed this task.Jun 19 2018, 8:43 AM
Joe updated the task description. (Show Details)
Joe added a project: User-Joe.

Change 441396 had a related patch set uploaded (by Giuseppe Lavagetto; owner: Giuseppe Lavagetto):
[operations/software/conftool@master] [WIP] Add a WMF-specific tool for managing db config in MediaWiki

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

Vvjjkkii renamed this task from Create tool to handle the state of database configuration in MediaWiki in etcd to l3aaaaaaaa.Jul 1 2018, 1:04 AM
Vvjjkkii removed Joe as the assignee of this task.
Vvjjkkii raised the priority of this task from Normal to High.
Vvjjkkii updated the task description. (Show Details)
Vvjjkkii removed subscribers: gerritbot, Aklapper.
Marostegui renamed this task from l3aaaaaaaa to Create tool to handle the state of database configuration in MediaWiki in etcd.Jul 2 2018, 5:13 AM
Marostegui assigned this task to Joe.
Marostegui lowered the priority of this task from High to Normal.
Marostegui updated the task description. (Show Details)
Joe moved this task from Backlog to Doing on the User-Joe board.Jul 2 2018, 1:03 PM
Joe moved this task from Doing to Blocked on others on the User-Joe board.Aug 3 2018, 9:54 AM
CDanis added a subscriber: CDanis.Mar 18 2019, 1:54 PM
Joe updated the task description. (Show Details)Apr 2 2019, 9:37 AM

Change 441396 had a related patch set uploaded (by CDanis; owner: Giuseppe Lavagetto):
[operations/software/conftool@master] Add a WMF-specific tool for managing db config in MediaWiki

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

CDanis added a comment.EditedMay 13 2019, 3:33 PM

Here's my tentative plan for moving forward with this, including a rollout procedure:

  • get https://gerrit.wikimedia.org/r/441396 checked in ASAP, almost certainly with a bunch of TODOs from reviewers to be fixed in followups
  • add functionality for showing diffs on dbctl config commit; also add a --dry-run
  • get final feedback from DBAs on interface
  • get the conftool schema files checked in to puppet
  • populate conftool objects with the then-current state of prod
  • either as a handler on noc.wikimedia.org, or as a password-protected Mediawiki handler, show diffs between what is returned from EtcdConfig vs what is live via the usual mediawiki-config files
  • have an emergency procedure for persisting and restoring a particular version of the compiled config to etcd directly
  • add some client-side data validation in EtcdConfig
  • add an Icinga check for diffs between etcd's instance/section config vs the compiled version
  • add an Icinga check for whatever client-side validation EtcdConfig does failing on the compiled config (likely per-appserver)
  • write up runbooks for the above

Once all of the above is done:

  • add a config flag that can be controlled by puppet hiera to use either mediawiki-config or EtcdConfig on a per-appserver basis; make those hosts easily depool-able
  • add an Icinga check for differences between mediawiki-config and compiled etcd config
  • start making all database changes both places, with just a few appservers using EtcdConfig
  • after a validation period, switch over the whole fleet to use EtcdConfig
  • delete the obsolete stanzas from db-eqiad.php and db-codfw.php
CDanis claimed this task.May 13 2019, 3:36 PM

It would be nice to have a mockup of the API to test soon (with no production effect except maybe some debug information). That will allow to test automation from scripts we have already. I think that would be step #6 ?

delete db-eqiad.php and db-codfw.php

That is unlikely to happen, there is unrelated configuration there, or which is not dynamic or undesirable to be on etcd. 90% of it could probably go away indeed.

I believe I had a set of tests to check different failure scenarios (DROP, REJECT, large timeout, etc.)

Jdforrester-WMF awarded a token.

It would be nice to have a mockup of the API to test soon (with no production effect except maybe some debug information). That will allow to test automation from scripts we have already. I think that would be step #6 ?

Yeah, that is the intent once the diffs handler on noc.wm.o or MW itself is present -- so everyone can try using the 'real' tool without affecting anything aside from that.

delete db-eqiad.php and db-codfw.php

That is unlikely to happen, there is unrelated configuration there, or which is not dynamic or undesirable to be on etcd. 90% of it could probably go away indeed.

Ah, yeah of course. Updated!

I believe I had a set of tests to check different failure scenarios (DROP, REJECT, large timeout, etc.)

Definitely curious to see whatever you have here :)

Joe added a comment.May 14 2019, 6:20 AM

EtcdConfig in MediaWiki has been extensively tested against failures before it was introduced, if that's what @jcrespo was referring to.

As for the deployment, I'd rather reason per-wiki rather than per-appserver, if possible. Although I think probably releasing the new function just on the debug servers (with an if $hostname guard in MediaWiki config) is enough.

As far as merging the first patch, I would consider you a valid reviewer given the bulk of the code was written before you looked into it, and you fixed most of the issues in the meantime.

Change 441396 merged by jenkins-bot:
[operations/software/conftool@master] Add a WMF-specific tool for managing db config in MediaWiki

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

Volans added a comment.Jul 5 2019, 6:46 AM

All patches for v1 of dbconfig are merged, including the ones to make a new conftool release. I'll rebuild the packages on Monday and upload them to our APT and I'll start testing the upgrade of python3-conftool (the base package, pretty much untouched).
The testing of python3-conftool-dbctl will follow after.

Mentioned in SAL (#wikimedia-operations) [2019-07-11T14:57:52Z] <cdanis> cdanis@cumin1001.eqiad.wmnet ~ % sudo debdeploy deploy -u T197126-2019-07-11-conftool.yaml -s mw-canary

Mentioned in SAL (#wikimedia-operations) [2019-07-11T15:05:26Z] <cdanis> cdanis@cumin1001.eqiad.wmnet ~ % sudo debdeploy deploy -u T197126-2019-07-11-conftool.yaml -s cp-canary

Mentioned in SAL (#wikimedia-operations) [2019-07-11T16:19:09Z] <cdanis> cdanis@cumin1001.eqiad.wmnet ~ % sudo debdeploy deploy -u T197126-2019-07-11-conftool.yaml -s ulsfo

Mentioned in SAL (#wikimedia-operations) [2019-07-11T16:47:57Z] <cdanis> cdanis@cumin1001.eqiad.wmnet ~ % sudo debdeploy deploy -u T197126-2019-07-11-conftool.yaml -s eqsin

Mentioned in SAL (#wikimedia-operations) [2019-07-11T17:02:21Z] <cdanis> cdanis@cumin1001.eqiad.wmnet ~ % sudo debdeploy deploy -u T197126-2019-07-11-conftool.yaml -s esams

Mentioned in SAL (#wikimedia-operations) [2019-07-11T17:37:35Z] <cdanis> cdanis@cumin1001.eqiad.wmnet ~ % sudo debdeploy deploy -u T197126-2019-07-11-conftool.yaml -s codfw

Mentioned in SAL (#wikimedia-operations) [2019-07-11T18:02:04Z] <cdanis> cdanis@cumin1001.eqiad.wmnet ~ % sudo debdeploy deploy -u T197126-2019-07-11-conftool.yaml -s eqiad

Change 523943 had a related patch set uploaded (by CDanis; owner: CDanis):
[operations/puppet@production] conftool: update schemata for dbctl

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

Change 523013 had a related patch set uploaded (by CDanis; owner: CDanis):
[operations/puppet@production] dbctl: monitor for uncommitted changes

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

Change 524551 had a related patch set uploaded (by CDanis; owner: CDanis):
[operations/mediawiki-config@master] noc: allow db.php?dc=codfw instead of just eqiad

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

Change 524551 merged by jenkins-bot:
[operations/mediawiki-config@master] noc: allow db.php?dc=codfw instead of just eqiad

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

Change 523943 merged by CDanis:
[operations/puppet@production] conftool: update schemata for dbctl

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

Change 523013 merged by CDanis:
[operations/puppet@production] dbctl: monitor for uncommitted changes

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

Change 527137 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[operations/mediawiki-config@master] noc: Add cross-dc navigation links to db.php footer

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

CDanis closed this task as Resolved.Aug 1 2019, 11:38 PM

This is resolved, just a few post-deploy cleanup tasks left to do.

Is there a tracking task for the features we've discussed? (depooling x1/es hosts, comments on hosts, etc..)

Change 527451 had a related patch set uploaded (by Marostegui; owner: Marostegui):
[operations/puppet@production] dbctl_client.pp: Remove dbctl diff alerts

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

Change 527451 abandoned by Marostegui:
dbctl_client.pp: Remove dbctl diff alerts

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

Change 527137 merged by jenkins-bot:
[operations/mediawiki-config@master] noc: Add cross-dc navigation links to db.php footer

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