Page MenuHomePhabricator

GCloud SQL does not support configuring sql_mode
Closed, WontfixPublic

Description

AWS and GCE currently break Phabcricator (spitting out a warning about sql_mode and possibly errors down the line with 2FA) - basically, most managed SQL services without the "STRICT_ALL_TABLES" flag set in SQL.

It's fine if you run your own SQL, but using managed SQL (especially ones that you have limited access to) break. Thankfully, you can set SQL modes per-session instead of globally. That would mean adding a statement somewhere where we've just set the mode manually. This would make the warnings all but useless, unless somehow setting the session's mode failed.

I created a diff earlier which resides in the libphutil and shows an example of what that patch would look like. This may not be the correct place for it- my assumption is the whole thing should be moved to Phabricator itself somewhere, but I'm not familiar with the codebase enough to say exactly where.

Related: D16971: Add WP-like set_sql_mode to Aphront to avoid errors in managed SQL databases (such as AWS or GCE)

Event Timeline

egg82 created this task.Dec 1 2016, 8:33 PM

As far as I can tell, Amazon RDS allows configuration of sql_mode. Here's their announcement of support (from May 2016):

https://forums.aws.amazon.com/thread.jspa?threadID=232266

Here's the option in the web UI:

Before we pursue support upstream, I'd like to understand this:

  • Which specific managed services do not support sql_mode?
  • Have you requested that they support configuring sql_mode?

The patch in D16971 imposes a global performance penalty on all connections on all installs, and is not upstreamable.

It is possible that some more tailored version of it is, but a more likely upstreamable change is a documentation note that specific services which do not support sql_mode are not supported by Phabricator.

egg82 added a comment.Dec 1 2016, 9:57 PM

Specifically I'm looking at GCloud SQL, which does not yet allow you to use config options that require system-level changes. You can still use a dedicated SQL GCE instance, but using the managed SQL itself isn't yet possible without running into errors about the sql_mode.

There may be more, however. I'd recommend if doing that as an option to use a whitelist instead of a blacklist, stating that it is possible the service they are using isn't capable of the specified SQL mode.

What about adding another AphrontMySQLDatabaseConnection for known managed solutions triggering the specific mode query? Persistent connections may also help with the performance hit.

We aren't interested in introducing complexity into Phabricator to work around limitations in GCloud SQL at this time. Instead, you should ask Google to fix this limitation with their service.

Generally, the barrier to add workarounds to Phabricator for limitations in services provided by other companies is extremely high. These kinds of changes burden us with a complexity cost created by limitations in some other company's software. In most cases where we are asked to do this, the other company has thousands of times our resources and the user making the request is a paying customer of the other company, but uses Phabricator for free. I do not want to bear these costs just because the Phabricator upstream is more accessible.

I'll accept a patch to add a note to the documentation noting that GCloud SQL is not supported (in src/docs/contributor/database.diviner).

We can leave this open to collect support from other users who are interested in GCloud workarounds, but it is virtually inconceivable to me that this is something we would ever bring upstream in any form.

epriestley renamed this task from Making Phabricator more compatible with managed SQL services to GCloud SQL does not support configuring sql_mode.Dec 1 2016, 10:13 PM
epriestley triaged this task as Wishlist priority.
epriestley added projects: Setup, Infrastructure.
egg82 added a comment.Dec 1 2016, 10:26 PM

From what I understand it's something Google is working on. I've been dealing with the limitations myself for a while and it sucks, but hopefully it'll be fixed on their end sooner rather than later.

For now, I'd say adding some documentation is worthwhile. I'd say adding a whitelist of known good providers (docs providing resources to change modes and such) would be, in the long-term, a much simpler solution that adding a blacklist of known providers that cause problems as the hope is they'll eventually change.

epriestley closed this task as Wontfix.
epriestley claimed this task.

...it is virtually inconceivable to me that this is something we would ever bring upstream in any form.

Nothing has changed here in the last two years, so I don't anticipate moving forward on this.