[PATCH] transaction: support for callbacks during abort

Pierre-Yves David pierre-yves.david at ens-lyon.org
Sat Jul 11 14:47:00 CDT 2015



On 07/11/2015 11:31 AM, FUJIWARA Katsunori wrote:
> At Tue, 06 Jan 2015 22:00:18 -0800,
> Gregory Szorc wrote:
>>
>> # HG changeset patch
>> # User Gregory Szorc <gregory.szorc at gmail.com>
>> # Date 1420610193 28800
>> #      Tue Jan 06 21:56:33 2015 -0800
>> # Node ID 5affbb31eea2a1034bc73ee20182880adcc9af36
>> # Parent  b9d06fa10ef29c012d48bd4f3c93fd7bf1347d40
>> transaction: support for callbacks during abort
>>
>> Previous transaction work added callbacks to be called during regular
>> transaction commit/close. As part of refactoring Mozilla's pushlog
>> extension (an extension that opens a SQLite database and tries to tie
>> its transaction semantics to Mercurial's transaction), I discovered that
>> the new transaction APIs were insufficient to avoid monkeypatching
>> transaction instance internals. Adding a callback that is called during
>> transaction abort removes the necessity for monkeypatching and completes
>> the API.
>>
>> diff --git a/mercurial/transaction.py b/mercurial/transaction.py
>> --- a/mercurial/transaction.py
>> +++ b/mercurial/transaction.py
>
> (snip)
>
>> @@ -442,8 +455,10 @@ class transaction(object):
>>
>>               self.report(_("transaction abort!\n"))
>>
>>               try:
>> +                for cat in sorted(self._abortcallback):
>> +                    self._abortcallback[cat](self)
>>                   _playback(self.journal, self.report, self.opener, self._vfsmap,
>>                             self.entries, self._backupentries, False)
>>                   self.report(_("rollback completed\n"))
>>               except Exception:
>
> (also CC-ed to Pierre-Yves as a implementer of 'txnabort' hook)
>
> Now, I'm working for dirstate consistency around inside and border of
> transaction, and planning to use this abort callback mechanism to know
> the end of transaction at failure.

I'm a bit curious about what code you need to run in such case. If you 
use the file generator mechanism you do not need to worries about 
restoring backup.

However, I'm not sure how rolling back the file would atomatically 
invalide the in memory object content (probably not). Is this why you 
need a callback? could we (would it make sense to) integrate that as 
part of one of our current cache invalidation mechanism?

> But current implementation doesn't invoke abort callbacks, if there is
> no change on repository data at aborting transaction: e.g. "hg import"
> fails to import the first patch.

The change to dirstate should most probably register himself as change 
to the transaction. That would trigger it.

> So, there is no easy and certain way to know the end of transaction at
> failure.
>
> BTW, this also causes that 'txnabort' hook isn't invoked at aborting
> transaction in such case, because 'txnabort' hook invocation is
> achieved by abort callback mechanism. This may need some cleanup for
> orphan 'pretxnopen' hook assuming that one of 'txnclose' or 'txnabort'
> will be invoked subsequently.
>
> Then, please let me confirm some points below before changing around
> 'transaction._abort()' for my purpose:
>
>    - are there any usecases to expect actual changes to be rollbacked ?
>
>      In other words, does it cause problem to invoke abort callbacks
>      regardless of actual changes ?

If I remember correctly, it make "transaction abort" "abort complete 
message to happears in a lot of new place. Basically any place where we 
open a transaction but eventually do nothing. We can probably not change it

>    - are there any usecases to refer changes to be rollbacked in abort
>      callbacks ?
>
>      In other words, does it cause problem to invoke abort callbacks
>      after rollbacking ?

I would says yes, but detail could be complicated (to be continued)

>
>      BTW, 'txnabort' hooks are invoked without 'pending', and it means
>      that such changes should be invisible to 'txnabort' hooks, doesn't
>      it ? > Pierre-Yves

And here we have proof of (1) people do not rely of it too much (2) 
definition of the state of the transaciton during abort is far from trivial.
We should probably keep it with rolled back content/

>
>      Then, IMHO, such changes should be invisible to abort callbacks,
>      too.
>
> Maybe, should I introduce another callback to know the end of
> transaction (regardless of the result of it, or only at failure) ?

Sounds like an awesome idea. Python have:

try:
     .... # equivalent to transaction open
except:
     .... # equivalent to transaction abort
else:
     .... # equivalent to transaction success
finally:
     .... # equivalent of your proposal

Finally is definitly very useful, so go for it.

-- 
Pierre-Yves David






>
>
> ----------------------------------------------------------------------
> [FUJIWARA Katsunori]                             foozy at lares.dti.ne.jp
>

-- 
Pierre-Yves David


More information about the Mercurial-devel mailing list