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

NAPPS-2280: fix deletion timeout #110

Merged
merged 3 commits into from
Oct 3, 2023
Merged

Conversation

Kat-Alo
Copy link
Collaborator

@Kat-Alo Kat-Alo commented Sep 28, 2023

On dev and prod, deleting pages with lots of snapshots results in a 504 timeout error. My suspicion is that it's because the Rails deletion of records with children is a very costly operation.

Luckily, folks at an org called Census had a very similar problem and created a little tool they call Miss Hannigan. The tool nullifies the relationship between the children snapshots and the parent page and purges the orphaned children records ansynchronously.

If we are concerned about relying on this particular (seemingly little-known) gem, we could port some of the logic into Klaxon — not sure what that protocol looks like.

To test

  • docker-compose down && docker-compose build && docker-compose up
  • sign at http://localhost:3001/klaxon/ (you won't receive a sign-in email since you're running things locally. Look for the HTML of the body of the email in your terminal to find the link w the sign-in token)
  • Add a page that will change frequently, so we know it'll generate a snapshot
  • Wait for a snapshot to be generated
  • in a new terminal...
    • klaxon-psql
    • \c klaxon;
    • select page_id from page_snapshots; (to confirm a snapshot has been generated for a page)
  • Back in the UI, delete the page
  • Return to the terminal window with the psql shell and confirm the snapshot you saw is no longer there

The real test will be getting these changes to dev and confirming we can delete the big record that's been causing a timeout. Then, we'll be able to open a PR with the original Klaxon.

@Kat-Alo Kat-Alo requested a review from a team as a code owner September 28, 2023 22:33
@Kat-Alo Kat-Alo requested review from jtotoole and aaronbrezel and removed request for a team September 28, 2023 22:33
@Kat-Alo
Copy link
Collaborator Author

Kat-Alo commented Sep 28, 2023

Just a note that I'm not sure what these tests are given that I didn't think we had tests in our workflow for Klaxon yet?? 🤔

@Kat-Alo
Copy link
Collaborator Author

Kat-Alo commented Sep 29, 2023

Also for posterity: I opened an issue with the original Klaxon.

Copy link

@jtotoole jtotoole left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kat-Alo Kat-Alo merged commit fd0ceb3 into develop Oct 3, 2023
1 check passed
@Kat-Alo Kat-Alo deleted the napps-2280-fix-deletion-timeout branch October 3, 2023 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants