1960981 - Back out Bug 1907124 (Bypassing Updates for Multiple Profiles feature)

Status: RESOLVED FIXED

(Toolkit :: Application Update, task, P1)

Description

[Robin Steuber (she/her) [:bytesized]]

So the last status update on this project was Bug 1941931 Comment 81:

I have a few other small things on my plate before I can get back to this, but then I guess I will try option (a) from Comment 36.

(In reply to Robin Steuber (she/her) [:bytesized] from comment #36)

I think that the solutions here are either (a) delete the lock file when the "Restart" button is clicked, or (b) let go of the lock itself earlier in shutdown. I'm leaning towards option (b).

...

If the above does not work (and we still don't know why), we basically are left with two choices:

1. Revert this feature's functionality to the old functionality (pre-Bug 1907124), or
2. Spend a few days making a custom build with tons and tons of extra logging, hope that someone that can reproduce this will be interested in running it and giving us the logs, and then hope that those logs point us in the direction of the actual fix without more custom builds with more logging.

We have talked to the team that we were working on this feature for and, given the problems this has had and the timeline we can realistically expect at this point, they are okay with us dropping this project. So we are actually just going to jump to choice (1): Backing out the feature entirely and reverting it to the old functionality.

I'll be posting the backout patches here. We have been asked to have them ready for 138, so I will be uplifting this backout to Beta. I'm not concerned about this uplift being risky. Other than my attempts at fixing this problem (which will also be backed out), no code has changed in the area so the backout should be clean. And the feature that we are reverting to seemed to work as expected for years. So I expect this to be very safe.

Comment 1

[Robin Steuber (she/her) [:bytesized]]

Ah. I forgot about Bug 1938657. That complicates this slightly, because I feel like that change was actually needed independently of this, so I don't want to back it out. But that is pretty isolated to the metrics YAML(s). So I don't think that this will make the backout too horrifying.

Comment 2

[Robin Steuber (she/her) [:bytesized]]

I think I actually want to keep Bug 1922395 too. It solved some real problems and was really only prerequisite work for Bug 1907124, not part of the feature itself.

Comment 3

[Robin Steuber (she/her) [:bytesized]]

Attachment: Bug 1960981 - Backed out changeset 30dda8e7ae32 (Bug 1959492) r=nrishel!

Comment 4

[Robin Steuber (she/her) [:bytesized]]

Attachment: Bug 1960981 - Backed out changeset 12f8c6bbd677 (Bug 1941931) r=nrishel!

Comment 5

[Robin Steuber (she/her) [:bytesized]]

Attachment: Bug 1960981 - Backed out changeset a4acaa6581bb (Bug 1907126 - Make common test functionality more self contained) r=nrishel!

Comment 6

[Robin Steuber (she/her) [:bytesized]]

Attachment: Bug 1960981 - Backed out changeset 0743d5998d6a (Bug 1907126 - Add testing for the multi session install lock feature) r=nrishel!

Comment 7

[Robin Steuber (she/her) [:bytesized]]

Attachment: Bug 1960981 - Backed out changeset 2c26ce3f8dd3 (Bug 1907126 - Split `--test-process-updates` functionality) r=nrishel!

Comment 8

[Robin Steuber (she/her) [:bytesized]]

Attachment: Bug 1960981 - Backed out changeset b5f433a476de (Bug 1907126 - Refactor out repeated update testing code) r=nrishel!

Comment 9

[Robin Steuber (she/her) [:bytesized]]

Attachment: Bug 1960981 - Backed out changeset 811fa9d7c8a6 (Bug 1907128 - Add telemetry to monitor reasons why we don't apply updates at startup) r=nrishel!

Note, this wasn't a perfectly clean backout because I am not backing out Bug 1938657, but I am very confident in it because the necessary merge was simply to remove the added lines from shared_metrics.yaml instead of metrics.yaml.

Comment 10

[Robin Steuber (she/her) [:bytesized]]

Attachment: Bug 1960981 - Backed out changeset 2a19a01424f1 (Bug 1907120 - Remove other multi-instance checks now that we check at startup) r=nrishel!

Comment 11

[Robin Steuber (she/her) [:bytesized]]

Attachment: Bug 1960981 - Backed out changeset 118a3fe99c1e (Bug 1907127 - Read update.timestamp at startup and use it to delay updates) r=nrishel!

Comment 12

[Robin Steuber (she/her) [:bytesized]]

Attachment: Bug 1960981 - Backed out changeset 8f6065837da3 (Bug 1907127 - Add support for writing update.timestamp) r=nrishel!

Comment 13

[Robin Steuber (she/her) [:bytesized]]

Attachment: Bug 1960981 - Backed out changeset 608d68ca5b0e (Bug 1907127 - Adds `usingDefaultInstallDate` to `Update` objects) r=nrishel!

Comment 14

[Robin Steuber (she/her) [:bytesized]]

It feels like it would be chaotic to attach the Beta patches to the same bug. I'll file a separate one.

Comment 15

[Phabricator Automation]

Comment on attachment 9480377 [details]
Bug 1960981 - Backed out changeset 30dda8e7ae32 (Bug 1959492) r=nrishel!

Revision D246289 was moved to bug 1961905. Setting attachment 9480377 [details] to obsolete.

Comment 16

[Phabricator Automation]

Comment on attachment 9480378 [details]
Bug 1960981 - Backed out changeset 12f8c6bbd677 (Bug 1941931) r=nrishel!

Revision D246290 was moved to bug 1961905. Setting attachment 9480378 [details] to obsolete.

Comment 17

[Phabricator Automation]

Comment on attachment 9480379 [details]
Bug 1960981 - Backed out changeset a4acaa6581bb (Bug 1907126 - Make common test functionality more self contained) r=nrishel!

Revision D246291 was moved to bug 1961905. Setting attachment 9480379 [details] to obsolete.

Comment 18

[Phabricator Automation]

Comment on attachment 9480380 [details]
Bug 1960981 - Backed out changeset 0743d5998d6a (Bug 1907126 - Add testing for the multi session install lock feature) r=nrishel!

Revision D246292 was moved to bug 1961905. Setting attachment 9480380 [details] to obsolete.

Comment 19

[Phabricator Automation]

Comment on attachment 9480381 [details]
Bug 1960981 - Backed out changeset 2c26ce3f8dd3 (Bug 1907126 - Split --test-process-updates functionality) r=nrishel!

Revision D246293 was moved to bug 1961905. Setting attachment 9480381 [details] to obsolete.

Comment 20

[Phabricator Automation]

Comment on attachment 9480382 [details]
Bug 1960981 - Backed out changeset b5f433a476de (Bug 1907126 - Refactor out repeated update testing code) r=nrishel!

Revision D246294 was moved to bug 1961905. Setting attachment 9480382 [details] to obsolete.

Comment 21

[Phabricator Automation]

Comment on attachment 9480383 [details]
Bug 1960981 - Backed out changeset 811fa9d7c8a6 (Bug 1907128 - Add telemetry to monitor reasons why we don't apply updates at startup) r=nrishel!

Revision D246295 was moved to bug 1961905. Setting attachment 9480383 [details] to obsolete.

Comment 22

[Phabricator Automation]

Comment on attachment 9480384 [details]
Bug 1960981 - Backed out changeset 2a19a01424f1 (Bug 1907120 - Remove other multi-instance checks now that we check at startup) r=nrishel!

Revision D246297 was moved to bug 1961905. Setting attachment 9480384 [details] to obsolete.

Comment 23

[Phabricator Automation]

Comment on attachment 9480385 [details]
Bug 1960981 - Backed out changeset 118a3fe99c1e (Bug 1907127 - Read update.timestamp at startup and use it to delay updates) r=nrishel!

Revision D246304 was moved to bug 1961905. Setting attachment 9480385 [details] to obsolete.

Comment 24

[Phabricator Automation]

Comment on attachment 9480386 [details]
Bug 1960981 - Backed out changeset 8f6065837da3 (Bug 1907127 - Add support for writing update.timestamp) r=nrishel!

Revision D246305 was moved to bug 1961905. Setting attachment 9480386 [details] to obsolete.

Comment 25

[Phabricator Automation]

Comment on attachment 9480387 [details]
Bug 1960981 - Backed out changeset 608d68ca5b0e (Bug 1907127 - Adds usingDefaultInstallDate to Update objects) r=nrishel!

Revision D246306 was moved to bug 1961905. Setting attachment 9480387 [details] to obsolete.

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cdac4669b3dc
Backed out changeset 30dda8e7ae32 (Bug 1959492) r=nrishel
https://hg.mozilla.org/integration/autoland/rev/0be204c5e3c7
Backed out changeset 12f8c6bbd677 (Bug 1941931) r=nrishel
https://hg.mozilla.org/integration/autoland/rev/9739f121c8fe
Backed out changeset 0743d5998d6a (Bug 1907126 - Add testing for the multi session install lock feature) r=nrishel
https://hg.mozilla.org/integration/autoland/rev/e04a3ed8030f
Backed out changeset 811fa9d7c8a6 (Bug 1907128 - Add telemetry to monitor reasons why we don't apply updates at startup) r=nrishel
https://hg.mozilla.org/integration/autoland/rev/f60528636529
Backed out changeset 2a19a01424f1 (Bug 1907120 - Remove other multi-instance checks now that we check at startup) r=nrishel,fluent-reviewers,desktop-theme-reviewers,bolsson,emilio
https://hg.mozilla.org/integration/autoland/rev/0b68cddd0a11
Backed out changeset 118a3fe99c1e (Bug 1907127 - Read update.timestamp at startup and use it to delay updates) r=nrishel
https://hg.mozilla.org/integration/autoland/rev/181a78b73b8e
Backed out changeset 8f6065837da3 (Bug 1907127 - Add support for writing update.timestamp) r=nrishel
https://hg.mozilla.org/integration/autoland/rev/3fefa68ab74d
Backed out changeset 608d68ca5b0e (Bug 1907127 - Adds `usingDefaultInstallDate` to `Update` objects) r=nrishel

Comment 27

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/cdac4669b3dc
https://hg.mozilla.org/mozilla-central/rev/0be204c5e3c7
https://hg.mozilla.org/mozilla-central/rev/9739f121c8fe
https://hg.mozilla.org/mozilla-central/rev/e04a3ed8030f
https://hg.mozilla.org/mozilla-central/rev/f60528636529
https://hg.mozilla.org/mozilla-central/rev/0b68cddd0a11
https://hg.mozilla.org/mozilla-central/rev/181a78b73b8e
https://hg.mozilla.org/mozilla-central/rev/3fefa68ab74d