724876 - ^C'ing a push shouldn't break the pushlog

Status: RESOLVED DUPLICATE of bug 966545

(Developer Services :: Mercurial: hg.mozilla.org, defect)

Description

[[e-mail] (:bhearsum)]

Apparently canceling a push at a certain point breaks the pushlog database to the point where it needs manual intervention before the hook will accept new pushes. Eg, bug 724741.

This needs to be fixed. It's not scalable to have anyone with push privileges able to break repositories.

bkero says: "my understanding is that the hooks associated with pushing aren't able to back out and keep a consistent history".

Comment 1

[Hal Wine [:hwine] use NI!]

Per discussions in #developers, there is no value to ever allowing ^C to work, as (depending on stage of commit), it does one or more of the following:
 - invalidates the hg cache, causing next user to pay the price to rebuild
 - makes the pushlog inconsistent, causing problems for automation & sheriffs

Note that any solution must be handle broken connections in a sane way. E.g. dev's ISP goes down before all data sent to server.

Comment 2

[(not currently active) Ted Mielczarek]

Maybe gps knows what we're doing wrong in the pushlog. The documentation says (so I assume we have in production) to enable it as a pretxnchangegroup hook:
http://hg.mozilla.org/hgcustom/hghooks/file/aac2f70d8ea7/example-hgrc#l7

The hook itself is here:
http://hg.mozilla.org/hgcustom/hghooks/file/aac2f70d8ea7/mozhghooks/pushlog.py

Maybe this actually wants to be a "changegroup" hook instead?  Per the Hg docs:
"changegroup"
    Run after a changegroup has been added via push, pull or unbundle. ID of the first new changeset is in "$HG_NODE". URL from which changes came is in "$HG_URL". 

"pretxnchangegroup"
    Run after a changegroup has been added via push, pull or unbundle, but before the transaction has been committed. Changegroup is visible to hook program. This lets you validate incoming changes before accepting them. Passed the ID of the first new changeset in "$HG_NODE". Exit status 0 allows the transaction to commit. Non-zero status will cause the transaction to be rolled back and the push, pull or unbundle will fail. URL that was source of changes is in "$HG_URL". 

From http://www.selenic.com/hg/help/hgrc

Comment 3

[Gregory Szorc [:gps]]

If we move to changegroup, we risk pushlog not running because the process was aborted after pretxnchangegroup but before changegroup. I believe this happened in the real world.

We should have pushlog writing during pretxnchangegroup. The bug is that pushlog currently doesn't honor transaction rollback.

My work to convert pushlog to revlogs would have solved this since revlogs are implicitly part of the transaction. Mercurial has since added some APIs around transaction management to make the situation a bit better that may allow us to continue using sqlite. But it's still not perfect.

Comment 4

[Gregory Szorc [:gps]]

(In reply to Hal Wine [:hwine] (use needinfo) from comment #1)
> Per discussions in #developers, there is no value to ever allowing ^C to
> work, as (depending on stage of commit), it does one or more of the
> following:
>  - invalidates the hg cache, causing next user to pay the price to rebuild
>  - makes the pushlog inconsistent, causing problems for automation & sheriffs
> 
> Note that any solution must be handle broken connections in a sane way. E.g.
> dev's ISP goes down before all data sent to server.

Ignoring ^C only appears to fix the problem for many cases: it doesn't cause the underlying problem (broken connections, unexpected process killing, server power loss, etc) from going away. Mercurial should work in the face of ^C. The bug here is that we've installed software on the server that doesn't play by Mercurial's rules. We should fix the software (pushlog) instead of resorting to non-robust and, quite frankly, scary hackiness around mucking about with signal handling.

Comment 5

[Gregory Szorc [:gps]]

This was fixed in bug 966545.