Page MenuHomePhabricator

Provide a narrow interface for code that needs to wait for DB replication lag
Open, MediumPublic

Description

Code that does lots of DB writes and needs to wait for DB replication lag must either call the deprecated wfWaitForSlaves() method or pull in LBFactory; the first goes against dependency injection principles, the second adds more dependencies and complexity than needed. There should be a narrower interface (whether it's implemented by LBFactory or not) for waiting for replicas to catch up.

Event Timeline

It's unclear to me what problem this is aiming to solve.

Would "code that does lots of DB writes" not naturally hold an LBFactory object regardless?

How could this "replication manager" interface be realistically be implemented by anything other than LBFactory? Hence, what is the added value of the distinction, if we were to make that?

Krinkle triaged this task as Medium priority.Jul 23 2019, 6:14 PM
Anomie subscribed.

DI advocates like to have such single-method interfaces. The claimed benefit is that the resulting code doesn't actually "depend" on the whole of LBFactory even though LBFactory is needed to implement the interface.

Application code typically needs three things from the DB layer: a way to get connection objects, a way to defer slow queries during performance-sensitive operations and a way to ensure long-running queries do not cause disruption. Historically, the first was done via wfGetDB, the second via DeferredUpdates, the third via wfWaitForSlaves. With dependency injection, the first is done via injecting lazy DB handles, the second will presumably be done by turning DeferredUpdates into a service, but there's no obvious path forward for the third. Se we end up injecting LBFactory, which is an obvious violation of the interface segregation principle: it's a complex interface with very little cohesion and lots of random staff in it that pretty much no caller ever cares about (setTableAliases? getChronologyProtectorClientId? flushReplicaSnapshots? rollbackMasterChanges? etc).

The implementation would presumably be by the replication manager holding the LBFactory instance and proxying calls to it (the traditional way would be to split ILBFactory into smaller pieces, but per T193613: RFC: Establish stable interface policy for PHP code we are trying to move away from relying on interfaces). Whether LBFactory itself should be split up into smaller classes or not is not relevant for the purposes of this task (and I'm not familiar enough with the code to have an opinion on it).

The status quo is that most web-facing code, deferred updates, jobs, and CLI scripts don't worry about transactions. They are started and ended around them automatically, including any back-off or chronology protection.

When you want to perform multiple batches of db-heavy work inside a deferred update, job, or CLI script; there are various tools in place to facilitate this. For example:

  • CLI scripts: The Maintenance class provides beginTransaction and commitTransaction, where the latter enforces graceful behaviour with replication waits.
  • Deferred updates: Instances of the DataUpdate base class are given a transaction ticket, delegating the permission to call LBFactory::commitAndWaitForReplication which commits the externally controlled transaction, wait for replication, and then re-creates a similar transaction to continue on with.
  • Job: Similar to DeferredUpdates although for jobs it isn't currently injected. Instead, the empty ticket is reserved ahead of time, and any job can call $lbFactory->getEmptyTransactionTicket( __METHOD__ ); from its entry point method to opt-in before it performs any queries, and thus then be able to use commitAndWaitForReplication the same way as DeferredUpdates and CLI scripts do.

I don't understand the problem behind the Interface Segregation Principle, at least not if it means that every subset of methods a consumer might want should be materialized in an infinite number of arbitrary interfaces that have no logical contract or reason for being. I assume that's not the intent, so I'm likely missing something here.

More generally, I think a not insignificant number of historic coding principles relate to how C compiles and traverses interface-include files, which doesn't really apply to most modern runtimes. Some of these principles do (also) have innate maintenance or social benefits for how code can be understood and structured. If that's the case here, that would be helpful to quantify a bit more as it isn't clear to me. Not clear why it matters how RDBMS internally divides its service responsibilities, and what other methods might be provided by a given service.