Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cleanup): Improve delete orphans SQL query #527

Conversation

barthez
Copy link
Contributor

@barthez barthez commented Nov 26, 2021

Original query to delete orphans was terribly slow and was unable to finish within default 15s timeout. Instead of excluding all ids from union, we left join all 3 tables and select only rows that have no joined values in pact_publications or verifications.

Additionaly, the query was fixed for mysql. Whenever the Mysql error occures instead of embedded query, ids are fetched and then directly used in delete query.

[edit]

Added sql_enable_caller_logging configuration documentation as it was causing specs failures.

@barthez barthez force-pushed the improve-clean-incrementail-orphan-deletion branch 2 times, most recently from 4ade8ff to 87ce772 Compare November 26, 2021 22:44
Original query to delete orphans was terribly slow and was unable to
finish within default 15s timeout. Instead of excluding all ids from
union, we left join all 3 tables and select only rows that have no
joined values in pact_publications or verifications.

Additionaly, the query was fixed for mysql. Whenever the Mysql error
occures instead of embedded query, ids are fetched and then directly
used in delete query.
@barthez barthez force-pushed the improve-clean-incrementail-orphan-deletion branch from 87ce772 to 8e7e111 Compare November 26, 2021 23:13
@bethesque
Copy link
Member

bethesque commented Nov 27, 2021

You're a champion. Thank you so much. I'd fixed one of these super inefficient queries, but hadn't gotten around to doing the rest. Sorry about the broken build on master - it's been SUCH a week!

@bethesque bethesque merged commit 853354e into pact-foundation:master Nov 27, 2021
@barthez barthez deleted the improve-clean-incrementail-orphan-deletion branch November 27, 2021 08:27
@bethesque
Copy link
Member

I've put out a new release with this PR.

@bethesque
Copy link
Member

@barthez would you mind accepting my connection request on LinkedIn? I'd love to have a chat!

@bethesque
Copy link
Member

I need to revert this PR because I believe it has caused the following errors in our clean logs.

message->PG::ForeignKeyViolation: 
DETAIL:  Key (id)=(9901) is still referenced from table "verifications".: DELETE FROM "pact_versions" WHERE ("id" IN (SELECT "pact_versions"."id" FROM "pact_versions" LEFT JOIN "pact_publications" ON ("pact_publications"."pact_version_id" = "pact_versions"."id") LEFT JOIN "verifications" ON ("verifications"."pact_version_id" = "pact_publications"."id") WHERE (("pact_publications"."id" IS NULL) AND ("verifications"."id" IS NULL))))

There was obviously some scenario not covered in the tests. I'll try and have a look into it, but not sure when that will be yet.

bethesque added a commit that referenced this pull request Dec 2, 2021
bethesque added a commit that referenced this pull request Dec 2, 2021
bethesque pushed a commit that referenced this pull request Dec 2, 2021
* fix(cleanup): Improve delete orphans SQL query

Original query to delete orphans was terribly slow and was unable to
finish within default 15s timeout. Instead of excluding all ids from
union, we left join all 3 tables and select only rows that have no
joined values in pact_publications or verifications.

Additionaly, the query was fixed for mysql. Whenever the Mysql error
occures instead of embedded query, ids are fetched and then directly
used in delete query.

* fix: Add sql_enable_caller_logging documentation

* fix: Fix unreliable spec
bethesque added a commit that referenced this pull request Dec 2, 2021
* fix(cleanup): Improve delete orphans SQL query (#527)

* fix(cleanup): Improve delete orphans SQL query

Original query to delete orphans was terribly slow and was unable to
finish within default 15s timeout. Instead of excluding all ids from
union, we left join all 3 tables and select only rows that have no
joined values in pact_publications or verifications.

Additionaly, the query was fixed for mysql. Whenever the Mysql error
occures instead of embedded query, ids are fetched and then directly
used in delete query.

* fix: Add sql_enable_caller_logging documentation

* fix: Fix unreliable spec

* fix: fully qualify pact_version table joins in database clean

Co-authored-by: Bartek Bułat <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants