← Back to team overview

maria-developers team mailing list archive

Re: [Merge] lp:~maria-captains/maria/maria-xtradb into lp:maria

 

Kristian,

Uh, Thank you for looking into this.

You ask good question re: INNODB_RW_LOCKS_USE_ATOMICS, this is actually
one of main points of XtraDB to improve scalability on 8CPU+ boxes, so
we just rely on it.

XtraDB can be built without INNODB_RW_LOCKS_USE_ATOMICS, but effect may
be opposite - we can see slowdown instead of improvement.

I am not sure what to do with this, I also need to look into
InnoDB-plugin 1.0.3, what solution they have and may be use new InnoDB
rw_locks instead of ours. Anyway - this is something to discuss.


About tests you should execute
setup.sh
from storage/innobase before running test, as it patches main test suite
by InnoDB specific tests.

I did not want to put them into main test suite to keep all changes only
to storage/innobase repository.

In our tests only two tests do not pass after patches - it is
main.variables_big and main.read_many_rows_innodb, but they also do not
pass in native InnoDB-plugin.

Again what to do with tests it is topic to discuss, I do not see obvious
way for now...

Thanks!
Vadim



Kristian Nielsen wrote:
> 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.
> 


-- 
Vadim Tkachenko, CTO
Percona Inc.
ICQ: 369-510-335, Skype: vadimtk153, Phone +1-888-401-3403
MySQL Performance Blog - http://www.mysqlperformanceblog.com
MySQL Consulting http://www.percona.com/

  Attend the 2009 Percona Performance Conference
  April 22-23 - http://conferences.percona.com/



Follow ups

References