Bugfix for tdb transactions

[simo]

On Mon, 2010-02-01 at 12:11 +1100, tridge at samba.org wrote:
> Hi Volker,
> 
> That looks better, but I'm still not sure we've completely fixed
> this. Your patch will fix the case where a 2nd process opens the tdb
> during the commit of the first process, but I don't think it fixes the
> following case:
> 
>   - process 1 and 2 are both connected to the tdb
>   - process 1 starts a transaction
>   - process 1 starts a commit
>   - admin uses kill -9 on process 1, while transaction is partially
>     written (or segv or similar)
>   - process 2 starts a transaction, and uses corrupt data from process 1
> 
> The problem is that the transaction code assumes that in the case of a
> crash that all processes accessing the tdb die (such as when the
> machine as a whole crashes). That is not always true, especially if
> you have an admin who likes to use kill -9.
> 
> Fixing this completely could be quite expensive (check for recover
> flag on every IO??), but perhaps we could do the following:
> 
>   - when we start a transaction, check the recovery flag. If set then
>     db needs recovery.
> 
>   - in tdb_transaction_recover() get a write lock from FREELIST_TOP to
>     EOF if recovery is needed.
> 
> That will at least mean that in databases where writes are wrapped in
> transactions that we don't compound the problem caused by the kill -9.
> 
> Another nasty problem is that tdb_transaction_recover() can reduce the
> file size. This is fine if only one process is attached when recovery
> is started, but in the above scenario there could be more than 1
> process attached. That could lead those processes to get a SIGBUS when
> they try to access beyond EOF via mmap. So I think we'd need to remove
> the code that does the ftruncate() in tdb_transaction_recover().
> 
> Cheers, Tridge

Tridge,
I wonder if we can use some sort of smart detection to know when it is
safe to do some operations and when it is not.
For example we could make the first process to open a tdb file to do a
lock on a specific bit of data, so that any other opener will know the
file is shared. At the same time we set up an inotify watch to know from
the first process if any other process opens the tdb. This way we would
know at any time if more than one process has the tdb open and avoid
doing ftruncates or add additional checks only when this happens.
I know inotify is linux only, but we could simply optimize only for
those platforms that have meaningful ways to check for concurrent
access.
I am not sure how hard it would be to make this race free though.

Simo.


-- 
Simo Sorce
Samba Team GPL Compliance Officer <simo at samba.org>
Principal Software Engineer at Red Hat, Inc. <simo at redhat.com>