#3275: Slowness on Domain, Domain Info and Domain Requests - [NL]

[CocoByte]

Ticket 3275

Resolves #3275

Changes

• Revised GenericOrgFilter and FederalTypeFilter to be more efficient
• Fixtures for DomainRequests was updated to allow us to specify number of entries to load.

Context for reviewers

The admin tables for Domain Requests, Domains, and Domain Information were given a thorough check-up to tease out any remaining opportunities for optimization and to diagnose causes of slow load times.

Findings for root cause of slowness

We already have taken measures to optimize performance of the admin tables, so the cause of slowness this time around was determined to be two filters that were recently added and not optimized for performance -- GenericOrgfilter and FederalTypeFilter. These were added during the course of updating our model structure to use converted fields. However, they were making expensive database calls. They have been revised to be avoid making such calls.

Areas that were verified to be optimized

It is worth noting that the following structures/subroutines were examined and determined to NOT cause slowness. (Code was commented out down to a bare-bones load, then different areas where systematically added back in to see performance impact. Performance was tested for 100000 records. We might want to re-examine some of these areas if we see performance issues with larger data sets)

• Pagination (This came up alot in the Django admin community as being a cause for performance slowdown, but I did not see any significant slowdown yet for 100000 records)
• Django's notoriously expensive .count() method (This one came as a surprise for me -- impact on performance was negligible if I disabled the count. We might still want to disable it in the future for very large data sets but I left it alone for now.)
• Filters other than the two listed above (I inspected each filter for performance impact. Only the two listed above created significant slowdown)
• Sorting
  • It appears that sorting on non-indexed data isn't having significant negative impact on performance so far. We are doing OK on making sure we add indexes to our models.
  • The custom multisort structure had negligible impact on performance
• Pre-fetching is in place for foreign key data, so I did not see any obvious places to optimize there.

Setup

• This might be easiest to test locally, since you need to load the database with large datasets and you need compare performance across both old and new code
• Load a LOT of data (I used 100000, but smaller values might work, too) into your local database for DomainRequests, Domains, and DomainInformation (for DomainRequests, you can do this by modifying the value of "total_domain_requests_to_make" in the fixture and refreshing your docker OR use whatever other method works for you)

Code Review Verification Steps

Do the following for DomainRequest, Domain, and DomainInformation tables

• Compare load times of old code with load times of new code on large data sets. (Play around with filters, search, and sorting)
• Verify that load times on the new code is significantly faster.

As the original developer, I have

Satisfied acceptance criteria and met development standards

• Met the acceptance criteria, or will meet them in a subsequent PR
• Created/modified automated tests
• Update documentation in READMEs and/or onboarding guide

Ensured code standards are met (Original Developer)

• If any updated dependencies on Pipfile, also update dependencies in requirements.txt.
• Interactions with external systems are wrapped in try/except
• Error handling exists for unusual or missing values

Validated user-facing changes (if applicable)

• Tag @dotgov-designers in this PR's Reviewers for design review. If code is not user-facing, delete design reviewer checklist
• Verify new pages have been added to .pa11yci file so that they will be tested with our automated accessibility testing
• Checked keyboard navigability
• Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI)

As a code reviewer, I have

Reviewed, tested, and left feedback about the changes

• Pulled this branch locally and tested it
• Verified code meets all checks above. Address any checks that are not satisfied
• Reviewed this code and left comments. Indicate if comments must be addressed before code is merged
• Checked that all code is adequately covered by tests
• Verify migrations are valid and do not conflict with existing migrations

Validated user-facing changes as a developer

Note: Multiple code reviewers can share the checklists above, a second reviewer should not make a duplicate checklist. All checks should be checked before approving, even those labeled N/A.

• New pages have been added to .pa11yci file so that they will be tested with our automated accessibility testing
• Checked keyboard navigability
• Meets all designs and user flows provided by design/product
• Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI)
• (Rarely needed) Tested as both an analyst and applicant user

As a designer reviewer, I have

Verified that the changes match the design intention

• Checked that the design translated visually
• Checked behavior. Comment any found issues or broken flows.
• Checked different states (empty, one, some, error)
• Checked for landmarks, page heading structure, and links

Validated user-facing changes as a designer

• Checked keyboard navigability
• Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI)
• Tested with multiple browsers (check off which ones were used)
  • Chrome
  • Microsoft Edge
  • FireFox
  • Safari
• (Rarely needed) Tested as both an analyst and applicant user

References

• Code review best practices

Screenshots

[github-actions]

🥳 Successfully deployed to developer sandbox nl.

[github-actions]

🥳 Successfully deployed to developer sandbox nl.

[github-actions]

🥳 Successfully deployed to developer sandbox nl.

[github-actions]

🥳 Successfully deployed to developer sandbox nl.

[github-actions]

🥳 Successfully deployed to developer sandbox nl.

[github-actions]

🥳 Successfully deployed to developer sandbox nl.

[github-actions]

🥳 Successfully deployed to developer sandbox nl.

[zandercymatics]

Nearly there, just getting a 500 error and an error on startup with fixtures. Seems like typos for both. It seems like the code wasn't there on getgov-backup, but when I ported this over to getgov-za (which has staging data) this ran great

[github-actions]

🥳 Successfully deployed to developer sandbox nl.

duplicate feedback

[github-actions]

🥳 Successfully deployed to developer sandbox nl.

[github-actions]

🥳 Successfully deployed to developer sandbox nl.

[github-actions]

🥳 Successfully deployed to developer sandbox nl.

[zandercymatics]

I was able to confirm that everything was working great with the two changes I suggested above. Feel free to merge after you implement those two things - but not blocking as its a very small fix for the 500 error

Also, remember to lint

[github-actions]

🥳 Successfully deployed to developer sandbox nl.

[github-actions]

🥳 Successfully deployed to developer sandbox nl.

[github-actions]

🥳 Successfully deployed to developer sandbox nl.