Bedrock doesn't allow concurrent schema changes

[plaublin]

@quinthar It is currently not possible to create a table once the system has started:

$ nc localhost 8000
Query: create table foobar ( foo int, bar int );

502 Query failed
error: cannot modify database schema within CONCURRENT transaction

Bedrock creates a CONCURRENT transaction for every query, which doesn't allow schema changes. Replacing the CONCURRENT transaction by a normal one (by replacing line 85 in BedrockCore.cpp from if (!_db.beginConcurrentTransaction()) to if (!_db.beginTransaction()) fixes the issue. However this change affects every transaction.

[mcnamamj]

Thanks for your report! We're undergoing a massive change that's unintentionally preventing that from working, but do think we'd like to support this functionality.

In the meantime, if you're looking for a non-code based work around, for our existing Bedrock deployments we just take down one node at a time (in a 4+ node cluster you can do this without incurring production downtime), use sqlite3 command line tools to go into interactive mode on the db and apply the schema change, then bring the node back up to replicate before taking down the next node. In a non-production environment or one with 3 nodes, you would just take down all the nodes, apply the change to each bedrock.db file, and then bring them back up.

[quinthar]

@tylerkaraszewski is this still a limitation of CONCURRENT transactions?

[tylerkaraszewski]

Unless sqlite changed this without notifying us, then yes, it's still a limitation. I assume that's the case as I don't see them making this change without being asked specifically for it.

[treps]

Hm, so the consequence is right now, Bedrock::DB can only change schema manually using the command-line? That... sucks. Is there a way to determine that a query has schema changes (eg, when we get this error) and then re-execute the command with a non-concurrent transaction? I'm not sure how concurrent and non-concurrent transactions play together.

On Wed, Aug 15, 2018 at 10:41 AM Tyler Karaszewski notifications@github.com wrote:

Unless sqlite changed this without notifying us, then yes, it's still a limitation. I assume that's the case as I don't see them making this change without being asked specifically for it.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Expensify/Bedrock/issues/218#issuecomment-413276179, or mute the thread https://github.com/notifications/unsubscribe-auth/ADT7wO81-VWAsiRSjRbDzviPK8IQ_-_Wks5uRF0sgaJpZM4N5i2- .

[tylerkaraszewski]

What do we need this for?

[treps]

Well we don't need it internally as schema changes are a "big deal" at our scale and need to be done manually with a bunch of planning. But for a smaller deployment (ie, by the community), being able to do schema changes just like any other SQL would be cool. I'm mostly curious how difficult it would be to do, not saying we need to do it. What we might do is update this issue with hints for how someone in the community could do it as well.

On Wed, Aug 15, 2018 at 10:53 AM Tyler Karaszewski notifications@github.com wrote:

What do we need this for?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Expensify/Bedrock/issues/218#issuecomment-413280022, or mute the thread https://github.com/notifications/unsubscribe-auth/ADT7wE9loxSrHJ1rLCU0QFKV6tVKW39Vks5uRGARgaJpZM4N5i2- .

[tylerkaraszewski]

The idea behind concurrent commits is that they're automatically retryable.

If these two queries run simultaneously:

UPDATE table SET column = 1 WHERE key = 'A';

and

UPDATE table SET column = 2 WHERE key = 'A';

If they happen at the same time they conflict, and the second one is automatically retried and can succeed. At the end of the whole process, the value for column is either 1 or 2 for the row with the key A, but both queries succeed.

If you allow schema changes and run these two queries:

UPDATE table SET column = 1 WHERE key = 'A';

and

ALTER TABLE table RENAME to newTableName;

What happens when the second query finishes first? The first one conflicts, is automatically retried, and fails because the table it wants to write to doesn't exist.

Sure, we could detect queries that are going to change the DB schema and run them non-concurrently, but then that just exposes a host of other problems (like above) that we need to come up with solutions for.

[cannikin]

It's been a couple of years, has there been any updates on this issue?

We're using Prisma and the idea is to track changes to your database schema in code and be able to apply those changes in a known, repeatable manner both in development at deploy time. This same process is also part of Ruby on Rails. At deploy time any outstanding database migrations are run and new code referencing those changes is made live.

Changing the format of existing tables that are in use by the application in the middle of a deploy (someone executing a query against the "old" schema after the migration has run but before the code supporting that change is live) and the query will fail causing errors is a known issue, but is a generally accepted risk. This can be mitigated against, for example, by putting the site into maintenance mode.

If it's a node synchronization issue we can be sure that the migrations are only every applied to a single node, although it sounds like from @mcnamamj's comment that you actually need to apply DDL statements to each node individually?

[quinthar]

Hi! So sorry for the delay on this. A fix was just merged into master; our friends at SQLite have upgraded BEGIN CONCURRENT to allow schema changes, so now it should work. Thanks for asking!

-david

On Wed, Jul 1, 2020 at 3:02 PM Rob Cameron notifications@github.com wrote:

It's been a couple of years, has there been any updates on this issue?

We're using Prisma https://www.prisma.io/docs/reference/tools-and-interfaces/prisma-migrate and the idea is to track changes to your database schema in code and be able to apply those changes in a known, repeatable manner both in development at deploy time. This same process is also part of Ruby on Rails https://guides.rubyonrails.org/active_record_migrations.html. At deploy time any outstanding database migrations are run and new code referencing those changes is made live.

Changing the format of existing tables that are in use by the application in the middle of a deploy (someone executing a query against the "old" schema after the migration has run but before the code supporting that change is live) and the query will fail causing errors is a known issue, but is a generally accepted risk. This can be mitigated against, for example, by putting the site into maintenance mode.

If it's a node synchronization issue we can be sure that the migrations are only every applied to a single node, although it sounds like from @mcnamamj https://github.com/mcnamamj's comment that you actually need to apply DDL statements to each node individually?

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/Expensify/Bedrock/issues/218#issuecomment-652668068, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEMNUTPNYOF2BWDF6XEQCDRZOW7PANCNFSM4DPGFW7A .

[janpio]

(Prisma here, which @cannikin mentioned) Saw the new release that probably includes this, but we are unfortunately blocked by another error right now so we can not really test if this now works: https://github.com/Expensify/Bedrock/issues/818