Optimize database foreign keys references - MySQL/MariaDB

[tiagolima90]

What does it do?

Optimize the "getForeignKeys" when synchronizing with the database.

Why is it needed?

Using Strapi in a MySQL 5.7 database with lots of schemas and tables with foreign keys, the SQL query to get the foreign keys using inner joins is painfully slow, leading to the error "Knex: Timeout acquiring a connection. The pool is probably full. Are you missing a .transacting(trx) call?" on every single content type update.

How to test it?

Trigger database synchronization on startup updating any content type.

Related issue(s)/PR(s)

#11860

[strapi-cla]

All committers have signed the CLA.

[derrickmehaffy]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.

@tiagolima90 can you sign the CLA before the engineers take a look? Thank you!

[tiagolima90]

@derrickmehaffy hi. CLA signed. Thanks.

[codecov]

Codecov Report

Merging #12843 (01e38c1) into master (02768b6) will decrease coverage by 5.68%.
The diff coverage is n/a.

❗ Current head 01e38c1 differs from pull request most recent head ce27863. Consider uploading reports for the commit ce27863 to get more accurate results

@@            Coverage Diff             @@
##           master   #12843      +/-   ##
==========================================
- Coverage   54.25%   48.56%   -5.69%     
==========================================
  Files        1198      242     -956     
  Lines       30598     8940   -21658     
  Branches     5571     2006    -3565     
==========================================
- Hits        16601     4342   -12259     
+ Misses      12179     3789    -8390     
+ Partials     1818      809    -1009     

Flag	Coverage Δ
front	?
unit	48.56% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...gsPage/pages/Webhooks/EditView/utils/formatData.js
...ges/EditSettingsView/components/DisplayedFields.js
...ssions/utils/createDefaultPluginsFormFromLayout.js
...fosPage/components/LogoModalStepper/FromUrlForm.js
...kages/core/upload/admin/src/utils/getRequestUrl.js
...nents/SingleTypeFormWrapper/utils/getRequestUrl.js
...n/admin/src/components/ModalCreate/AdvancedForm.js
...dlewares/addCommonFieldsToInitialDataMiddleware.js
...ages/SettingsPage/pages/Webhooks/ListView/index.js
...n/src/pages/AdvancedSettings/tests/utils/server.js
... and 946 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 02768b6...ce27863. Read the comment docs.

[tiagolima90]

Hi,

Did you have a chance to check this PR?

Currently I have to use my forked version to work with strapi 4, because on every single change I get a knex timeout on restart.

Thank you.

Tiago Lima

[strapi-bot]

This pull request has been mentioned on Strapi Community Forum. There might be relevant details there:

https://forum.strapi.io/t/strapi-postgres-connection-pool-breaks-connection/17926/16

[derrickmehaffy]

I noticed this only really is being applied to MySQL/MariaDB and doesn't address possible issues in PostgreSQL and SQLite too maybe?

[tiagolima90]

Hi,

Since I use MySQL at work, I focused on that dialect.

Since the changes are made only on this dialect, it shouldn't break anything for PostgreSQL and SQLite.

If those dialects are having the same issues, you can try to apply the same fixes. The only thing I have done was to query each table separately and merge the info later.
The joins is what leads to the timeout when the instance have a lot of foreign keys across the schemas.

I have been using this fix since I have made this PR and is working perfectly with MySQL. No timeouts since then.

[esiao]

I can confirm that this issue exists on PostgreSQL too.

I came across this PR because I do experience the same issue on a PostgreSQL Database hosted on Digital Ocean Managed Databases.

As a matter of fact, I do have two projects there, one has few relations and database tables (~80) where I never had any issues so far. The other one has multiple relations and tables (~190) and it fails their automated health check when deploying because the DB work isn't done yet.
My workaround has been to restore the new relations and columns by connecting to the database directly.

[meherchandan]

Has anyone resolved it on postgres?

[meherchandan]

I was facing the same issue when adding a new component on strapi v4 on existing database. I have resolved it by applying the similar logic. Now no more knex error for me. i tried to raise a PR but looks like I am not authorize to push a new branch. Patch has been posted on.
https://forum.strapi.io/t/strapi-postgres-connection-pool-breaks-connection/17926/24?u=meher_chandan

[esiao]

@meherchandan, thank you. I've tried applying your patch but ran into this server starting error:

[2022-06-13 16:32:24] [2022-06-13 16:32:24.004] debug: ⛔️ Server wasn't able to start properly.
[2022-06-13 16:32:24] [2022-06-13 16:32:24.005] error: insert into "admin_permissions" ("action", "conditions", "created_at", "properties", "subject", "updated_at") values ($1, $2, $3, $4, $5, $6) returning "id" - duplicate key value violates unique constraint "admin_permissions_pkey"
[2022-06-13 16:32:24] error: insert into "admin_permissions" ("action", "conditions", "created_at", "properties", "subject", "updated_at") values ($1, $2, $3, $4, $5, $6) returning "id" - duplicate key value violates unique constraint "admin_permissions_pkey"
[2022-06-13 16:32:24]     at Parser.parseErrorMessage (/workspace/node_modules/pg-protocol/dist/parser.js:287:98)
[2022-06-13 16:32:24]     at Parser.handlePacket (/workspace/node_modules/pg-protocol/dist/parser.js:126:29)
[2022-06-13 16:32:24]     at Parser.parse (/workspace/node_modules/pg-protocol/dist/parser.js:39:38)
[2022-06-13 16:32:24]     at TLSSocket.<anonymous> (/workspace/node_modules/pg-protocol/dist/index.js:11:42)
[2022-06-13 16:32:24]     at TLSSocket.emit (node:events:527:28)
[2022-06-13 16:32:24]     at TLSSocket.emit (node:domain:475:12)
[2022-06-13 16:32:24]     at addChunk (node:internal/streams/readable:315:12)
[2022-06-13 16:32:24]     at readableAddChunk (node:internal/streams/readable:289:9)
[2022-06-13 16:32:24]     at TLSSocket.Readable.push (node:internal/streams/readable:228:10)
[2022-06-13 16:32:24]     at TLSWrap.onStreamRead (node:internal/stream_base_commons:190:23)
[2022-06-13 16:32:24] error Command failed with exit code 1.
[2022-06-13 16:32:24] info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
[]

I suppose a check must be made for the permissions table or probably check all tables with an IF exists condition?

[derrickmehaffy]

I was facing the same issue when adding a new component on strapi v4 on existing database. I have resolved it by applying the similar logic. Now no more knex error for me. i tried to raise a PR but looks like I am not authorize to push a new branch. Patch has been posted on. https://forum.strapi.io/t/strapi-postgres-connection-pool-breaks-connection/17926/24?u=meher_chandan

You need to make a fork and submit the PR from the fork.

[meherchandan]

I was facing the same issue when adding a new component on strapi v4 on existing database. I have resolved it by applying the similar logic. Now no more knex error for me. i tried to raise a PR but looks like I am not authorize to push a new branch. Patch has been posted on. https://forum.strapi.io/t/strapi-postgres-connection-pool-breaks-connection/17926/24?u=meher_chandan

You need to make a fork and submit the PR from the fork.

Thank you. Created the PR #13550

[meherchandan]

@meherchandan, thank you. I've tried applying your patch but ran into this server starting error:

[2022-06-13 16:32:24] [2022-06-13 16:32:24.004] debug: ⛔️ Server wasn't able to start properly.
[2022-06-13 16:32:24] [2022-06-13 16:32:24.005] error: insert into "admin_permissions" ("action", "conditions", "created_at", "properties", "subject", "updated_at") values ($1, $2, $3, $4, $5, $6) returning "id" - duplicate key value violates unique constraint "admin_permissions_pkey"
[2022-06-13 16:32:24] error: insert into "admin_permissions" ("action", "conditions", "created_at", "properties", "subject", "updated_at") values ($1, $2, $3, $4, $5, $6) returning "id" - duplicate key value violates unique constraint "admin_permissions_pkey"
[2022-06-13 16:32:24]     at Parser.parseErrorMessage (/workspace/node_modules/pg-protocol/dist/parser.js:287:98)
[2022-06-13 16:32:24]     at Parser.handlePacket (/workspace/node_modules/pg-protocol/dist/parser.js:126:29)
[2022-06-13 16:32:24]     at Parser.parse (/workspace/node_modules/pg-protocol/dist/parser.js:39:38)
[2022-06-13 16:32:24]     at TLSSocket.<anonymous> (/workspace/node_modules/pg-protocol/dist/index.js:11:42)
[2022-06-13 16:32:24]     at TLSSocket.emit (node:events:527:28)
[2022-06-13 16:32:24]     at TLSSocket.emit (node:domain:475:12)
[2022-06-13 16:32:24]     at addChunk (node:internal/streams/readable:315:12)
[2022-06-13 16:32:24]     at readableAddChunk (node:internal/streams/readable:289:9)
[2022-06-13 16:32:24]     at TLSSocket.Readable.push (node:internal/streams/readable:228:10)
[2022-06-13 16:32:24]     at TLSWrap.onStreamRead (node:internal/stream_base_commons:190:23)
[2022-06-13 16:32:24] error Command failed with exit code 1.
[2022-06-13 16:32:24] info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
[]

I suppose a check must be made for the permissions table or probably check all tables with an IF exists condition?

Can you try the updated patch on same forum post. For me this is working fine on prod server as well.

[derrickmehaffy]

patches-12843.zip
^ Patch files based on v4.2.0

[derrickmehaffy]

Initial testing, I'm seeing the initial cold boot time drop by half, first time I've ever seen a Strapi app boot in less than a second.....

[derrickmehaffy]

Waiting for automated tests to go through but this is a HUGE improvement.

So far it LGTM but we need @alexandrebodin to review as well.
Something @sunnysonx mentioned though he still has some performance issues and might have an additional fix to improve this further and we might want to consider moving away from using the information schema in the future.

[alexandrebodin]

LGTM

[alexandrebodin]

Great contribution. That a good step for optimization. We can still work on optimization concurrency and pooling but that's great 👍

[strapi-bot]

This pull request has been mentioned on Strapi Community Forum. There might be relevant details there:

https://forum.strapi.io/t/new-release-strapi-v4-2-2/19927/1