1181255 - Get TSan builds and tests running in automation

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Chris Manchester (limited bugmail, email directly)]

I'd like to start with try.

Comment 1

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1181255 - Turn off tsan checking during packaging. r=glandium

Bug 1181255 - Turn off tsan checking during packaging. r=glandium

Comment 2

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1181255 - Mozconfigs for tsan builds. r=glandium

Bug 1181255 - Mozconfigs for tsan builds. r=glandium

Comment 3

[Chris Manchester (limited bugmail, email directly)]

Attachment: Schedule tsan builds and tests on try

I did this essentially by pattern matching and looking at asan and cc jobs, please let me know if I overlooked something obvious.

Comment 4

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1181255 - Add configs for (linux64 opt) tsan builds. r=jlund

Bug 1181255 - Add configs for (linux64 opt) tsan builds. r=jlund

Comment 5

[Chris Manchester (limited bugmail, email directly)]

Attachment: Schedule tsan builds and tests on try

Ah, I think I want to turn off try_by_default for now.

Comment 6

[Chris Manchester (limited bugmail, email directly)]

Attachment: Add linux64-tsan to production-masters.json

Comment 7

[Chris Manchester (limited bugmail, email directly)]

Attachment: tsan build type in treeherder

Comment 8

[Mike Hommey [:glandium]]

Comment on attachment 8633646 [details]
MozReview Request: Bug 1181255 - Turn off tsan checking during packaging. r=glandium

https://reviewboard.mozilla.org/r/13229/#review11893

Ship It!

Comment 9

[Mike Hommey [:glandium]]

Comment on attachment 8633647 [details]
MozReview Request: Bug 1181255 - Mozconfigs for tsan builds. r=glandium

https://reviewboard.mozilla.org/r/13231/#review11895

::: build/unix/mozconfig.tsan:26
(Diff revision 1)
> +export MOZ_DEBUG_SYMBOLS=1
> +ac_add_options --enable-debug-symbols

I know these two are in mozconfig.asan, but they shouldn't be necessary (as in, 1. they both do the same thing and 2. that's the default)

::: build/unix/mozconfig.tsan:11
(Diff revision 1)
> +export TSANFLAGS="-fsanitize=thread -fPIC -pie"

-fPIC is the default.
-pie is better not given to everything, so use ac_add_options --enable-pie instead.

::: build/unix/mozconfig.tsan:32
(Diff revision 1)
> +ac_add_options --disable-debug

--disable-debug is the default.

::: build/unix/mozconfig.tsan:31
(Diff revision 1)
> +ac_add_options --enable-optimize="-O2 -gline-tables-only"

Please add a comment as to why you add -gline-tables-only. Also, it would be better *not* to pass this through --enable-optimize, and thus not pass -O2 as well. You can use --enable-debug-symbols=-gline-tables-only instead.

::: browser/config/mozconfigs/linux64/opt-tsan:6
(Diff revision 1)
> +export MOZ_PACKAGE_JSSHELL=1

Do you plan to use that js shell? If not, why package it?

::: build/unix/mozconfig.tsan:18
(Diff revision 1)
> +ac_add_options --disable-sandbox

I'd say that's unfortunate. It would be good to document why sandbox and tsan don't work together.

Comment 10

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8633646 [details]
MozReview Request: Bug 1181255 - Turn off tsan checking during packaging. r=glandium

Bug 1181255 - Turn off tsan checking during packaging. r=glandium

Comment 11

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8633647 [details]
MozReview Request: Bug 1181255 - Mozconfigs for tsan builds. r=glandium

Bug 1181255 - Mozconfigs for tsan builds. r=glandium

Comment 12

[Mike Hommey [:glandium]]

Comment on attachment 8633647 [details]
MozReview Request: Bug 1181255 - Mozconfigs for tsan builds. r=glandium

https://reviewboard.mozilla.org/r/13231/#review11923

Ship It!

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/da3d456cee21
https://hg.mozilla.org/integration/mozilla-inbound/rev/3d7cf668b5f8

Comment 14

[Jordan Lund (:jlund)]

Comment on attachment 8633678 [details]
MozReview Request: Bug 1181255 - Add configs for (linux64 opt) tsan builds. r=jlund

https://reviewboard.mozilla.org/r/13235/#review11931

Ship It!

Comment 15

[Jordan Lund (:jlund)]

Comment on attachment 8633680 [details] [diff] [review]
Schedule tsan builds and tests on try

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

wow. well done at a blind attempt of this. There might be some unneeded items: legacy stuff for buildbot based factories before mozharness and mentions of talos configs even though we have talos disabled. In saying that, this patch is no different than asan/code-coverage (victim of this too) and it would be tedious to go through one by one removing all unused items especially as we are moving off buildbot anyway.

I believe this will add builds+tests for tsan on try if you explicitly select that in the trychooser syntax (i.e. try-by-default == False). This won't run on any other branches.

I think all that's left is:

- a cloud-tools patch to tell our instance manager to recognize the new builders and spin up instances accordingly
- trychooser patch so that this shows up as an option in: http://trychooser.pub.build.mozilla.org/

I did the cloud-tools one already (see next comment)

Comment 16

[Jordan Lund (:jlund)]

Attachment: 150714_tsan_watch_pending-cloud-configs.patch

Comment 17

[(not currently active) Ted Mielczarek]

(In reply to Mike Hommey [:glandium] from comment #9)
> ::: browser/config/mozconfigs/linux64/opt-tsan:6
> (Diff revision 1)
> > +export MOZ_PACKAGE_JSSHELL=1
> 
> Do you plan to use that js shell? If not, why package it?

If you're going to run jit-tests you need this nowadays. We should probably just make MOZ_PACKAGE_JSSHELL the default because of that.

Comment 18

[Jordan Lund (:jlund)]

Comment on attachment 8633885 [details] [diff] [review]
150714_tsan_watch_pending-cloud-configs.patch

thanks https://github.com/mozilla/build-cloud-tools/commit/354f4a9e7b0859869946170157594b28b8ac04cc

Comment 19

[Chris Manchester (limited bugmail, email directly)]

https://hg.mozilla.org/build/mozharness/rev/25190526f0c6

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/da3d456cee21
https://hg.mozilla.org/mozilla-central/rev/3d7cf668b5f8

Comment 21

[Chris Manchester (limited bugmail, email directly)]

https://hg.mozilla.org/build/buildbot-configs/rev/34925337358e

Comment 22

[Chris Manchester (limited bugmail, email directly)]

https://hg.mozilla.org/build/tools/rev/04945a026458

Comment 23

[Rail Aliiev [:rail]]

in production https://hg.mozilla.org/build/mozharness/rev/227c76104db4

Comment 24

[Treeherder GitHub Bugbot]

Commits pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/287738dd8bbdee503dc1e6e7ce5ae247f230c13e
Bug 1181255 - Make treeherder aware of a tsan build type.

https://github.com/mozilla/treeherder/commit/1d934685c1982e20c5c251422d92536f5477b020
Merge pull request #757 from chmanchester/tsan-builds

Bug 1181255 - Make treeherder aware of a tsan build type.

Comment 25

[Chris Manchester (limited bugmail, email directly)]

Attachment: PR with fixup

Comment 26

[Ed Morley [:emorley]]

Comment on attachment 8635018 [details] [review]
PR with fixup

r=me with comment

Comment 27

[Treeherder GitHub Bugbot]

Commits pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/8ed72907900588b52bb4f1de40047b3c551de7db
Bug 1181255 - Add tsan field to thOptionOrder.

https://github.com/mozilla/treeherder/commit/e3e47d6133fe41ca54f4ac7af543e0aa2b04aafe
Merge pull request #781 from chmanchester/tsan_fix

Bug 1181255 - Add tsan field to thOptionOrder.

Comment 28

[Jordan Lund (:jlund)]

In production: https://hg.mozilla.org/build/buildbot-configs/rev/34925337358e

Comment 29

[Chris Manchester (limited bugmail, email directly)]

Follow up for buildbot configs (sign off from jlund in #releng): https://hg.mozilla.org/build/buildbot-configs/rev/546c7ea19f2b

Comment 30

[Release Engineering SlaveAPI Service]

In production: https://hg.mozilla.org/build/buildbot-configs/rev/546c7ea19f2b

Comment 31

[Chris Manchester (limited bugmail, email directly)]

Attachment: Follow up to fix slave platform in mozilla/config.py

Comment 32

[Jordan Lund (:jlund)]

Comment on attachment 8636761 [details] [diff] [review]
Follow up to fix slave platform in mozilla/config.py

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

feel free to land when 1186188 is resolved. I wasn't able to verify this will completely solve our issue as the max_builder assert was blocking my tests from finishing but logically, this is what we need.

Comment 33

[Chris Manchester (limited bugmail, email directly)]

(In reply to Jordan Lund (:jlund) from comment #32)
> Comment on attachment 8636761 [details] [diff] [review]
> Follow up to fix slave platform in mozilla/config.py
> 
> Review of attachment 8636761 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> feel free to land when 1186188 is resolved. I wasn't able to verify this
> will completely solve our issue as the max_builder assert was blocking my
> tests from finishing but logically, this is what we need.

Bug 1186280 got us back under the limit:
https://hg.mozilla.org/build/buildbot-configs/rev/84310ed35dc0

Comment 34

[Release Engineering SlaveAPI Service]

In production: https://hg.mozilla.org/build/buildbot-configs/rev/84310ed35dc0

Comment 35

[Jordan Lund (:jlund)]

Attachment: 150822_tsan_tests_fix_builderdir_dupe-bbot-cfgs.patch

https://bugzilla.mozilla.org/attachment.cgi?id=8636761&action=edit was backed out because of:
INFO  - ValueError: builder Ubuntu TSAN VM 12.04 x64 try opt test mochitest-1 reuses builddir try_ubuntu64_vm_test-mochitest-1

the previous patch was telling buildbot not to bother creating a new slave_type for tsan (which would require a big patch and duplicate code). Instead, use the existing ubuntu64_vm just like code-coverage does.

this will work fine except, when we go to create builddirs on the slave, the builddir derives from the slave_type + platform name, which would cause duplicate builddirs as there is already a ubuntu64_vm mochitest-1. To compensate, we are using build_dir_prefix to manually set the builddir and telling the scheduler there is magic (scheduler_slave_platform_identifier)

builderlist diff: http://people.mozilla.org/~jlund/tsan_test_builderlist.diff
dumpmaster diff: http://people.mozilla.org/~jlund/tsan_test_dump_master.diff

Comment 36

[Jordan Lund (:jlund)]

Comment on attachment 8637455 [details] [diff] [review]
150822_tsan_tests_fix_builderdir_dupe-bbot-cfgs.patch

thanks!

remote:   https://hg.mozilla.org/build/buildbot-configs/rev/3e6a23a3e0f0

Comment 37

[Jordan Lund (:jlund)]

In production: https://hg.mozilla.org/build/buildbot-configs/rev/3e6a23a3e0f0

Comment 38

[Chris Manchester (limited bugmail, email directly)]

Attachment: Keep building with gtk2 for tsan

Comment 39

[Mike Hommey [:glandium]]

Comment on attachment 8638159 [details] [diff] [review]
Keep building with gtk2 for tsan

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

Please make them use gtk3. All the other builds are going to be switched soon (bug 1186748).

Comment 40

[Chris Manchester (limited bugmail, email directly)]

Attachment: Get tsan builds on gtk3

Comment 41

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b21f74e37b60

Comment 42

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b21f74e37b60

Comment 43

[Chris Manchester (limited bugmail, email directly)]

These run with "-p linux64-tsan", this just needs to get in to trychooser.

Comment 45

[Chris Manchester (limited bugmail, email directly)]

RyanVM just pointed out this bug is still open.

Comment 46

[Firefox Bug Husbandry Bot]

Removing leave-open keyword from resolved bugs, per :sylvestre.