maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxxx>]

Hi, Kristian!

On Sep 07, Kristian Nielsen wrote:
> 
> Sergei Golubchik <serg@xxxxxxxxxxxx> writes:
> 
> > Now, WL#132 - Transaction coordinator plugin
> 
> > Wouldn't it be simpler to create only group_log_xid() interface, no
> > log_and_order() or log_xid() ? The tc plugin gets the list in
> > group_log_xid() - it can reorder the list any way it wants, call
> > prepare_ordered() and commit_ordered() as needed and so on.  In this
> > interpretation, group_log_xid() can meet all the use cases.  And
> > there's no need to create a multitude of methods that one needs to
> > get familiar with before implementing a TC plugin.
> 
> I do not see how this would work. The group_log_xid() interface as
> specified here does not allow the TC to reorder transactions, on the
> contrary the commit order has already been decided by the ordering of
> transactions in the passed list.

Eh. Above I wrote that group_log_xid() calls prepare_ordered() and
commit_ordered() - so it's not the same group_log_xid() that you had in
WL#116. Perhaps it's the same as log_and_order() method, and then we
agree that it's all what is needed.
 
> Something like group_log_xid(list_of_transactions) does not really work here I
> think. Galera may need to reorder a local transaction with another transaction
> that has not even started yet when group_log_xid() is called, so even allowing
> to reorder the passed-in list seems insufficient.

Why, no. Galera may remove some thd's from the list and put them to a
internal tc plugin buffer. That is, it commits only transactions that it
knows when and how to commit. Next time log_and_order() is called,
Galera will add delayed transactions back to the queue, reorder, remove
"not ready" transactions again, and so on.

I mean, log_and_order() seems sufficient, although using it in this
scenario won't be trivial - but it can be expected, the scenario itself
is complex.
 
> >> A TC based on this interface overrides group_log_xid() and
> >> xid_log_after() instead of log_and_order(), and again does not need
> >> to deal with any {prepare,commit}_ordered().
> >
> > Why do you need xid_log_after here ?
> 
> I think the original motivation was that group_log_xid() handles many
> transactions in one thread, so it cannot call my_error() on each
> transaction individually. After all, it is possible for some
> transactions to fail while others succeed.
> 
> So xid_log_after() runs in each individual thread once group_log_xid()
> is done, and can call my_error() for any deferred error.
> 
> But it seems in any case appropriate to have a part of TC logging that
> runs in parallel, giving the TC the opportunity to reduce the amount
> of work done in the critical code path under the global
> LOCK_group_commit mutex. Just like the serialised prepare_ordered()
> and commit_ordered() calls have parallel counterparts prepare() and
> commit().

I'm not completely convinced, but it doesn't matter anyway - if the only
interface function is log_and_order() - then there's no need for
xid_log_after().
 
> >> If need_prepare_ordered or need_commit_ordered is passed as FALSE,
> >> then the corresponding call need not be done. It is safe to do it
> >> anyway, however omitting it avoids the need to take a global mutex.
> >
> > Why would this ever be needed ?  (I mean need_prepare_ordered or
> > need_commit_ordered being FALSE)
> 
> This is for engines that do not install prepare_ordered() and/or
> commit_ordered() methods (or that disables them due to user
> configuration, in case it provides better performance when consistent
> commit order is not needed).
> 
> If these calls are not needed, then log_and_order() can take less
> locks, avoiding LOCK_prepare_ordered and/or LOCK_commit_ordered.

Uhm, I don't know if this case is worth optimizing for.
 
> Well, we already discussed changing LOCK_prepare_ordered to be the
> queue lock, and removing LOCK_commit_ordered completely. That may
> leave nothing to be saved, so I would just remove this.
 
Regards,
Sergei