As part of the dawn project, as we are removing aurora, we need to still keep the diagnostic assert enabled for some period of time. I believe that we can enable it during during the beta staged rollout period (which will be the current aurora population anyway). In parallel, we will probably sync EARLY_BETA_OR_EARLIER with the beta staged roll out period.

Comment on attachment 8859637 [details] Bug 1357799 - Enable the diagnostic assert during the beta staged roll out period https://reviewboard.mozilla.org/r/131656/#review134492

Pushed by sledru@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/19ea645f562e Enable the diagnostic assert during the beta staged roll out period r=bholley

Should we uplift this MOZ_DIAGNOSTIC_ASSERT change to Beta 54? I don't see any reason to wait. MOZ_DIAGNOSTIC_ASSERTs are enabled in Nightly and nothing during Nightly 55 is going to make MOZ_DIAGNOSTIC_ASSERTs any less scary for the wider beta population before Beta 55.

The biggest unknown with shipping this is how much crashier will Beta "appear" with this change. We could try uplifting this to 54 and find out. If this makes Beta 2x as crashy, perhaps this is not the right solution. Do we need to update our stability dashboard (arewestableyet) to disregard crashes owing to these diagnostic asserts similar to the infobar crash submissions?

I like the idea of evaluating this with 54.

Comment on attachment 8859637 [details] Bug 1357799 - Enable the diagnostic assert during the beta staged roll out period Approval Request Comment [Feature/Bug causing the regression]: Dawn project (end of aurora) [User impact if declined]: Less information about unexpected cases [Is this code covered by automated tests?]: I don't think so :/ [Has the fix been verified in Nightly?]: With this change, it will enable this feature until the middle of the beta cycle. [Needs manual test from QE? If yes, steps to reproduce]: Nope [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: I don't think so [Why is the change risky/not risky?]: because it will enable a feature which has been on nightly & aurora for a while [String changes made/needed]: none

(In reply to Ritu Kothari (:ritu) from comment #6) > The biggest unknown with shipping this is how much crashier will Beta > "appear" with this change. From the time I've spent looking at Nightly and Aurora crashes, I'd informally say that diagnostic crashes are a very small part of our overall crash volume. The bigger issue might be the performance impact, but I don't think that is big enough to worry about most of the time.

(In reply to Andrew McCreight [:mccr8] from comment #9) > (In reply to Ritu Kothari (:ritu) from comment #6) > > The biggest unknown with shipping this is how much crashier will Beta > > "appear" with this change. > From the time I've spent looking at Nightly and Aurora crashes, I'd > informally say that diagnostic crashes are a very small part of our overall > crash volume. The bigger issue might be the performance impact, but I don't > think that is big enough to worry about most of the time. Great! Thanks. That is very reassuring.

Comment on attachment 8859637 [details] Bug 1357799 - Enable the diagnostic assert during the beta staged roll out period Get more information during beta stage rollout period. Beta54+. Should be in 54 beta 2.

also needed to push https://hg.mozilla.org/releases/mozilla-beta/rev/8ad85f4bb919841e980fe7251df54fba4466fd42 as followup fix to fix a bustage

and https://hg.mozilla.org/releases/mozilla-beta/rev/7fad67b91a0888164d7f0257c359bcaab49648cf for another bustage fix

backed this changes out from beta for continued bustage to unbreak the tree

Hi Sylvestre, Ben, if we go down the path of doing two builds off of Beta: regular beta + DevEd builds, we could turn on diagnostic asserts on just DevEd builds. Is this fix still needed on 55+? Or do we want to allow diagnotics asserts in DevEd builds only for early beta population?

(In reply to Ritu Kothari (:ritu) from comment #16) > Hi Sylvestre, Ben, if we go down the path of doing two builds off of Beta: > regular beta + DevEd builds, we could turn on diagnostic asserts on just > DevEd builds. Yes > Is this fix still needed on 55+? Or do we want to allow > diagnotics asserts in DevEd builds only for early beta population? Not my call, but it seems like we'd probably want them on permanently for DevEdition, since that's what we've done when it was on Aurora.

Sorry about that tomcat, I will have a look on how to reproduce this locally

Should we back this out on the trunk, too, since it's a bustage train rushing toward beta-55, and unlikely to be what we actually want anyway?

yes, thanks for the ping. Tomcat, could you take care of the backout? Danke

(In reply to Sylvestre Ledru [:sylvestre] from comment #20) > yes, thanks for the ping. > > Tomcat, could you take care of the backout? Danke np :) done in https://hg.mozilla.org/integration/mozilla-inbound/rev/c5a9ce06152f

Merged backout: https://hg.mozilla.org/mozilla-central/rev/c5a9ce06152f

Sylvestre, my understanding is that we need diagnostic assertions enabled for early DevEdition builds from each cycle at least. Are you still working on this?

Ben, I was thinking that this flag could be enabled for the whole devedition cycle. But if we want it only for a part of the devedition cycle, why not. I can work on this tomorrow or Thursday.

(In reply to Sylvestre Ledru [:sylvestre] from comment #24) > Ben, I was thinking that this flag could be enabled for the whole devedition > cycle. > But if we want it only for a part of the devedition cycle, why not. > > I can work on this tomorrow or Thursday. I don't have any preferences as to whole vs. part. We need to use a new flag that we can control from a mozconfig if we want to do it for the entire cycle, though. Totally up to you.

This won't go in 54 at this point.

Comment on attachment 8859637 [details] Bug 1357799 - Enable the diagnostic assert during the beta staged roll out period resetting approval flag as this was backed out.

FWIW I'm opposed to this change if it means we can no longer use diagnostic asserts where we used to. I've had at least one person infer that outcome as a result of enforcing diagnostics in our larger beta population.

The work is happening here now: bug 1368079

(In reply to Ben Hearsum (:bhearsum) from comment #25) > (In reply to Sylvestre Ledru [:sylvestre] from comment #24) > > Ben, I was thinking that this flag could be enabled for the whole devedition > > cycle. > > But if we want it only for a part of the devedition cycle, why not. > > > > I can work on this tomorrow or Thursday. > > I don't have any preferences as to whole vs. part. We need to use a new flag > that we can control from a mozconfig if we want to do it for the entire > cycle, though. Totally up to you. I would really like to recommend that we enable it for the whole cycle. When people pick a channel they are opting in to a certain risk profile. Either that risk includes diagnostic assertions or it doesn't. Users are not going to understand the differences if we turn them on/off over time. Also, if the current plan is just the dev-edition binaries then I hope the concerns from comment 28 are addressed. Sorry for the noise.