Skip to content
This repository has been archived by the owner on Jan 17, 2023. It is now read-only.

Deleting all shots works, but shows a 504 #3568

Closed
jaredhirsch opened this issue Sep 29, 2017 · 4 comments
Closed

Deleting all shots works, but shows a 504 #3568

jaredhirsch opened this issue Sep 29, 2017 · 4 comments
Assignees
Milestone

Comments

@jaredhirsch
Copy link
Member

jaredhirsch commented Sep 29, 2017

STR:

  1. create a screenshot in a test account
  2. visit screenshots.firefox.com/leave-screenshots
  3. click the 'proceed' button

Expected: shots are deleted

Actual: shots are deleted, but the page hangs for 60 seconds, then shows an nginx 504 Gateway Time-out error.

@jaredhirsch jaredhirsch changed the title can't delete shots can't delete all shots Sep 29, 2017
@jaredhirsch
Copy link
Member Author

jaredhirsch commented Sep 29, 2017

Note: If you just sit at the page till it times out, the shots should be gone next time you visit https://screenshots.firefox.com/shots

Workaround: deleting individual shots works fine. Users can remove all their shots one by one.

@jaredhirsch jaredhirsch changed the title can't delete all shots Deleting all shots works, but shows a 504 Sep 29, 2017
@ianb
Copy link
Contributor

ianb commented Sep 29, 2017

I see this query in Shot.deleteEverythingForDevice that looks like the one that caused a problem on My Shots:

    return db.select(
      `SELECT DISTINCT devices.id
       FROM devices, devices AS devices2
       WHERE devices.id = $1
             OR (devices.accountid = devices2.accountid
                 AND devices2.id = $1)
      `,
      [deviceId]);

@SoftVision-CosminMuntean

I have managed to reproduce this issue on latest Nightly (58.0a1) build with Screenshots 19.0.0 and also on Firefox 56.0 release with Screenshots 10.12.0 .

But, it seems that the issue is not reproducible using Screenshots (22.0.0) stage/dev versions installed from here. Probably the issue is only reproducible on production server?

@ghost ghost added this to the Launch 58 milestone Oct 2, 2017
@chenba chenba self-assigned this Oct 11, 2017
chenba added a commit to chenba/screenshots that referenced this issue Oct 12, 2017
chenba added a commit to chenba/screenshots that referenced this issue Oct 12, 2017
chenba added a commit to chenba/screenshots that referenced this issue Oct 12, 2017
@chenba
Copy link
Collaborator

chenba commented Oct 12, 2017

A possible reason why this is happening on prod but not reproducible elsewhere is the size of the database. The patch removes three sequential scans and a sort from the SQL queries involved in deleting all shots.

ianb pushed a commit that referenced this issue Oct 18, 2017
* Remove two seq scans and a sort from a select query. (#3568)
* Add index to column that's used in a WHERE.
* Update db schema.
* Up the DB level.
* Use acct id to get device ids; del img data for all device ids.
* Use device id when there's no account id to delete shots.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants