maria-developers team mailing list archive

[Kristian Nielsen <knielsen@xxxxxxxxxxxxxxx>]

Percona <launchpad@xxxxxxxxxxx> writes:

> Percona has proposed merging lp:~maria-captains/maria/maria-xtradb into lp:maria.
>
> Requested reviews:
>     Maria-captains (maria-captains)
>
> Proposal to merge replacement InnoDB->XtraDB

Thanks a lot for your efforts in this!

I branched the tree and took a look. There are a couple of issues that I think
need to be resolved before we can merge it into MariaDB. I have some questions
below, but please don't hesitate to ask me for any kind of help needed to move
this forward.

> === modified file 'storage/innobase/include/sync0rw.h'
> --- storage/innobase/include/sync0rw.h	2008-02-19 16:44:09 +0000
> +++ storage/innobase/include/sync0rw.h	2009-03-31 04:19:17 +0000

> +#ifndef INNODB_RW_LOCKS_USE_ATOMICS
> +#error INNODB_RW_LOCKS_USE_ATOMICS is not defined. Do you use enough new GCC or compatibles?
> +#error Or do you use exact options for CFLAGS?
> +#error e.g. (for x86_32): "-m32 -march=i586 -mtune=i686"
> +#error e.g. (for Sparc_64): "-m64 -mcpu=v9"
> +#error Otherwise, this build may be slower than normal version.
> +#endif
> +

My attempt to build (BUILD/compile-pentium64-max) failed with this
error. There were also other build errors (I assume places where the
atomics-using code has not been extended with a part that works without the
availability of atomics).

The reason is that in the MariaDB tree, HAVE_GCC_ATOMIC_BUILTINS is disabled,
which caused XtraDB to disable INNODB_RW_LOCKS_USE_ATOMICS, which triggers the
above error.

I think I understand why this makes sense for Percona, after all using these
better synchronisation primitives is part of the reason for using the Percona
server in the first place.

Can you tell me if Percona has decided not to maintain XtraDB working without
the availability of atomic operations? Or if it is just an oversight?

I need to discuss with other MariaDB people whether XtraDB for MariaDB should
be maintained working without the atomic operations (if so, we should of
course be willing to do the work/effort required).

So, any thoughts about the best way to deal with this? Should the above #error
be removed and XtraDB extended to work without atomics in the MariaDB tree?
And is this something Percona wants to do, or should I look into it?

Also, Sergei Golubchik told me that HAVE_GCC_ATOMIC_BUILTINS is for
my_atomic_ops, and InnoDB/XtraDB shouldn't really be using it. But I need to
look more into the code to understand what the problem is, if any.

> === added directory 'storage/innobase/mysql-test'

> === added directory 'storage/innobase/mysql-test/patches'

When I ran the test suite, I got test failures in test main.innodb.

I see that the patch contains patches for main MySQL test cases in
mysql-test/t/*.test, and also seems to add separate test cases in the
storage/innobase/mysql-test/ directory.

Do you know what the status is of these test suite modifications? Do the
patches need to be applied to the existing test suite, and/or should the extra
test cases be used to add to/overwrite the existing tests?

We would need to get the test suite to run without problems before
merging. Does Percona run the test suite with no failures? Can you suggest
which directions I should work in to solve the test failures? Ie. I'm unsure
to what extent the extra test cases/patches have already been applied in main
MySQL sources, and whether failures are expected or are just due to not being
adapted for current MariaDB source changes.

Any help with the above would be great. I plan to continue working with you on
this so we can get it merged without unnecessary delays.

 - Kristian.