tdb transaction_prepare

tridge at samba.org tridge at samba.org
Mon Mar 30 23:29:53 GMT 2009


Hi Howard,

 > Tridge, here's a first pass as we discussed on irc.

Thanks! This looks really good, with only a couple of very minor
quibbles.

- I'd prefer a bool for 'prepared', and 'ret' over 'i' for the check
  of the return value of tdb_transaction_prepare().

- I think the function should be called
  tdb_transaction_prepare_commit(), just to make it clear from the
  name that it isn't a function called to prepare the start of a
  transaction.

- I think we probably want some paranoia checks on calling prepare
  twice, and on doing any modify ops once a prepare is done. So if
  someone tries to store a record after having done a prepare then
  that should be a transaction error I think (either that, or we need
  to remove the prepared flag)

- we will need this to be added to the test suite, the headers and the
  docs. We also need to make it clear in the docs that a failed
  prepare does an implicit cancel. I think that is the right behaviour
  (and it is what you coded), but it isn't necessarily obvious to a
  user of this function.

 > I think that zeroing out the recovery data from the magic_offset
 > needs to be moved into transaction_cancel but I haven't tried that
 > yet.

I think you're right. I also think that is an existing bug, although
its probably harmless in most cases. If the tdb_expand_file() fails
with the current code, we don't remove the recovery marker, so we
would do a recovery on the nest open. Zeroing the magic_offset marker
in the transaction_cancel would fix that.

Andrew just pointed out to me that we should be using
tdb_transaction_prepare_commit() in the ldb code too, as ldb in Samba4
uses a separate tdb for each ldap partition. By using
tdb_transaction_prepare_commit() we can commit safely across all those
databases.

Thanks for suggesting this!

Cheers, Tridge


More information about the samba-technical mailing list