Re: [Maria-developers] [Merge] lp:~maria-captains/maria/maria-xtradb into lp:maria
Percona 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.h2008-02-19 16:44:09 + > +++ storage/innobase/include/sync0rw.h2009-03-31 04:19:17 + > +#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. ___ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
Re: [Maria-developers] [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 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 + >> +++ storage/innobase/include/sync0rw.h 2009-03-31 04:19:17 + > >> +#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.mysqlperformanc
Re: [Maria-developers] [Merge] lp:~maria-captains/maria/maria-xtradb into lp:maria
Vadim Tkachenko writes: > 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. Yes, understood. > XtraDB can be built without INNODB_RW_LOCKS_USE_ATOMICS, but effect may > be opposite - we can see slowdown instead of improvement. Yes. I think there are two issues here. One is that we are replacing InnoDB with XtraDB in MariaDB. Thus we need XtraDB to build and work on all the platforms that MariaDB should work on. The other is to get good performance/scalability of XtraDB. This is very important on the major platforms, but maybe not on all platforms. So it seems that right now XtraDB relies on GCC intrinsics for the performance improvements. I personally think this is fine. We can build with GCC on the main performance platforms I think. The main non-GCC platform currently is Windows, so we need XtraDB to build and work with non-GCC, but I think for now 8CPU+ scalability on Windows is not the most important priority. Later we can then add Windows method for scalability, or maybe switch to also build with GCC on Windows. > 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. Ok. I will also look closer as time permits, checking the ./configure tests (already discussed this a bit with Sergei Golubchik) and the build failures when INNODB_RW_LOCKS_USE_ATOMICS is not defined. > 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. Ok. I understand why you keep this separate for XtraDB, thanks for the info. I will try this. It seems to me that once we replace InnoDB with XtraDB in the MariaDB tree, we can commit the changes done by setup.sh, do you agree? > 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. Ok, I can hopefully take a look later, maybe we need to disable them until Oracle/Innobase team fixes them. - Kristian. ___ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
Re: [Maria-developers] [Merge] lp:~maria-captains/maria/maria-xtradb into lp:maria
Kristian Nielsen writes: > I think there are two issues here. > > One is that we are replacing InnoDB with XtraDB in MariaDB. Thus we need > XtraDB to build and work on all the platforms that MariaDB should work on. > > The other is to get good performance/scalability of XtraDB. This is very > important on the major platforms, but maybe not on all platforms. > > So it seems that right now XtraDB relies on GCC intrinsics for the performance > improvements. I personally think this is fine. We can build with GCC on the > main performance platforms I think. The main non-GCC platform currently is > Windows, so we need XtraDB to build and work with non-GCC, but I think for now > 8CPU+ scalability on Windows is not the most important priority. > > Later we can then add Windows method for scalability, or maybe switch to also > build with GCC on Windows. > >> 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. > > Ok. > > I will also look closer as time permits, checking the ./configure tests > (already discussed this a bit with Sergei Golubchik) and the build failures > when INNODB_RW_LOCKS_USE_ATOMICS is not defined. Just to avoid duplicate work, let me just mention that I am now starting to look into this issue. I will try to find a good way to make XtraDB compile/work on all the platforms/compilers we want to support, and report back what I discover. - Kristian. ___ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
Re: [Maria-developers] [Merge] lp:~maria-captains/maria/maria-xtradb into lp:maria
Kristian, Ok, thank you. Please let me know if some code changes required from our side! Kristian Nielsen wrote: Kristian Nielsen writes: I think there are two issues here. One is that we are replacing InnoDB with XtraDB in MariaDB. Thus we need XtraDB to build and work on all the platforms that MariaDB should work on. The other is to get good performance/scalability of XtraDB. This is very important on the major platforms, but maybe not on all platforms. So it seems that right now XtraDB relies on GCC intrinsics for the performance improvements. I personally think this is fine. We can build with GCC on the main performance platforms I think. The main non-GCC platform currently is Windows, so we need XtraDB to build and work with non-GCC, but I think for now 8CPU+ scalability on Windows is not the most important priority. Later we can then add Windows method for scalability, or maybe switch to also build with GCC on Windows. 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. Ok. I will also look closer as time permits, checking the ./configure tests (already discussed this a bit with Sergei Golubchik) and the build failures when INNODB_RW_LOCKS_USE_ATOMICS is not defined. Just to avoid duplicate work, let me just mention that I am now starting to look into this issue. I will try to find a good way to make XtraDB compile/work on all the platforms/compilers we want to support, and report back what I discover. - 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/ ___ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
Re: [Maria-developers] [Merge] lp:~maria-captains/maria/maria-xtradb into lp:maria
Kristian Nielsen writes: > Just to avoid duplicate work, let me just mention that I am now starting to > look into this issue. I will try to find a good way to make XtraDB > compile/work on all the platforms/compilers we want to support, and report > back what I discover. I got some good progress on this. I re-did the merge using the merge-into bzr plugin (advise from bzr developers). This should allow us to merge directly from the lp:percona-xtradb tree into lp:maria in the future. I placed the files in storage/xtradb/ and left storage/innodb/ in place. This should hopefully make merging from MySQL slightly easier, and seems to work well (and it is kind of nice also). I integrated the XtraDB testsuite into the tree so there is no need to run setup.sh or anything. I also fixed a number of test suite failures, and even a small bug or two. I fixed the code to compile without GCC atomic operation intrinsics. Performance will be worse of course, but it is good to have it at least working. I still need to fix Maria configure.in to not disable GCC atomics for xtradb by default. I pushed the result so far to Launchpad: lp:~maria-captains/maria/mariadb-xtradb-merge2 In particular you might be interested in the changeset 2702: http://bazaar.launchpad.net/~maria-captains/maria/mariadb-xtradb-merge2/revision/2702 Maybe you will want to merge some of that back into XtraDB? Comments are welcome, of course. I think that after fixing GCC atomics in configure this should be ready to push into MariaDB 5.1. Would be good to get a review of especially the 2702 commit. - Kristian. ___ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
Re: [Maria-developers] [Merge] lp:~maria-captains/maria/maria-xtradb into lp:maria
Kristian, Thank you for looking into this! I will look how we can backport your changes. We need to have xtradb compiled against standard 5.1, if it is not broken - it should be not problem to include your fixes. Thanks! Vadim Kristian Nielsen wrote: > Kristian Nielsen writes: > >> Just to avoid duplicate work, let me just mention that I am now starting to >> look into this issue. I will try to find a good way to make XtraDB >> compile/work on all the platforms/compilers we want to support, and report >> back what I discover. > > I got some good progress on this. I re-did the merge using the merge-into bzr > plugin (advise from bzr developers). This should allow us to merge directly > from the lp:percona-xtradb tree into lp:maria in the future. > > I placed the files in storage/xtradb/ and left storage/innodb/ in place. This > should hopefully make merging from MySQL slightly easier, and seems to work > well (and it is kind of nice also). > > I integrated the XtraDB testsuite into the tree so there is no need to run > setup.sh or anything. I also fixed a number of test suite failures, and even a > small bug or two. > > I fixed the code to compile without GCC atomic operation > intrinsics. Performance will be worse of course, but it is good to have it at > least working. I still need to fix Maria configure.in to not disable GCC > atomics for xtradb by default. > > I pushed the result so far to Launchpad: > > lp:~maria-captains/maria/mariadb-xtradb-merge2 > > In particular you might be interested in the changeset 2702: > > > http://bazaar.launchpad.net/~maria-captains/maria/mariadb-xtradb-merge2/revision/2702 > > Maybe you will want to merge some of that back into XtraDB? > > Comments are welcome, of course. I think that after fixing GCC atomics in > configure this should be ready to push into MariaDB 5.1. Would be good to get > a review of especially the 2702 commit. > > - 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/ ___ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
Re: [Maria-developers] [Merge] lp:~maria-captains/maria/maria-xtradb into lp:maria
Kristian Nielsen writes: > I got some good progress on this. I re-did the merge using the merge-into bzr > plugin (advise from bzr developers). This should allow us to merge directly > from the lp:percona-xtradb tree into lp:maria in the future. I just tested this, and it seems to work ok. I was able to merge latest XtraDB with just `bzr merge lp:parcona-xtradb`. And merging that into latest MariaDB also worked fine with just `bzr merge`. There were a number of conflicts in the merges. But I think that is only expected, and unavoidable in fact. Eg. when same problem was fixed in different ways in the different trees, or changes to InnoDB plugin files removed in MariaDB tree. Nothing serious, and we can reduce some of this by cooperating closely between XtraDB and MariaDB (and Sun and Oracle if they are willing). - Kristian. ___ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
Re: [Maria-developers] [Merge] lp:~maria-captains/maria/maria-xtradb into lp:maria
On Tue, Jun 09, 2009 at 05:30:04PM +0200, Kristian Nielsen wrote: > I fixed the code to compile without GCC atomic operation > intrinsics. Performance will be worse of course, but it is good to have it at > least working. I still need to fix Maria configure.in to not disable GCC > atomics for xtradb by default. We have some headers in drizzle so we get atomic ops through the SunStudio provided way as well (or gcc). may want to look it up for better Solaris perf. -- Stewart Smith ___ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
