561235 - Make Talos use mozcrash for minidump processing

Status: RESOLVED FIXED

(Testing :: Talos, defect, P4)

Description

[Chris AtLee [:catlee]]

We could save a lot of time and bandwidth if we only download symbols on crash.

This would require changes to talos, so it could be passed a URL to download and unpack.

Comment 1

[Nick Thomas [:nthomas] (UTC+12)]

Still needed if the symbol files are 2.0MB ?

Comment 2

[Chris AtLee [:catlee]]

Symbols for OSX are still 15MB.  Not as critical to do right away, but would be nice to do soon.

Comment 3

[Chris AtLee [:catlee]]

We should make use of the cgi that's going to be set up to handle this.

Comment 4

[(not currently active) Ted Mielczarek]

I'm going to write some Python code in bug 563745 to do this, so we can probably just copy and paste it to Talos for now.

Comment 5

[(not currently active) Ted Mielczarek]

I wrote a test script:
http://hg.mozilla.org/users/tmielczarek_mozilla.com/minidump-stackwalk-cgi/file/9c6291a68500/testsubmit.py

and wound up using that code almost verbatim in the unittest harnesses.

Comment 6

[Armen [:armenzg]]

What would it be required to make this happen? My comprehension is limited on this area.

Comment 7

[(not currently active) Ted Mielczarek]

I think we should go with the approach I described in comment 4 and comment 5. I wrote a CGI that accepts a minidump + a URL to symbols, and produces a stack trace. This way, the slaves don't have to download anything, only the server does (and it can cache symbol so it only has to download them once).

Comment 8

[cmtalbert]

(In reply to comment #7)
> I think we should go with the approach I described in comment 4 and comment
> 5. I wrote a CGI that accepts a minidump + a URL to symbols, and produces a
> stack trace. This way, the slaves don't have to download anything, only the
> server does (and it can cache symbol so it only has to download them once).

I agree that's a fine solution.  What are the steps to roll this into production?

Comment 9

[Chris AtLee [:catlee]]

(In reply to comment #8)
> (In reply to comment #7)
> > I think we should go with the approach I described in comment 4 and comment
> > 5. I wrote a CGI that accepts a minidump + a URL to symbols, and produces a
> > stack trace. This way, the slaves don't have to download anything, only the
> > server does (and it can cache symbol so it only has to download them once).
> 
> I agree that's a fine solution.  What are the steps to roll this into
> production?

1) need a host to put said CGI on
2) test harnesses need to know how to interact with the  CGI
3) change buildbot code to stop downloading/unpacking of symbols and instead pass symbol URL and CGI URL to test harnesses
4) ???
5) profit!

Comment 10

[(not currently active) Ted Mielczarek]

2 is fixed for unittests, but not for Talos. Should be easy enough to port the automation.py code to Talos.

Comment 11

[Chris AtLee [:catlee]]

(In reply to comment #10)
> 2 is fixed for unittests, but not for Talos. Should be easy enough to port
> the automation.py code to Talos.

I wonder about failover behaviour too here. Should we retry, or specify an alternate server to talk to, or just accept that sometimes the CGI won't be available?

Comment 12

[(not currently active) Ted Mielczarek]

If we were to fix bug 642167, it might not be a big deal if the CGI doesn't respond.

Comment 13

[(not currently active) Ted Mielczarek]

Porting the unittest code to Talos would just involve reusing the code here:
http://mxr.mozilla.org/mozilla-central/source/build/automationutils.py#103

Comment 14

[William Lachance (:wlach)]

Attachment: Add support to talos for use of CGI crashhandler

So here's a first cut at making talos able to use the cgi server for parsing crashdumps. I copied over the code from automationutils.py into a seperate "crashhandler.py" module inside Talos. The idea here is that we might eventually want to factor crash parsing out into MozBase, and it'll be easier to do that if we know that we're using virtually the same code in automationutils.py and talos. 

The behaviour for choosing a minidump crash parser is slightly different between automationutils.py and I opted to go with the former's behaviour. If the user wants to do local minidump crash parsing, they'll need to set the MINIDUMP_STACKWALK environment variable to a path to a minidump crash parser. Before, talos would try to guess what platform the user was on and set the minidump parser to the appropriate file checked into talos inside the breakpad subdirectory.

I haven't really though enough about this to know which approach is really "better", so I opted for that of automationutils.py because I guessed it was touched most recently. I may have made the wrong call.

Comment 15

[(not currently active) Ted Mielczarek]

Comment on attachment 552246 [details] [diff] [review]
Add support to talos for use of CGI crashhandler

I'm not a Talos peer, so you'll probably want Alice to review this. (Also I wrote all the code you copied there, so it would be a bit inappropriate for me to review it!)

Just one note, you're using the poster lib here, you'll need to hg add poster.zip as well.

Comment 16

[William Lachance (:wlach)]

Attachment: Add support to talos for use of CGI crashhandler (take 2)

This adds poster.zip, required on systems without this package installed

Comment 17

[William Lachance (:wlach)]

Apparently we're planning to take a different approach to this due to load issues on the buildmaster (Bug 679759). I'll wait til' that cooks, then probably adapt it to Talos. Can hold off on reviews until then.

Comment 18

[alice nodelman [:alice] [:anode]]

Being sick and falling behind on reviews pays off!

Can you remove the review flag until you are ready to go?  Otherwise it will keep showing up in my queue.

Comment 19

[William Lachance (:wlach)]

Comment on attachment 552372 [details] [diff] [review]
Add support to talos for use of CGI crashhandler (take 2)

Unassigning anode as reviewer

Comment 20

[(not currently active) Ted Mielczarek]

Yeah, we should implement the same approach used in bug 679759 for Talos.

Comment 21

[Chris AtLee [:catlee]]

any progress here?

Comment 22

[Jeff Hammel]

AFAIK, no one is actively working on this.  If it is a high priority, we should probably figure out someone.

Comment 23

[William Lachance (:wlach)]

(In reply to Jeff Hammel [:jhammel] from comment #22)
> AFAIK, no one is actively working on this.  If it is a high priority, we
> should probably figure out someone.

If it's not super high priority (and I'm guessing it isn't if we managed to get this far without doing it), it would make a nice first bug for someone.

Comment 24

[Armen [:armenzg]]

It would reduce the run time for each job that does not crash since the 1) download  2) unzip and 3) remove steps for symbols would not be executed.
In other words, it has value to increase slightly our capacity.

Comment 25

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

Any progress? 

This is blocking some work in RelEng that would reduce load (see bug#561754 for details).

Comment 26

[William Lachance (:wlach)]

I am not currently working on this, remove myself to correct that impression.

Comment 27

[Joel Maher ( :jmaher ) (UTC -8)]

Do we have a preferred solution for this?

Comment 28

[(not currently active) Ted Mielczarek]

Yeah, this is implemented in mozcrash. We should just use that.

Comment 29

[(not currently active) Ted Mielczarek]

Dup of bug 675688 now?

Comment 30

[Jeff Hammel]

(In reply to Ted Mielczarek [:ted] from comment #29)
> Dup of bug 675688 now?

I think bug 675688 should be closed and this bug kept open for the talos side of things.

Comment 33

[Ed Morley [:emorley]]

Fixed in bug 824984.