1248711 - NSPR log upload is broken

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Honza Bambas (:mayhemer)]

Bug 961147 introduces NSPR log uploads.  At the end runtests.py collects all logs in a specified temp dir, zips them, deletes them and puts the zip (fixed nsprlog.zip name) to the upload dir.  However, these days a single run may start multiple instances of runtests.py which all race on the files, so I'm not getting all files from e.g. asan runs.  

I would like to identify each log folder and zip file by the running instance of runtests.py.  Is there some global counter or identifier I could use?  

If not, I think I can simply use a random generator (not sure how to seed it safely to avoid overlaps)

Comment 1

[(not currently active) Ted Mielczarek]

jmaher did most of the work to get run-by-dir working, which is what's mucking you up here. Joel: can you help Honza out? He built a feature in the Mochitest harness to save NSPR logs to get uploaded by blobber, but run-by-dir breaks this because we run the browser multiple times.

Comment 2

[Honza Bambas (:mayhemer)]

I think I can use uuid4().

Comment 3

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

it seems as though we create the .zip file, but end up over writing it for each iteration of the browser.

we have a couple of loops in the mochitest harness.  Here is where we loop per directory:
https://dxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.py#2184

we don't really track the directory as it passes down into the main runner, so we would need to add that, maybe in the options or as a new parameter.  Once we get the directory we could easily modify the nsprlog.zip.

Does that help Honza?

Comment 4

[Honza Bambas (:mayhemer)]

(In reply to Joel Maher (:jmaher) from comment #3)
> it seems as though we create the .zip file, but end up over writing it for
> each iteration of the browser.
> 
> we have a couple of loops in the mochitest harness.  Here is where we loop
> per directory:
> https://dxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.
> py#2184
> 
> we don't really track the directory as it passes down into the main runner,
> so we would need to add that, maybe in the options or as a new parameter. 
> Once we get the directory we could easily modify the nsprlog.zip.
> 
> Does that help Honza?

I'll use UUID.  I'm currently testing it locally on a win box.

Comment 5

[Honza Bambas (:mayhemer)]

Attachment: v1

Comment 6

[Honza Bambas (:mayhemer)]

Depends on which of the two is going to be fixed sooner: bug 1248565 or bug 961430.  W/o 961430 we won't be able to delete the log files on windows because (the same file) is open twice.  Bug 1248565 (with using %PID token in file name) just gives each a different name, one with actual pid number and other with %PID in the name.

Comment 7

[Honza Bambas (:mayhemer)]

First tests show this is kinda impractical: https://treeherder.mozilla.org/#/jobs?repo=try&author=[e-mail]&selectedJob=16810268

There is 30 files to download.

Seems like moving the 'zip all logs + move the zip to upload dir' part needs to happen at the end of run-by-dir session.  Going to check at what has been suggested in comment 3.  Thanks!

Comment 8

[Honza Bambas (:mayhemer)]

Comment on attachment 8719984 [details] [diff] [review]
v1

(not very practical solution)

Comment 9

[Honza Bambas (:mayhemer)]

Attachment: v2

- each run's log name is added a uuid (cheap uuid4 - just random)
- logs are collected at the end of all test runs, we have all of them in a single zip file again

Example use: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a2b599d0f9d1

Comment 10

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

Comment on attachment 8720303 [details] [diff] [review]
v2

Review of attachment 8720303 [details] [diff] [review]:
-----------------------------------------------------------------

your approach is fine, I would prefer moz-upload-dir, but if you have good reasons for a tempdir, I have no issues.

::: testing/mochitest/runtests.py
@@ +2291,5 @@
>          if self.browserEnv is None:
>              return 1
>  
> +        if self.nsprLogs:
> +            self.browserEnv["NSPR_LOG_FILE"] = "{}/nspr-pid=%PID-uid={}.log".format(tempfile.gettempdir(), str(uuid.uuid4()))

can we just use MOZ_UPLOAD_DIR instead of gettempdir()?  We do delete the files once we combine them, so there seems to be no downside to moz_upload_dir.

Comment 11

[Honza Bambas (:mayhemer)]

Attachment: v2.1

carrying r+, using the upload dir (I don't recall reason not to use it, this works well too, OTOH, if we fail to zip+delete them, we can end up uploading a large amount of data..)

Comment 12

[Honza Bambas (:mayhemer)]

(only a by default non-used small test infra change)

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/22fe66c4313f

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/22fe66c4313f