1297153 - Detect and recover from active locks and transactions

Status: RESOLVED FIXED

(Developer Services :: Mercurial: robustcheckout, defect)

Description

[Gregory Szorc [:gps]]

See bug 1297151. Essentially a previous TC task was likely forcefully aborted with SIGKILL. Mercurial didn't release the lock or transaction. The next task to use the cache inherited a stale lock and timed out waiting on its release because the process creating it had been terminated.

We can work around this issue by teaching `hg robustcheckout` to forcefully release an active lock and/or transaction if instructed to do so. This would likely be the default behavior on TaskCluster, where we're guaranteed to only have a single task/process accessing a repo at the same time, since TC guarantees only a single task has access to a cache.

Comment 3

[Gregory Szorc [:gps]]

This has cropped up a few times recently. I'm planning to install a newer Mercurial in CI for the Mercurial security fixes. We might as well get some robustcheckout fixes in there as well.

Comment 4

[Gregory Szorc [:gps]]

Example log, for posterity.

[vcs 2017-08-10T01:10:55.390297Z] executing ['hg', 'robustcheckout', '--sharebase', '/home/worker/checkouts/hg-store', '--purge', '--upstream', 'https://hg.mozilla.org/mozilla-unified', '--revision', '886618c4e475d310e9e5e5177ee1a9899e3f3dd1', 'https://hg.mozilla.org/try', '/home/worker/checkouts/gecko']
[vcs 2017-08-10T01:10:55.454389Z] ensuring https://hg.mozilla.org/try@886618c4e475d310e9e5e5177ee1a9899e3f3dd1 is available at /home/worker/checkouts/gecko
[vcs 2017-08-10T01:10:55.454549Z] (existing repository shared store: /home/worker/checkouts/hg-store/8ba995b74e18334ab3707f27e9eb8f4e37ba3d29/.hg)
[vcs 2017-08-10T01:10:55.526199Z] (pulling to obtain 886618c4e475d310e9e5e5177ee1a9899e3f3dd1)
[vcs 2017-08-10T01:10:56.738195Z] waiting for lock on repository /home/worker/checkouts/gecko held by 'abd5439b1213:7'
[vcs 2017-08-10T01:20:57.467127Z] abort: repository /home/worker/checkouts/gecko: timed out waiting for lock held by abd5439b1213:7

Comment 5

[Gregory Szorc [:gps]]

Attachment: robustcheckout: support Mercurial 4.3;

The old code worked fine. However, a developer warning is being issued
due to the deprecation of cmdutil.command() in favor of
registrar.command(). Porting is trivial.

registrar.py was introduced in Mercurial 3.7. So it should import
everywhere where the extension is supported.

Review commit: https://reviewboard.mozilla.org/r/167336/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/167336/

Comment 6

[Gregory Szorc [:gps]]

Attachment: robustcheckout: tweak warning message;

Clarify that it is the working directory that we're deleting (as
opposed to the shared store).

Review commit: https://reviewboard.mozilla.org/r/167338/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/167338/

Comment 7

[Gregory Szorc [:gps]]

Attachment: robustcheckout: add tests demonstrating lock interaction (bug 1297153);

This is pretty self-explanatory. Just showing what happens when
various locks are in play.

Review commit: https://reviewboard.mozilla.org/r/167340/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/167340/

Comment 8

[Gregory Szorc [:gps]]

Attachment: robustcheckout: detect and recover from open locks (bug 1297153);

In automation, `hg` processes can get killed by SIGKILL. SIGKILL
doesn't give processes time to clean up and gracefully abort
whatever operation they were performing. If a Mercurial operation
is forcibly killed while holding a lock, it will orphan a lock
file.

On normal machines with local filesystems, Mercurial can tell
that the process behind the lock has gone away, that the lock is
stale, and that it can be removed automatically. However, in
TaskCluster, operations exist in PID namespaces and may have
random hostnames between tasks. This confuses Mercurial's lock
disambiguation mechanism and prevents Mercurial from reclaiming
the lock. This leads to repositories getting wedged.

This commit adds detection for locks before we attempt any Mercurial
operation. If a lock is present, we assume it was left over from a
dead process and nuke the repository data because we can't easily
guarantee the repository isn't corrupt. (In many scenarios a
SIGKILL'd process will corrupt the repository.) It is much easier
to just re-clone the repo or create a new working directory than
to validate the state of the repo. And because we use streaming
clones, it is probably faster too!

Review commit: https://reviewboard.mozilla.org/r/167342/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/167342/

Comment 9

[Gregory Szorc [:gps]]

Comment on attachment 8896061 [details]
robustcheckout: support Mercurial 4.3;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/167336/diff/1-2/

Comment 10

[Gregory Szorc [:gps]]

Comment on attachment 8896062 [details]
robustcheckout: tweak warning message;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/167338/diff/1-2/

Comment 11

[Gregory Szorc [:gps]]

Comment on attachment 8896063 [details]
robustcheckout: add tests demonstrating lock interaction (bug 1297153);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/167340/diff/1-2/

Comment 12

[Gregory Szorc [:gps]]

Comment on attachment 8896064 [details]
robustcheckout: detect and recover from open locks (bug 1297153);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/167342/diff/1-2/

Comment 13

[:glob ✱]

Comment on attachment 8896061 [details]
robustcheckout: support Mercurial 4.3;

https://reviewboard.mozilla.org/r/167336/#review173802

Comment 14

[:glob ✱]

Comment on attachment 8896062 [details]
robustcheckout: tweak warning message;

https://reviewboard.mozilla.org/r/167338/#review173804

Comment 15

[:glob ✱]

Comment on attachment 8896063 [details]
robustcheckout: add tests demonstrating lock interaction (bug 1297153);

https://reviewboard.mozilla.org/r/167340/#review173808

Comment 16

[:glob ✱]

Comment on attachment 8896064 [details]
robustcheckout: detect and recover from open locks (bug 1297153);

https://reviewboard.mozilla.org/r/167342/#review173810

Comment 17

[Gregory Szorc [:gps]]

(In reply to Byron Jones ‹:glob› from comment #16)
> Comment on attachment 8896064 [details]
> robustcheckout: detect and recover from open locks (bug 1297153);
> 
> https://reviewboard.mozilla.org/r/167342/#review173810

r- with no issues or comment?

Comment 19

[:glob ✱]

sorry about that; i swear i r+'d it.  guess i slipped.  will correct.

Comment 20

[:glob ✱]

Comment on attachment 8896064 [details]
robustcheckout: detect and recover from open locks (bug 1297153);

https://reviewboard.mozilla.org/r/167342/#review174306

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/e6d65314e2e8
robustcheckout: add tests demonstrating lock interaction ; r=glob
https://hg.mozilla.org/hgcustom/version-control-tools/rev/19db5f4b5b10
robustcheckout: detect and recover from open locks ; r=glob

Comment 22

[Gregory Szorc [:gps]]

This will go out to in-tree Docker images in bug 1389715.