Closed Bug 461977 Opened 16 years ago Closed 15 years ago

Duplicate Signatures in top crashers db; number crashes not aggregated

Categories

(Socorro :: General, task)

task
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ozten, Unassigned)

References

()

Details

Attachments

(1 file)

The topcrashers page has duplicate signatures with low totals. 

It should have only 1 entry per signature + build. The totals should be higher and be the total across 2 weeks worth of crash data.
Did bug 426389 ever get pushed? The implication from the last dupe in that bug was no.
Yes 426389 got pushed. This bug is in the php layer's query.
This patch assumes a new index
 CREATE INDEX topcrashers_last_updated_build_version_product_key ON topcrashers (last_updated, build, version, product);
which I will add to socorro/database/schema.py
Attachment #345104 - Flags: review?(morgamic)
Attachment #345104 - Flags: review?(lorchard)
Here is my version of this code running against a copy of production top crashers table:

Prod Builds
http://aking.khan.mozilla.org/reporter/topcrasher/byversion/Firefox/3.0.3
Note - signature / build are unique.
JS_RestoreFrameChain in list only once
signature @0x0 is in list twice for build 2008092417 and 2008092414

Dev Builds:
http://aking.khan.mozilla.org/reporter/topcrasher/byversion/Firefox/3.0b5
http://aking.khan.mozilla.org/reporter/topcrasher/byversion/Firefox/3.1a2pre

Also Note:
I have added a WHERE SUM(total) > 0 to filter out "top crashers" with no crashes in the last 2 weeks. I don't this this is necessary on second thought, as it only cleans up older data, before the cron bug fix. I don't see any total = 0 since the night of 10/23, so I should probably remove this clause.

I don't see it link in the app, but we also support 
http://aking.khan.mozilla.org/reporter/topcrasher/bybranch/Firefox/1.9.1
All the links points to non-existing pages. Austin, could you please give working ones so I can have a look at?
Henrik, those links are behind the VPN and I'm guessing he was just linking so his reviewer could test. This will be testable by everyone when it moves to production.
Comment on attachment 345104 [details] [diff] [review]
Change to topcrashers model to aggregate table rows

Works for me.
Attachment #345104 - Flags: review?(morgamic) → review+
Henrik - we have a public stage server as well, but since topcrashers is broken for non-release builds (versions with multiple build ids) we are trying to get this out ASAP.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Austin, do we need the keyword push-needed or is it already live?
Added push-needed, thanks Henrik.
Fixed in revision 653.
I filed a Bug 462056 for release to prod.
Keywords: push-needed
(In reply to comment #11)
> Added push-needed, thanks Henrik.
> Fixed in revision 653.
> I filed a Bug 462056 for release to prod.

Bug 462056 seems to be security-protected. Don't say _why_, but _whether_ it was intentional.
(In reply to comment #12)
> Bug 462056 seems to be security-protected. Don't say _why_, but _whether_ it
> was intentional.

It's not. It's infra-protected as many bugs filed within the infra team are. This is expected and normal.
This fix is now live in prod. Thanks.
Looks definitely better but not fixed yet. The numbers still differ between the top crasher list and the reports table like you can see here:

http://crash-stats.mozilla.com/topcrasher/byversion/Firefox/3.1b1

Take entry 1 [imgRequest::NotifyProxyListener(imgRequestProxy*)] which should have 615 crashes. When clicking on that signature and opening the table view you can see that 1560 crashes are reported.

Finally we should have exact the same number of crash numbers listed.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Looks like we should do a couple of things:
1) in the conventional topcrasher view drop the build column from the view and the query -- this is likely limiting the result count by constraining on a random build
2) offer a secondary view from the topcrasher list page (which is a usability funny ha-ha right now) that lets you delve into a specific build (which I think would be useful for QA/Devs)

We should spend a little time updating the topcrasher home page as a part of this to lay it out a little better.
Austin, sorry I did not catch this in my review.  When we get our test suite set up we'll catch stuff like this automagically though. :)

Henrik - thanks for the feedback, we really appreciate it.
Comment on attachment 345104 [details] [diff] [review]
Change to topcrashers model to aggregate table rows

Belated r+ since I just noticed this in my review queue.  :)
Attachment #345104 - Flags: review?(lorchard) → review+
21:46 < ss> Hm
21:46 < ss> Why does 
http://crash-stats.mozilla.com/?do_query=1&product=Firefox&branch=1.9&version=Firefox%3A3.0.4&query_search=signature&query_type=contains&query=&date=&range_value=1&range_unit=weeks show 19579 crashes for the topcrash in 1 week
21:46 < ss> And http://crash-stats.mozilla.com/topcrasher/byversion/Firefox/3.0.4 show 9059 in the past two weeks
21:46 < ss> That seems not good

I'm guessing that's this bug? I'm back to using queries for topcrash reports since the topcrash pages are wildly inaccurate.
Assignee: ozten.bugs → nobody
Fixed with the refactor of materialized views.
Status: REOPENED → RESOLVED
Closed: 16 years ago15 years ago
Resolution: --- → FIXED
Component: Socorro → General
Product: Webtools → Socorro
You need to log in before you can comment on or make changes to this bug.