Re: [boost] [transaction] New Boost.Transaction library underdiscussion

Subject: Re: [boost] [transaction] New Boost.Transaction library underdiscussion
From: Stefan Strasser (strasser_at_[hidden])
Date: 2010-01-18 18:10:03

Am Monday 18 January 2010 21:52:49 schrieb vicente.botet:
> Looking at the code I think that the lazy creation of resource associated
> to the created transaction introduce more problems that it solves. How can
> you ensure that each resource has an equivalent stack of local nested
> transactions if you create them only when the application access a resource
> on the context of a global transaction?

why couldn't you ensure that? line 170 recursively calls 
resource_transaction() to do exactly that.

>
> > basic_transaction_manager.hpp: my implementation of the
> > TransactionManager concept
>
> I find extrange the way basic_transaction_manager is made a singleton.
>
>  static bool has_active(){  return active_; }
>
>  static basic_transaction_manager &active(){
>   if(active_) return *active_;
>   else persistent::detail::throw_(no_active_transaction_manager());
>  }
>  void bind(){ active_=this; }
>  static void unbind(){ active_=0; }
>
> What can the application do when there is no transaction_manager? Nothing
> in my opinion, so the system need to ensure this invariant. I would preffer
> just an instance() static function. 

correct, Nothing. and it is a singleton.
but it is not invariant.

1. the transaction manager is constructed by the user, and it must be. the 
entire runtime configuration of a library depends on that, including the 
filename of a database file, desired cache size, etc.
(by passing those to a resource manager and then passing the resource manager 
to the transaction manager constructor).

how do you construct a transaction manager (and the resource managers used by 
it) at the point of (lazy) singleton construction if you don't have any 
constructor arguments?

2. why shouldn't it be possible to do:

{
  open and use database in file db1.db
}
{
  open and use database in file db2.db
}
?

> As the active transaction can be null the better is that the function
> return the pointer to the active transaction. I find this prototype simple.
>
>  transaction* active_transaction() const{
>   return this->active_transaction(mpl::bool_<Threads>());
>  }

this is not uber-important to me and returning a pointer would be ok, but I 
nevertheless disagree with that.

you are looking at the implementation of basic_transaction_manager.
I was foremost trying to define a concept TransactionManager without looking 
at implementation details.

when the active transaction is needed for an operation, for example by a 
basic_loc to access a persistent object, there are generally two cases:

a) "I need a transaction for the following operation, if there is none an 
exception must be thrown." for example: writing to a transactional object.

write_to_object(txmgr.active_transaction()); //throws

b) "although I could do this operation without a transaction, if there is one, 
another code path must be followed." for example: reading from a 
transactional object.

if(txmgr.has_active_transaction()){
  read_transactional();
}else{
  read_global();
}

especially case a) is much more verbose using your interface. every operation 
that requires a transaction (which is most of them) has to check for a 0 
return and manually throw an exception.

> With this interface the functions are clearer. Instead of
>
>  typename detail::transaction_construct_t begin_transaction(){
>   if(this->has_active_transaction()) return typename
> detail::transaction_construct_t(&this->active_transaction()); else return
> typename detail::transaction_construct_t(0);
>  }

implementation details.

>
> > basic_transaction.hpp: the transaction scope class, and the
> > atomic{}retry; macros.
>
> I would separate the language-like macros from the basic_transaction class.

into a seperate header? ok.
I'd also prefer another for the macro itself, considering boost::atomic...

>
> > transaction_manager.hpp: the (configurable) default transaction manager
> >
> > basic_loc.hpp and loc.hpp: an example of a C++98 pseudo-template-alias.
> > note the anonymous namespace and the conversion operators in loc.hpp. I
> > think this technically violates the One Definition Rule but I don't think
> > this actually causes a problem.
>
> If I have understood it will be only one transaction_manager class for an
> application. Why add the template parameter  TxMgr to all the other
> classes? In other words, which class other than transaction_manager can be
> a template for the basic_loc class?
>
> template<class T>
> class loc : public basic_loc<T,transaction_manager>{...}
> template<class T>
> class loc2 : public basic_loc<T,???>{...}

think of loc/loc2 not as classes but as typedefs. (they really are, they are 
only classes to work around the lack of template aliases in C++98.).

you could make up an example that uses 2 different loc's, but a much simpler 
example is 2 different transaction's:
an application might use Boost.Persistent to store some application 
state on disk. then, in a complete different part of the application, 
in another translation unit, someone tries to use Boost.STM and fails, 
because "transaction" is already configured to use Boost.Persistent.
(either because the translation unit includes a header that uses 
Boost.Persistent or because of a linker error because of One Definition Rule 
violation).

>
> template<class TxMgr>
> class basic_transaction;
> typedef basic_transaction<transaction_manager> transaction;
>
> we can have just
>
> class transaction; // using the single transaction_manager

...for the entire application, possibly including linked libraries.

Regards,