Re: [Maria-developers] Architecture review of MWL#132 Transaction coordinator plugin

Hi, Kristian! On Sep 07, Kristian Nielsen wrote:

Sergei Golubchik 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