Page MenuHomePhabricator

Refactor mariadb puppet code
Open, MediumPublic

Description

Issues with the current mariadb code:

  • There's a lot of duplication.
    • mariadb::core::multinstance has a bunch of duplication with mariadb::core.
    • prometheus::mysqld_exporter vs prometheus::mysqld_exporter::instance
  • It's very hard to follow the structure. These are the notes i wrote when trying to get my head around it: P11725
    • Further notes for the mysql exporter config: P12044
  • Inconsistent use of roles vs profiles (some roles like mariadb::core do everything in the role itself, others have very slim roles that defer the real work to profiles).
  • Inconsistent use of heira vs site.pp parameters. E.g. the mysql_role for core dbs is in hiera, but for role::mariadb::misc it's in site.pp. Same goes for shard/section.
  • Because of the confusing structure and the amount of duplication it's very hard to
    • see what's different between different profiles
    • make sure that any change touches all of the relevant profiles
    • see what the effect of any change is
  • A lot of the code doesn't follow the modern WMF puppet style.
  • Inconsistent use of "shard" vs "section"
  • Multiinstance profiles are very repetitive, with a repeated block per possible section.
  • Naming could be better. E.g. core is not very descriptive, something like mw would be a lot clearer.
  • my.cnf templates have a lot of divergence from each other, making it hard to see what the semantic differences are.
  • Config tries to load semisync plugins even on 10.4, where the functionality is built-in
  • Some modules are wmf-specific, and should therefore be profiles instead.
  • Sanitarium masters should have a specific role, instead of being mariadb::core and being distinguished by ROW replication.
  • role::mariadb::dbstore_multiinstance should be split into backup sources and analytics hosts.
  • modules/profile/manifests/mariadb/parsercache.pp uses datadir => /srv/sqldata-cache making it the only role that uses that one. We should change it to sqldata like any other single instance host.
  • Hiera key naming doesn't follow policy: T247956: Ensure hiera only has profile:: qualified or global hiera keys
  • modules/roles/templates/mariadb vs modules/profiles/templates/mariadb. Why have 2 places?
  • profile::mariadb::grants::production takes 2 params that aren't used any more ($prompt and $password).
  • mariadb::service isn't used consistently across various roles. E.g. db_inventory didn't use it at all. It doesn't support multi-instance hosts, and hence complicates attempts to improve our systemd configuration: T289488: Systemd enhancements for mariadb and prometheus-mysql-exporter

Details

Related Changes in Gerrit:
SubjectAuthorRepoBranchLines +/-
Jbondoperations/puppetproduction+27 -29
Dzahnoperations/puppetproduction+3 -3
Kormatoperations/puppetproduction+4 -4
Kormatoperations/puppetproduction+60 -90
Kormatoperations/puppetproduction+9 -9
Kormatoperations/puppetproduction+17 -22
Kormatoperations/puppetproduction+74 -158
Kormatoperations/puppetproduction+35 -107
Kormatoperations/puppetproduction+43 -64
Kormatoperations/puppetproduction+89 -84
Kormatoperations/puppetproduction+6 -29
Kormatoperations/puppetproduction+30 -110
Kormatoperations/puppetproduction+2 -1
Kormatoperations/puppetproduction+33 -61
Kormatoperations/puppetproduction+6 -2
Kormatoperations/puppetproduction+73 -129
Kormatoperations/puppetproduction+50 -75
Show related patches Customize query in gerrit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Change 622578 had a related patch set uploaded (by Kormat; owner: Kormat):
[operations/puppet@production] mariadb: Add profile::mariadb::common

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

Change 622743 had a related patch set uploaded (by Kormat; owner: Kormat):
[operations/puppet@production] mariadb: Use hiera for shard+role for all profiles.

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

Change 622743 merged by Kormat:
[operations/puppet@production] mariadb: Use hiera for shard+role for all profiles.

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

Change 622972 had a related patch set uploaded (by Kormat; owner: Kormat):
[operations/puppet@production] mariadb: Create profile::mariadb::packages_wmf

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

Change 622995 had a related patch set uploaded (by Kormat; owner: Kormat):
[operations/puppet@production] mariadb: Allow overriding of wmf-mariadb version in hiera

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

Change 623567 had a related patch set uploaded (by Kormat; owner: Kormat):
[operations/puppet@production] mariadb: Add profile::mariadb::packages_client

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

Kormat moved this task from Meta/Epic to In progress on the DBA board.

Change 623582 had a related patch set uploaded (by Kormat; owner: Kormat):
[operations/puppet@production] mariadb: Make mariadb::config basedir required.

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

Change 622569 had a related patch set uploaded (by Kormat; owner: Kormat):
[operations/puppet@production] mariadb: simplify mariadb::service

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

Change 623608 had a related patch set uploaded (by Jbond; owner: John Bond):
[operations/puppet@production] role:mx: add script to generate otrs aliases

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

Change 622972 merged by Kormat:
[operations/puppet@production] mariadb: Create profile::mariadb::packages_wmf

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

Change 622995 merged by Kormat:
[operations/puppet@production] mariadb: Allow overriding of wmf-mariadb version in hiera

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

Change 623567 merged by Kormat:
[operations/puppet@production] mariadb: Add profile::mariadb::packages_client

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

Change 623582 merged by Kormat:
[operations/puppet@production] mariadb: Make mariadb::config basedir required.

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

Change 622569 merged by Kormat:
[operations/puppet@production] mariadb: simplify mariadb::service

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

Change 631288 had a related patch set uploaded (by Dzahn; owner: Dzahn):
[operations/puppet@production] prometheus: convert mysqld_exporter from role to profile

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

Change 633768 had a related patch set uploaded (by Kormat; owner: Kormat):
[operations/puppet@production] mariadb: Remove unused hiera lookups

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

Change 633769 had a related patch set uploaded (by Kormat; owner: Kormat):
[operations/puppet@production] [WIP] mariadb: Convert role::mariadb::core to profile.

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

Change 633768 merged by Kormat:
[operations/puppet@production] mariadb: Remove unused hiera lookups

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

Change 633769 merged by Kormat:
[operations/puppet@production] mariadb: Convert role::mariadb::core to profile.

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

Change 634034 had a related patch set uploaded (by Kormat; owner: Kormat):
[operations/puppet@production] mariadb: misc::multiinstance - generate sections.

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

Change 634042 had a related patch set uploaded (by Kormat; owner: Kormat):
[operations/puppet@production] mariadb: sanitarium_multiinstance - generate sections.

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

Change 634044 had a related patch set uploaded (by Kormat; owner: Kormat):
[operations/puppet@production] mariadb: dbstore_multiinstance - generate sections.

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

Change 634208 had a related patch set uploaded (by Kormat; owner: Kormat):
[operations/puppet@production] mariadb: misc::analytics::multiinstance - generate sections

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

Change 634034 merged by Kormat:
[operations/puppet@production] mariadb: misc::multiinstance - generate sections.

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

Change 634042 merged by Kormat:
[operations/puppet@production] mariadb: sanitarium_multiinstance - generate sections.

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

Change 634044 merged by Kormat:
[operations/puppet@production] mariadb: dbstore_multiinstance - generate sections.

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

Change 634208 merged by Kormat:
[operations/puppet@production] mariadb: misc::analytics::multiinstance - generate sections

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

Kormat updated the task description. (Show Details)
Kormat updated the task description. (Show Details)

Change 634953 had a related patch set uploaded (by Kormat; owner: Kormat):
[operations/puppet@production] mariadb: Use lookup() instead of hiera()

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

Change 634953 merged by Kormat:
[operations/puppet@production] mariadb: Use lookup() instead of hiera()

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

Change 634387 had a related patch set uploaded (by Dzahn; owner: Dzahn):
[operations/puppet@production] mariadb::grants: hiera()->lookup()

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

Change 635268 had a related patch set uploaded (by Kormat; owner: Kormat):
[operations/puppet@production] mariadb: Move passwords include to profiles

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

Change 622578 abandoned by Kormat:
[operations/puppet@production] mariadb: Add profile::mariadb::common

Reason:
There are so many small differences between all the various roles/profiles that this is currently futile.

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

Change 635268 merged by Kormat:
[operations/puppet@production] mariadb: Move passwords include to profiles

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

Change 634387 merged by Dzahn:
[operations/puppet@production] mariadb::grants: hiera()->lookup()

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

Change 640210 had a related patch set uploaded (by Jbond; owner: John Bond):
[operations/puppet@production] prometheus::mysqld_exporter::instance: drop arguments paramteter

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

Change 640210 merged by Jbond:
[operations/puppet@production] prometheus::mysqld_exporter::instance: drop arguments paramteter

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

@Kormat: Hi, all related patches in Gerrit have been merged. Can this task be resolved (via Add Action...Change Status in the dropdown menu), or is there more to do in this task? Asking as you are set as task assignee. Thanks in advance!

Kormat is ooo, the ticket is not done yet, some parts have been addressed some didn't. I could take it over but won't get to do anything on it any time soon, so it's better to leave it like that.

@Kormat: Removing task assignee as this open task has been assigned for more than two years - See the email sent to task assignee on Feburary 22nd, 2023.
Please assign this task to yourself again if you still realistically [plan to] work on this task - it would be welcome! :)
If this task has been resolved in the meantime, or should not be worked on by anybody ("declined"), please update its task status via "Add Action… 🡒 Change Status".
Also see https://www.mediawiki.org/wiki/Bug_management/Assignee_cleanup for tips how to best manage your individual work in Phabricator. Thanks!

@Aklapper Kormat has been on leave for some time now; could you hold off unassigning their tasks, please, or at least co-ordinate with our team leader @KOfori first?

@MatthewVernon: Hi, there was only this one task this time.

Marostegui updated Other Assignee, removed: Kormat.