Page MenuHomePhabricator

Support an "overlay" database connection mode where multiple applications share a single connection
Open, LowPublic

Description

See T11044 for some context.

Now that applications can be partitioned, we can get rid of one-connection-per-application. This will provide a slight performance improvement and a significant reduction in the number of connections we need to create and hold open. These benefits are greatest for installs running on toasters and gameboys, since serious installs should have low web/database latency and little difficulty handling connection counts, especially with the persistent flag (see D16913).

The major blocker here is that everywhere Phabricator references a table when constructing a query it needs to reference a full database_name.table_name instead, because the connection it acts on may be connected to a different database.

Something like ~60-ish% of these can be fixed with a couple of updates to PolicyAwareQuery and LiskDAO, but there will be a long tail of cases where we dropped down to direct database access and the fix isn't quite as clean.

I think the implementation pathway is something like this:

  • Define a new PhutilDatabaseTableReferenceInterface which provides getDatabaseName() / getTableName() methods.
  • Have LiskDAO implement that.
  • Have qsprintf() accept it with some new %R (or whatever) and produce a full database + table reference.
  • Maybe have a generic, concrete version of it for ad-hoc uses? Might simplify them?
  • Find all %T in the codebase, convert it to %R.
  • Implement an overlay flag into cluster.databases.

This will also let us pass table objects directly to %R instead of needing to do $thing->getTableName(), which is a small convenience benefit.

Event Timeline

tsy added a subscriber: tsy.Dec 1 2016, 6:17 PM

The next step here is to find (almost) every instance of %T in the codebase and convert it to %R. In most cases, the conversion is trivial:

@@ -1,4 +1,4 @@
 queryfx(
   $conn,
-  'SELECT * FROM %T ...',
+  'SELECT * FROM %R ...',
-  $object->getTableName());
+  $object);

In cases where the table is referenced with a constant, the change should usually look like this:

queryfx(
  $conn,
-  'SELECT * FROM %T ...',
+  'SELECT * FROM %R ...',
-  SomeClass::TABLE_XYZ,
+  SomeClass::newXYZDatabaseTableRef());

We might need a helper for defining newXYZDatabaseTableRef(), like LiskDAO->newRefForTableOnSameDatabase(...). Then we'd get something like:

final class DiffusionPathChange {

...

final public static functioon newPathChangeDatabaseTableRef() {
  return id(new PhabricatorRepository())
    ->newDatabaseTableRefOnSameDatabase(self::TABLE_NAME);
}

...

}

In cases where this is impractical or the %T is in a migration or something, a ref can be built inline/manually with:

new AphrontDatabaseTableRef('database_name', 'table_name');

For migrations, we may also need a mode to force connections not to overlay. If we can reasonably convert almost all of them to %R we're fine, but if this is a huge untestable mess it may be simpler to just say "no overlay for migrations". There is no practical benefit to supporting overlay during migrations so this would be a small loss (just some additional complexity because the mode would need to exist).