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.

Sign up for GitHub

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

Jump to bottom

fix: database view performance #1091

Merged
merged 2 commits into from
Oct 19, 2023
Merged

fix: database view performance #1091

merged 2 commits into from
Oct 19, 2023

Conversation

bcgov-citz-ccft
Copy link
Collaborator

Description

Very slow performance of the database indicates that the view user_form_access_vw is running very slowly. This view is composed of multiple other views, two of which combine every form versus every user. There are a few things that can be done to improve the performance (and readability) of these views.

  1. Remove the join of "role" in user_form_permissions_vw.
  2. Remove the join of "role" in user_form_roles_vw.
  3. Simplify the EXISTS SELECT in user_form_permissions_vw.
  4. Simplify the EXISTS SELECT in user_form_roles_vw.
  5. Remove the sorting in the user_form_access_vw.

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist

  • I have read the CONTRIBUTING doc
  • I have checked that unit tests pass locally with my changes
  • I have run the npm script lint on the frontend and backend
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • I have approval from the product owner for the contribution in this pull request

@WalterMoar
fix: simplification of user form access view (#1087)
8f9528a
@WalterMoar
fix: database view performance (#1090)
9ede897 
* fix: simplification of user form access view

* fix: remove the sorting in user_form_access_vw
@github-actions
Copy link

Coverage Report (Application)

Totals Coverage
Statements: 41.89% ( 2636 / 6292 )
Methods: 36.84% ( 326 / 885 )
Lines: 47.43% ( 1759 / 3709 )
Branches: 32.45% ( 551 / 1698 )

@github-actions
Copy link

Coverage Report (Frontend)

Totals Coverage
Statements: 68.33% ( 1107 / 1620 )
Methods: 66.11% ( 199 / 301 )
Lines: 72.44% ( 778 / 1074 )
Branches: 53.06% ( 130 / 245 )

@bcvesalink
Copy link
Contributor

@WalterMoar do you think we should remove expensive sorting in auth api as well? Here https://github.com/bcgov/common-hosted-form-service/blob/master/app/src/forms/auth/service.js#L136 and here https://github.com/bcgov/common-hosted-form-service/blob/master/app/src/forms/auth/service.js#L141 ?

@WalterMoar
Copy link
Collaborator

@WalterMoar do you think we should remove expensive sorting in auth api as well? Here https://github.com/bcgov/common-hosted-form-service/blob/master/app/src/forms/auth/service.js#L136 and here https://github.com/bcgov/common-hosted-form-service/blob/master/app/src/forms/auth/service.js#L141 ?

We should definitely think about it. It won't affect the database load, though - only the load on the app pods. I think we have a lot of room for improvement, so let's keep that in mind!

@bcvesalink
Copy link
Contributor

@WalterMoar do you think we should remove expensive sorting in auth api as well? Here https://github.com/bcgov/common-hosted-form-service/blob/master/app/src/forms/auth/service.js#L136 and here https://github.com/bcgov/common-hosted-form-service/blob/master/app/src/forms/auth/service.js#L141 ?

We should definitely think about it. It won't affect the database load, though - only the load on the app pods. I think we have a lot of room for improvement, so let's keep that in mind!

I mean, we removed sorting in view, why would we need sorting in API?

@WalterMoar WalterMoar merged commit a9e6b7f into master Oct 19, 2023
19 checks passed
@WalterMoar WalterMoar deleted the fix/database-view-performance branch October 19, 2023 14:57
timisenco2015 added a commit that referenced this pull request Oct 20, 2023
@timisenco2015 @bcgov-citz-ccft
@WalterMoar @bcvesalink
update: update branch with upstream master branch (#1097)
9210ffb 
* fix: FORMS-891 vuetify traditional chinese support (#1072)

Co-authored-by: Walter Moar <[email protected]>

* fix: filter out draft submissions on the submission page search

* fix: remove boolean check for filterformSubmissionStatusCode props

* fix: urgent timeout increase to fix outage (#1084)

* Fix/urgent timeout increase (#1085)

* fix: urgent timeout increase to fix outage

* fix: database view performance (#1091)

* fix: simplification of user form access view (#1087)

* fix: database view performance (#1090)

* fix: simplification of user form access view

* fix: remove the sorting in user_form_access_vw

---------

Co-authored-by: Walter Moar <[email protected]>

* fix: FORMS-893 semver vuln and endpoint bug (#1094)

* FORMS-881 - Not allow to save drafts if form validation fails (#1048)

* fix: not allow to save drafts if form validation fails

* fix: make validation run on render for drafts. remove disable save draft button on validation fails

* update: add countdown timer to token expiration warning dialog

- add a countdown timer to the token expiration warning dialog
- wrote unit tests for the frontend changes
- added multi lang to the new components created

* feat: FORMS-882 add submissionId to exports

Co-authored-by: Walter Moar <[email protected]>

* fix: FORMS-899 revert timeout and simplify views (#1093)

* fix: FORMS-899 remove sorting in views

* fix: FORMS-899 revert frontend timeout change

---------

Co-authored-by: Walter Moar <[email protected]>

---------

Co-authored-by: bcgov-citz-ccft <[email protected]>
Co-authored-by: Walter Moar <[email protected]>
Co-authored-by: bcvesalink <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasonchung1871 jasonchung1871 jasonchung1871 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bcgov-citz-ccft @bcvesalink @WalterMoar @jasonchung1871