665251 - 500 / Internal Server Errors on Elastic-Search-running staging

Status: VERIFIED FIXED

(Socorro :: General, task)

Description

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Logfile

The new Elastic Search integration is throwing a ton of 500 Internal Server Errors on staging:

https://crash-stats.allizom.org/products/Firefox

Will include output from PowerFuzzer.

Comment 1

[[DEACTIVATED] Adrian Gaudebert]

Attachment: Fixing bug 665251 + securing data

This patch fixes the first programming error, and also fixes some security issues by encoding the data that goes into the query_string.

This was tested on khan with some of the URIs given by stephend, it seems to be ok now. Btw http://agaudebert.khan.mozilla.org/query is supposed to be the same as crash-stats.allizom.org so you can test here before the patch is applied.

Comment 2

[Robert Helmer [:rhelmer]]

Comment on attachment 540203 [details] [diff] [review]
Fixing bug 665251 + securing data

Committed revision 3228.

Comment 3

[[DEACTIVATED] Adrian Gaudebert]

@Stephen: did you test that again? Is this fixed?

Comment 4

[Matt Brandt [:mbrandt]]

adrian, if the patch is ready to be qa'd can you bump the bug to resolved/fixed

Comment 5

[[DEACTIVATED] Adrian Gaudebert]

Yep! :)

Comment 6

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #3)
> @Stephen: did you test that again? Is this fixed?

I tested again on both https://crash-stats-dev.allizom.org/query and https://crash-stats.allizom.org/query, and there are tons of 500s, still.

Reopening; on which should I be looking?

Comment 7

[[DEACTIVATED] Adrian Gaudebert]

@Stephen: https://crash-stats.allizom.org should be OK now, can you run your tests against it?

I have some problems with the Postgres part, queries time out, but the 500 problem should be fixed too... I'm working on it with jberkus, I'll let you know when it's ok. Hopefully tomorrow.

Comment 8

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #7)
> @Stephen: https://crash-stats.allizom.org should be OK now, can you run your
> tests against it?
> 
> I have some problems with the Postgres part, queries time out, but the 500
> problem should be fixed too... I'm working on it with jberkus, I'll let you
> know when it's ok. Hopefully tomorrow.

http://pastebin.mozilla.org/1259479 is what I'm still seeing.

Comment 9

[Laura Thomson :laura]

Adrian, status update?

Stephend, should this block ship?  We'd be shipping the pg version.

Comment 10

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #9)
> Adrian, status update?
> 
> Stephend, should this block ship?  We'd be shipping the pg version.

I don't fully understand the implications of shipping the Postgres version, so can't make that call.  Do you mean we wouldn't be using elasticsearch at all, this time?

Comment 11

[Robert Helmer [:rhelmer]]

(In reply to comment #10)
> (In reply to comment #9)
> > Adrian, status update?
> > 
> > Stephend, should this block ship?  We'd be shipping the pg version.
> 
> I don't fully understand the implications of shipping the Postgres version,
> so can't make that call.  Do you mean we wouldn't be using elasticsearch at
> all, this time?

We won't be using Elastic Search until the production boxes for it are ready, which will not be at least until tomorrow, so we have been planning to ship the new code with the configuration pointing everything to PG.

After discussing with Adrian in irc, at least one of the problems here is a severe performance regression (using an unindexed column on the very large reports table in where/order by) and at least one other minor bug, so we're holding the release on this. He's working on it now.

Comment 12

[[DEACTIVATED] Adrian Gaudebert]

Attachment: Fixing 500 Errors with Postgres + optimizing queries

Fix multiple things in the Postgres implementation:

* Fix 500 errors due to programming errors
* Optimize queries so it doesn't time out more than before

Tested on khan ( http://agaudebert.khan.mozilla.org/query ) with Powerfuzzer, no more 500 Errors reported.

Comment 13

[Matt Brandt [:mbrandt]]

Per conversation with adrian... this is ready to be resolved/fixed.

Ready for QA

Comment 14

[Matt Brandt [:mbrandt]]

Re-openned... power fuzzer is still discovering 500s. Adrian has the latest output.

Comment 15

[[DEACTIVATED] Adrian Gaudebert]

Attachment: Fixing 500 Errors with Postgres (securing user inputs)

'build_id' and 'reason' were assumed to be strings when they actually could be lists. This patch fixes this problem, and even extends the functionalities by allowing to specify several build ids or crash reasons.

Comment 16

[Robert Helmer [:rhelmer]]

Comment on attachment 542665 [details] [diff] [review]
Fixing 500 Errors with Postgres (securing user inputs)

Landed on trunk and branch:
3251, 3252

Comment 17

[Matt Brandt [:mbrandt]]

Bumping to resolved/fixed per a conversation on irc. The patch has landed on stage and is ready for QA.

Comment 18

[Matt Brandt [:mbrandt]]

Attachment: Missing platform counts in query results

Reopening. I'm working with adrian to test this. The two behaviors we're seeing:
1. The fuzzer is still finding 500's, ex: https://crash-stats.allizom.org/query/query?query=%2Fe%00&do_query=1&query_type=simple
2. The query results table appears to be missing results for the platform counts.

Comment 19

[[DEACTIVATED] Adrian Gaudebert]

Attachment: Fixing config + regression + 500 errors on stage

This patch fixes:

* A problem with the platforms config leading to not counting for Windows
* A regression that did not show the type of crash (browser, plugin, hang... )
* An issue with Apache refusing encoded slashes (%2F) in URLS

Comment 20

[Robert Helmer [:rhelmer]]

Comment on attachment 543001 [details] [diff] [review]
Fixing config + regression + 500 errors on stage

Landed on trunk: 3255
And branch: 3256

Comment 21

[[DEACTIVATED] Adrian Gaudebert]

Attachment: Fixing crash reason + crash type

Not really related to this bug, but I found 2 new bugs while testing my code: 

* crash_reason was called reason when calling the API, resulting in not filtering by crash reason ;
* the API did not return the types of crashes (browser, hang, plugin... )

This patch fixes those issues.

Comment 22

[[DEACTIVATED] Adrian Gaudebert]

Attachment: Fix search in plugins

The mode "starts_with" for searching in plugins doesn't work, this patch fixes it.

Comment 23

[Robert Helmer [:rhelmer]]

Comment on attachment 543209 [details] [diff] [review]
Fix search in plugins

committed 3260/3261

Comment 24

[Matt Brandt [:mbrandt]]

bumping to resolved/fixed per discussion in irc with adrian.

Comment 25

[Robert Helmer [:rhelmer]]

FYI accidentally used this bug number when committing rev 3263/3264 for the related bug 668621, note to any future archaeologists :)

Comment 26

[Matt Brandt [:mbrandt]]

adrian and rhelmer, thanks for the incredible amount of work done over the last couple of days to get this bug squashed. QA verified, ran the powerfuzzer and 0 500 errors. UI looks great.