Page MenuHomePhabricator

Break down monster class: Database
Open, MediumPublic

Description

This class has more than 6000 lines and more than 100 public functions, it does everything related to databases and needs to be broken down. Some ideas that were brought up by @daniel are:

  • Migrate transaction management to its own class and interface (T299698)
  • Move replication-based stuff to its own class, e.g. getLag()
  • Remove legacy cruft from introduction of ResultWrapper: T286694
  • Move schema update and related functions to IMaintainableDatabase or DatabaseUpdater (T190396)
  • Move SQL building code from Database class to SQLPlatform (T307616)

Related Objects

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

The "Move replication-based stuff to its own class" bullet could use some clarity. I'd imagine a lot of that could easily stay in Database/subclasses (due to rdbms specific logic). I guess you could have separate per-rdbms class hierarchies and inject the objects. Was this referring to more of the LoadBalancer code?

Yes, you basically answered yourself. This needs to be per-rdbms replication manager and we also need to move LB and LBFactory logic to it (as much as possible) as well, that would make the class more conceptually cohesive.

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

[mediawiki/core@master] rdmbs: Start of SQLPlatform to split out of Database

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

Change 745215 merged by jenkins-bot:

[mediawiki/core@master] rdmbs: Start of SQLPlatform to split out of Database

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

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

[mediawiki/core@master] rdbms: Start using SQLPlatform and move more methods there

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

Change 785807 merged by jenkins-bot:

[mediawiki/core@master] rdbms: Start using SQLPlatform and move more methods there

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

Jdforrester-WMF updated the task description. (Show Details)
Jdforrester-WMF updated the task description. (Show Details)
Jdforrester-WMF updated the task description. (Show Details)

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

[mediawiki/core@master] rdbms: Move several methods from IDatabase to IMaintainableDatabase

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

Change 821201 merged by jenkins-bot:

[mediawiki/core@master] rdbms: Move several methods from IDatabase to IMaintainableDatabase

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