- developers - lists.mariadb.org

[Maria-developers] MDEV-5273
by Alexey Botchkov 05 Feb '14

05 Feb '14

Hi, Serg, all. I'm not sure how about fixing this one: https://mariadb.atlassian.net/browse/MDEV-5273 So would like your opinion. The thing is that the SHOW commands with old-fasioned implementations (those that aren't mapped to the SELECT FROM information_schema.something) return stmt->field_count == 0 after the mysql_prepare_statement. The fix i see involves some boring coding. Is it worth to get into it right now? Best regards. HF

2 1

Re: [Maria-developers] [Commits] Rev 3966: MDEV-4439 ALTER TABLE .. [ADD|DROP] FOREIGN KEY IF [NOT] EXISTS does not work if constraint name is not used. in file:///home/hf/wmar/10-hf/
by Jan Lindström 04 Feb '14

04 Feb '14

Hi, In my opinion the test case is not complete, you are missing e.g. ALTER TABLE t2 ADD FOREIGN KEY IF NOT EXISTS (id) REFERENCES t1(id); -- should fail on warning ALTER TABLE t2 ADD FOREIGN KEY IF NOT EXISTS (id) REFERENCES t1(id); ALTER TABLE t2 ADD FOREIGN KEY IF NOT EXISTS a(id REFERENCES t1(id); -- should fail, there is already foreign key for id, yes it name is not b or a but anyway ALTER TABLE t2 ADD FOREIGN KEY IF NOT EXISTS b(id) REFERENCES t1(id); R: > At file:///home/hf/wmar/10-hf/ > > ------------------------------------------------------------ > revno: 3966 > revision-id: holyfoot(a)askmonty.org-20140201114304-7bi0ug2yg3eyyhlr > parent: psergey(a)askmonty.org-20140121100700-9g8hl8vx8nmk1e2w > committer: Alexey Botchkov <holyfoot(a)askmonty.org> > branch nick: 10-hf > timestamp: Sat 2014-02-01 15:43:04 +0400 > message: > MDEV-4439 ALTER TABLE .. [ADD|DROP] FOREIGN KEY IF [NOT] EXISTS does not work if constraint name is not used. > Patches for server and the Innodb engine. > Server is fixed so it does nothing if no indexes left to alter. > Innodb parser is fixed so it looks for the IF [NOT] EXISTS option in a string. > Another change is that it uses the index name for the internal dictionary. > Prior to that it only used the CONSTRAINT name for it. > > > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits -- -- Jan Lindström Principal Engineer MariaDB | MaxScale | skype: jan_p_lindstrom www.skysql.com <http://www.skysql.com/> Twitter <http://twitter.com/skysql> Blog <http://www.skysql.com/blog/> Facebook <http://www.facebook.com/skysql> LinkedIn <http://www.linkedin.com/company/1214250> Google+ <https://plus.google.com/117544963211695643458/posts>

2 3

[Maria-developers] Extra copying in debian/rules causes troubles
by Elena Stepanova 04 Feb '14

04 Feb '14

Hi all. There is a fragment in MariaDB debian/dist/*/rules files: ... ifeq ($(findstring nocheck,$(DEB_BUILD_OPTIONS)),) # Don't know why the following is necessary... cp unittest/unit.pl $(builddir)/unittest/ cp -r mysql-test/* $(builddir)/mysql-test/ cp -r sql/share/* $(builddir)/sql/share/ cp -r scripts/*sql $(builddir)/scripts/ if [ ! -f testsuite-stamp ] ; then \ ... Recently it started causing a problem. MTR assumes that the tests are run either in a proper source build (be it in-source or out-of-source), or in a binary build. With the extra-copying above, particularly due to copying of mysql-test folder, the builddir by the moment of running tests is in the "neither fish nor fowl" state. So if one runs dpkg-buildpackage with the tests enabled, it fails when it comes to the MTR stage. We had this problem reported at least 3 times over the last 24 hours, so it's not hypothetical. It's currently filed as https://mariadb.atlassian.net/browse/MDEV-5591. I tried to remove these 4 'cp' lines and the build seemed to work all right; but there might also be consequences that I fail to predict, so I would like to have the 2nd (3rd, 4th, ... Nth) opinion. Does anyone have any idea what might go wrong without these 4 lines? Also, *Kristian*, These lines are coming from the initial draft of the rules. I understand from the comment that there is no reason to ask *why* the lines were necessary, but I'm wondering if you remember why you felt they *were* necessary (even if didn't know why). Were you getting some errors without them? Or were they copied from some previous version of the file? I realize it was long time ago so you probably won't remember, but I thought it was worth checking anyway. Regards, Elena

3 4

[Maria-developers] MariaDB 5.5.35
by ImperialNetwork 04 Feb '14

04 Feb '14

Hi, I started to notice this on my error log since i upgraded from 5.5.34 to 5.5.35: 140203 5:26:25 InnoDB: Failed to set O_DIRECT on file /dev/shm/#sql7e8f6_1a94bc_10.ibd: CREATE: Invalid argument, continuing anyway 140203 5:26:25 InnoDB: O_DIRECT is known to result in 'Invalid argument' on Linux on tmpfs, see MySQL Bug#26662 I never seen this error/warning until 5.5.35. Should this be normal ? Thank you in advance. Alex

3 3

[Maria-developers] MDEV-5450 Assertion ... mysql_type_to_time_type(cached_field_type) == ltime.time_type' fails ...
by Alexander Barkov 04 Feb '14

04 Feb '14

Hi Sergei, Please review a patch for mdev-5450. Thanks.

2 3

[Maria-developers] MariaDB build dependencies
by Otto Kekäläinen 03 Feb '14

03 Feb '14

Hello, The original MariaDB debian/control in bzr/mariadb-5.5 lists many more dependencies than what are likely to be needed: Build-Depends: libtool (>= 1.4.2-7), procps | hurd, debhelper, file (>= 3.28), libncurses5-dev (>= 5.0-6), perl (>= 5.6.0), libwrap0-dev (>= 7.6-8.3), zlib1g-dev (>= 1:1.1.3-5), ${LIBREADLINE_DEV}, libevent-dev, libssl-dev, libpam0g-dev, psmisc, po-debconf, chrpath, automake1.9, doxygen, texlive-latex-base, ghostscript | gs-gpl, dpatch, gawk, bison, lsb-release, hardening-wrapper, ${CMAKE_DEP}libaio-dev The source builds error free with just: libaio-dev libncurses5-dev perl libwrap0-dev zlib1g-dev libreadline-gplv2-dev po-debconf psmisc chrpath gawk bison lsb-release hardening-wrapper cmake libssl-dev libboost1.48-dev libpam0g-dev Can anybody tell me how in particular these are supposed to affect the build process? - libevent-dev - doxygen, texlive-latex-base, ghostscript | gs-gpl - gawk Debian MySQL team has a page https://wiki.debian.org/Teams/MySQL/BuildDependencies but obiviously MariaDB is different in these aspects, for example MariaDB needs libpam0g-dev for auth_pam.so which does not exist in MySQL. If anybody can explain the above that would be great, thanks!

3 3

Re: [Maria-developers] Rev 3966: MDEV-4439 ALTER TABLE .. [ADD|DROP] FOREIGN KEY IF [NOT] EXISTS does not work if constraint name is not used
by Sergei Golubchik 02 Feb '14

02 Feb '14

Hi, Holyfoot! On Feb 01, holyfoot(a)askmonty.org wrote: > revno: 3966 > revision-id: holyfoot(a)askmonty.org-20140201114304-7bi0ug2yg3eyyhlr > parent: psergey(a)askmonty.org-20140121100700-9g8hl8vx8nmk1e2w > committer: Alexey Botchkov <holyfoot(a)askmonty.org> > branch nick: 10-hf > timestamp: Sat 2014-02-01 15:43:04 +0400 > message: > MDEV-4439 ALTER TABLE .. [ADD|DROP] FOREIGN KEY IF [NOT] EXISTS does not work if constraint name is not used. > Patches for server and the Innodb engine. > Server is fixed so it does nothing if no indexes left to alter. > Innodb parser is fixed so it looks for the IF [NOT] EXISTS option in a string. > Another change is that it uses the index name for the internal dictionary. > Prior to that it only used the CONSTRAINT name for it. ok to push Regards, Sergei

1 0

Re: [Maria-developers] Rev 3886: MDEV-4439 ALTER TABLE .. [ADD|DROP] FOREIGN KEY IF [NOT] EXISTS does not work if constraint name is not used
by Sergei Golubchik 01 Feb '14

01 Feb '14

Hi, Holyfoot! On Jan 31, holyfoot(a)askmonty.org wrote: > At file:///home/hf/wmar/mdev-4439/ > > ------------------------------------------------------------ > revno: 3886 > revision-id: holyfoot(a)askmonty.org-20140131134044-ej3ftpf240xbn0lc > parent: knielsen(a)knielsen-hq.org-20131107065240-gxxg4ilcs8o2v0q2 > committer: Alexey Botchkov <holyfoot(a)askmonty.org> > branch nick: mdev-4439 > timestamp: Fri 2014-01-31 17:40:44 +0400 > message: > MDEV-4439 ALTER TABLE .. [ADD|DROP] FOREIGN KEY IF [NOT] EXISTS does not work if constraint name is not used. > Patches for server and the Innodb engine. > Server is fixed so it does nothing if no indexes left to alter. > Innodb parser is fixed so it looks for the IF [NOT] EXISTS option, takes into account > the drop->if_not_exists parameter. > Another change is that it uses the index name for the internal dictionary. > Prior to that it only used the CONSTRAINT name for it. That looks pretty much ok, but please add tests that use generated FK names. For example, it doesn't look like you handle the case of ADD FOREIGN KEY IF NOT EXISTS existing_generated_name (...) Regards, Sergei

1 0

[Maria-developers] buildbot for non ~maria-captains branches on launchpad
by Alexander Barkov 31 Jan '14

31 Jan '14

Hello Daniel, Would it be possible to have buildbot watch branches which are on Launchpad, but which are outside of the /~maria-captains/ directory? For example: bzr+ssh://bazaar.launchpad.net/~mroonga/maria/mroonga/ ? Thanks.

3 5

Re: [Maria-developers] Rev 3965: MDEV-5403 - Reduce usage of LOCK_open: tc_count
by Sergei Golubchik 31 Jan '14

31 Jan '14

Hi, Sergey! Please, see below. On Jan 30, Sergey Vojtovich wrote: > revno: 3965 > revision-id: svoj(a)mariadb.org-20140130123642-wsysj31e1wwlqz0o > parent: psergey(a)askmonty.org-20140120123657-g1z4g11xmsdiw2dc > committer: Sergey Vojtovich <svoj(a)mariadb.org> > branch nick: 10.0 > timestamp: Thu 2014-01-30 16:36:42 +0400 > message: > MDEV-5403 - Reduce usage of LOCK_open: tc_count > > Lock-free tc_count. > === modified file 'sql/table_cache.cc' > --- a/sql/table_cache.cc 2013-12-12 17:49:14 +0000 > +++ b/sql/table_cache.cc 2014-01-30 12:36:42 +0000 > @@ -173,7 +175,9 @@ void tc_purge(void) > { > share->tdc.all_tables.remove(table); > purge_tables.push_front(table); > - tc_count--; > + my_atomic_rwlock_wrlock(&LOCK_tdc_atomics); > + my_atomic_add32(&tc_count, -1); > + my_atomic_rwlock_wrunlock(&LOCK_tdc_atomics); Okay, so in this new code tc_count is always equal or greater than the actual number of tables in the cache, right? (because you first remove the table from the cache, then decrement the counter) > } > } > tdc_it.deinit(); > @@ -245,47 +249,52 @@ static void check_unused(THD *thd) > > void tc_add_table(THD *thd, TABLE *table) > { > + bool need_purge; > DBUG_ASSERT(table->in_use == thd); > mysql_mutex_lock(&LOCK_open); > table->s->tdc.all_tables.push_front(table); > + mysql_mutex_unlock(&LOCK_open); > + > /* If we have too many TABLE instances around, try to get rid of them */ > - if (tc_count == tc_size) > + my_atomic_rwlock_wrlock(&LOCK_tdc_atomics); > + need_purge= my_atomic_add32(&tc_count, 1) >= (int32) tc_size; Hmm, but here you increment after adding a table. I'd suggest to keep tc_count consistently either always no smaller than the number of tables in the cache (that is, decrement after removing, increment before adding), or always no larger than that (derement before removing, increment after adding). > + my_atomic_rwlock_wrunlock(&LOCK_tdc_atomics); > + > + if (need_purge) > { > + TABLE *purge_table= 0; > + TABLE_SHARE *share; > TDC_iterator tdc_it; > - mysql_mutex_unlock(&LOCK_open); > > tdc_it.init(); > mysql_mutex_lock(&LOCK_open); > - if (tc_count == tc_size) > - { > - TABLE *purge_table= 0; > - TABLE_SHARE *share; > while ((share= tdc_it.next())) > { > TABLE_SHARE::TABLE_list::Iterator it(share->tdc.free_tables); > TABLE *entry; > while ((entry= it++)) > if (!purge_table || entry->tc_time < purge_table->tc_time) > purge_table= entry; > } > + tdc_it.deinit(); > + > if (purge_table) > { > - tdc_it.deinit(); > purge_table->s->tdc.free_tables.remove(purge_table); > purge_table->s->tdc.all_tables.remove(purge_table); > mysql_rwlock_rdlock(&LOCK_flush); > mysql_mutex_unlock(&LOCK_open); > intern_close_table(purge_table); > mysql_rwlock_unlock(&LOCK_flush); > + > + my_atomic_rwlock_wrlock(&LOCK_tdc_atomics); > + my_atomic_add32(&tc_count, -1); > + my_atomic_rwlock_wrunlock(&LOCK_tdc_atomics); > check_unused(thd); > - return; > } > - } > - tdc_it.deinit(); > - } > - /* Nothing to evict, increment tc_count. */ > - tc_count++; > + else > mysql_mutex_unlock(&LOCK_open); > + } > } > > > @@ -357,25 +366,35 @@ bool tc_release_table(TABLE *table) > DBUG_ASSERT(table->in_use); > DBUG_ASSERT(table->file); > > + if (table->needs_reopen() || tc_records() > tc_size) > + { > + mysql_mutex_lock(&LOCK_open); > + table->in_use= 0; > + goto purge; > + } Why did you move this up? > table->tc_time= my_interval_timer(); > > mysql_mutex_lock(&LOCK_open); > table->in_use= 0; > - if (table->s->has_old_version() || table->needs_reopen() || tc_count > tc_size) > - { > - tc_count--; > + if (table->s->has_old_version()) > + goto purge; > + /* Add table to the list of unused TABLE objects for this share. */ > + table->s->tdc.free_tables.push_front(table); > + mysql_mutex_unlock(&LOCK_open); > + check_unused(thd); > + return false; > + > +purge: > + my_atomic_rwlock_wrlock(&LOCK_tdc_atomics); > + my_atomic_add32(&tc_count, -1); > + my_atomic_rwlock_wrunlock(&LOCK_tdc_atomics); > table->s->tdc.all_tables.remove(table); and now you decrement before removal > mysql_rwlock_rdlock(&LOCK_flush); > mysql_mutex_unlock(&LOCK_open); > intern_close_table(table); > mysql_rwlock_unlock(&LOCK_flush); > return true; > - } > - /* Add table to the list of unused TABLE objects for this share. */ > - table->s->tdc.free_tables.push_front(table); > - mysql_mutex_unlock(&LOCK_open); > - check_unused(thd); > - return false; > } > > Regards, Sergei

2 1

[Maria-developers] FreeBSD compilation problems and a fix
by Alexander Barkov 31 Jan '14

31 Jan '14

Hi guys, I just tried to compile 10.0 on FreeBSD-9.1 RELEASE and noticed two problems: 1. It printed this warning for every source file: /usr/include/sys/timeb.h:42:2: warning: #warning "this file includes <sys/timeb.h> which is deprecated" This diff fixes the problem: -#ifdef HAVE_SYS_TIMEB_H +#if defined(HAVE_SYS_TIMEB_H) && !defined(__FreeBSD__) 2. It failed to compile because of pcre (both libraries and includes) installed in the system. Compiler used the *system* header pcre.h together with the *bundled* library libpcre.a. I had a similar complaint from the MacPorts maintainers. This diff fixes the problem: # Common defines and includes +SET(CMAKE_INCLUDE_DIRECTORIES_PROJECT_BEFORE ON) ADD_DEFINITIONS(-DHAVE_CONFIG_H) INCLUDE_DIRECTORIES(${CMAKE_CURRENT_BINARY_DIR}/include Does this look OK? If so, which version should I push it in? Thanks.

2 3

Re: [Maria-developers] Rev 3966: MDEV-5492 - Reduce usage of LOCK_open: TABLE::in_use
by Sergei Golubchik 30 Jan '14

30 Jan '14

Hi, Sergey! On Jan 29, Sergey Vojtovich wrote: > At lp:maria/10.0 > > ------------------------------------------------------------ > revno: 3966 > revision-id: svoj(a)mariadb.org-20140129083115-jwye7zfyncrq9crs > parent: svoj(a)mariadb.org-20140124075326-d7twer07e0bu42b5 > committer: Sergey Vojtovich <svoj(a)mariadb.org> > branch nick: 10.0 > timestamp: Wed 2014-01-29 12:31:15 +0400 > message: > MDEV-5492 - Reduce usage of LOCK_open: TABLE::in_use > > Move TABLE::in_use out of LOCK_open. > > This is done with assumtion that foreign threads accessing TABLE::in_use > will only need consistent value _after_ marking table for flush and purging > unused table instances. In this case TABLE::in_use will always point to a > valid thread object. I believe I was able to understand what you were doing here :) Seems ok. The assumption above looks a bit risky without any way to enforce it. Perhaps you could make in_use private (renaming to, say, m_in_use) and add an inline getter method TABLE::in_use() that in debug builds will assert that your assumption holds? Regards, Sergei

2 2

[Maria-developers] MDEV-4309, second patch: review
by Sergei Petrunia 29 Jan '14

29 Jan '14

Hello, This is a review for MDEV-4309, second patch: https://mariadb.atlassian.net/secure/attachment/21510/201303271828_sanja_co… === The name "memcmp_func" is confusing, because the function has nothing to do with memcmp. Did you mean memcpy actually? The function should be renamed to something like "memcpy_field_value". memcmp_func() still makes one virtual call. === Consider a scenario: - Item_field::setup_cpy_optimizer() is called. - It calls memcmp_possible(), which returns FALSE, which means field_conv() will be called for every row. - field_conv() will still call memcpy_field_value(), even if we know that it will return false. Why not add a field_conv_incompatible() and make field_conv look like this: find_conv() { if (memcmp_possible()) { ... } return field_conv_incompatible(); } === The name "cpy_optimizer" is very cryptic. Is "Fast_field_copier" better? === class Item_field has this member: Field *cpy_optimizer_setup; I don't see how it is useful without its corresponding 'cpy_optimizer' data. cpy_optimizer is only present inside ORDER structure which has both: cpy_optimizer cpy_optimizer_func; Field *cpy_optimizer_setup; If ORDER has both members, why have the Item_field::cpy_optimizer_setup ? === Let's discuss this on IRC. BR Sergei -- Sergei Petrunia, Software Developer MariaDB | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

Re: [Maria-developers] single thread performance (was: Re: Need to check if the slowdown is still there)
by Sergei Petrunia 29 Jan '14

29 Jan '14

Hi Axel, On Tue, Jan 28, 2014 at 04:27:10PM +0100, Axel Schwenke wrote: > Hi, > > I have now re-run a benchmark to test single thread performance. This is the > continuation of MDEV-4309. > Thanks for the data. > ... > The numbers demonstrate quite clearly the general observation that with each > new major version of MariaDB/MySQL the code path becomes longer and the > performance suffers. But there is also good news: sometimes during the > maturing of a release the performance may increase again. I.e. I have seen > significant changes from MySQL-5.6.13 to 5.6.14. I agree with the observations. I would even say that MySQL was getting worse in 5.6.11 - 5.6.13 versions, and then fixe something in 5.6.14. > Also from MariaDB-5.5.30 to > 5.5.31 (which was when MDEV-4309 was implemented). Unfortunately between > MariaDB-5.5.33 and 5.5.34 most of that was lost again :( The first step in not letting that happen is to have the benchmark data so that we know if there are any problems. The patch for MDEV-4309 part#2 is not yet ready. We will get back to you when it can be used for testing. BR Sergei -- Sergei Petrunia, Software Developer MariaDB | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

[Maria-developers] The "Monty Program AB" vendor check (rpm)
by Oden Eriksson 28 Jan '14

28 Jan '14

Hello, I got assigned to look at the rpm packaging and fixed some stuff there, but got stuck at the "Monty Program AB" vendor check in support-files/rpm/server- prein.sh when I wanted to replace the fedora packages with in this case mariadb galera packages. Is this check absolutely nessesary? This prevents you from an easy switch from the fedora mariadb packages to the ones provided by mariadb using yum and a yum repository as provided at http://yum.mariadb.org/. Basically this check prevents me from doing any serious work with the rpm packages because you manually have to, by force deinstall any conflicting packages and then install the packages provided by us. Cheers.

3 3

Re: [Maria-developers] Review request for a fix for MDEV-5506
by Sergei Golubchik 28 Jan '14

28 Jan '14

Hi, Alexander! Few questions, see below. > === modified file 'mysql-test/t/timezone2.test' > --- mysql-test/t/timezone2.test 2013-08-08 08:58:28 +0000 > +++ mysql-test/t/timezone2.test 2014-01-23 10:34:46 +0000 > @@ -298,5 +298,11 @@ SELECT CONVERT_TZ(TIME('00:00:00'),'+00: > SELECT CONVERT_TZ(TIME('2010-01-01 00:00:00'),'+00:00','+7:5'); > > --echo # > +--echo # MDEV-5506 safe_mutex: Trying to lock unitialized mutex at safemalloc.c on server shutdown after SELECT with CONVERT_TZ > +--echo # > +SELECT CONVERT_TZ('2001-10-08 00:00:00', MAKE_SET(0,'+01:00'), '+00:00' ); hmm, the bug description was "at shutdown". I don't see a shutdown in your test case. I suppose, you assume that the server will be eventually shut down and that'll do. But perhaps it needs to be shut down immediately, and if mtr will do a lot of work in this server the test case becomes invalid - it won't crash anymore? > === modified file 'sql/item_strfunc.cc' > --- sql/item_strfunc.cc 2013-09-25 12:30:13 +0000 > +++ sql/item_strfunc.cc 2014-01-23 10:17:57 +0000 > @@ -43,7 +43,7 @@ C_MODE_END > /** > @todo Remove this. It is not safe to use a shared String object. > */ I guess, you've fixed that and can remove the comment. > -String my_empty_string("",default_charset_info); > +String_const my_empty_string("", default_charset_info); > > === modified file 'sql/sql_string.h' > --- sql/sql_string.h 2012-11-09 08:11:20 +0000 > +++ sql/sql_string.h 2014-01-23 10:26:13 +0000 > @@ -56,25 +56,52 @@ uint convert_to_printable(char *to, size > > class String > { > + typedef enum > + { > + STRING_FLAG_NONE= 0, > + STRING_FLAG_ALLOCED= 1, > + STRING_FLAG_READ_ONLY= 2, > + STRING_FLAG_NULL_TERMINATED= 4 > + } flag_t; > char *Ptr; > uint32 str_length,Alloced_length, extra_alloc; > - bool alloced; > + flag_t flags; > CHARSET_INFO *str_charset; Okay, I understand what you did. But I don't understand why. What was the problem? Why there was a crash? Regards, Sergei

2 4

[Maria-developers] MariaDB 5.5.35 Release Prep
by Daniel Bartholomew 28 Jan '14

28 Jan '14

All, I've begun the prep work for the MariaDB 5.5.35 release. The draft changelog and release notes are here: - https://mariadb.com/kb/en/mariadb-5535-changelog/ - https://mariadb.com/kb/en/mariadb-5535-release-notes/ Please let me know ASAP if there's anything that needs to be mentioned/highlighted/removed/etc... The release is still building in buildbot. If all goes well, the plan is to begin uploading the files to the mirrors later today and then to activate the release as soon as most mirrors have picked up the files. Thanks. -- Daniel Bartholomew MariaDB

1 0

[Maria-developers] 5.5.35 Release ?
by ImperialNetwork 28 Jan '14

28 Jan '14

Hi, I was wondering if MariaDB 5.5.35 is going to be released today as stated on jira page. I plan some maintenance and i will do it as soon as 5.5.35 will be released. Thank you in advance Alexandru

2 1

Re: [Maria-developers] Rev 4009: MDEV-5345 - Deadlock between mysql_change_user(), SHOW VARIABLES
by Sergei Golubchik 28 Jan '14

28 Jan '14

Hi, Sergey! Ok to push. A couple of comments below: On Jan 27, Sergey Vojtovich wrote: > revno: 4009 > revision-id: svoj(a)mariadb.org-20140127090430-g63lt751yxw9ddud > parent: sergii(a)pisem.net-20131217162654-dw2zlm3td1p12bxl > committer: Sergey Vojtovich <svoj(a)mariadb.org> > branch nick: 5.5-mdev5345 > timestamp: Mon 2014-01-27 13:04:30 +0400 > message: > MDEV-5345 - Deadlock between mysql_change_user(), SHOW VARIABLES and > INSTALL PLUGIN > > === modified file 'sql/sql_plugin.cc' > --- a/sql/sql_plugin.cc 2013-12-12 17:14:08 +0000 > +++ b/sql/sql_plugin.cc 2014-01-27 09:04:30 +0000 > @@ -1196,6 +1190,9 @@ static void plugin_deinitialize(struct s > if (ref_check && plugin->ref_count) > sql_print_error("Plugin '%s' has ref_count=%d after deinitialization.", > plugin->name.str, plugin->ref_count); > + > + mysql_del_sys_var_chain(plugin->system_vars); > + restore_pluginvar_names(plugin->system_vars); Can mysql_del_sys_var_chain() be called from inside restore_pluginvar_names() ? They seem always to be called together. > @@ -1342,7 +1335,8 @@ void plugin_unlock_list(THD *thd, plugin > } > > > -static int plugin_initialize(struct st_plugin_int *plugin) > +static int plugin_initialize(MEM_ROOT *tmp_root, struct st_plugin_int *plugin, > + int *argc, char **argv, bool options_only) > { I thought you'll rename a function :) But that's just fine too, whatever you prefer. Regards, Sergei

1 0

Re: [Maria-developers] MDEV-5345 - Deadlock between mysql_change_user(), SHOW VARIABLES
by Sergei Golubchik 27 Jan '14

27 Jan '14

Hi, Sergey! Looks ok, see a couple of question (and minor style-related comments) below. On Dec 27, Sergey Vojtovich wrote: > At lp:maria/5.5 > > ------------------------------------------------------------ > revno: 4009 > revision-id: svoj(a)mariadb.org-20131227101419-ahpdahcundclr6gv > parent: sergii(a)pisem.net-20131217162654-dw2zlm3td1p12bxl > committer: Sergey Vojtovich <svoj(a)mariadb.org> > branch nick: 5.5-mdev5345 > timestamp: Fri 2013-12-27 14:14:19 +0400 > message: > MDEV-5345 - Deadlock between mysql_change_user(), SHOW VARIABLES and > INSTALL PLUGIN > > There was mixed lock order between LOCK_plugin, LOCK_global_system_variables > and LOCK_system_variables_hash. This patch ensures that write-lock on > LOCK_system_variables_hash doesn't intersect with LOCK_plugin. > > Fixed by moving initialization/deinitialization of plugin options from > plugin_add()/plugin_del() to plugin_initialize()/plugin_deinitalize(). > So that plugin options are handled without protection of LOCK_plugin. > === added file 'mysql-test/r/plugin_vars.result' > --- a/mysql-test/r/plugin_vars.result 1970-01-01 00:00:00 +0000 > +++ b/mysql-test/r/plugin_vars.result 2013-12-27 10:14:19 +0000 > @@ -0,0 +1,22 @@ > +# > +# MDEV-5345 - Deadlock between mysql_change_user(), SHOW VARIABLES and > +# INSTALL PLUGIN > +# > +CREATE PROCEDURE p_install(x INT) > +BEGIN > +DECLARE CONTINUE HANDLER FOR 1126 BEGIN END; > +WHILE x DO > +SET x= x - 1; > +INSTALL PLUGIN no_such_plugin SONAME 'no_such_object'; > +END WHILE; > +END| > +CREATE PROCEDURE p_show_vars(x INT) > +WHILE x DO > +SET x= x - 1; > +SHOW VARIABLES; > +END WHILE| > +CALL p_install(100);; > +CALL p_show_vars(100);; Note two semicolons at the end. The mysqltest syntax is either builtin_command arguments; or --builtin_command arguments I mean, it either ends with a semicolon or starts with dash-dash. Not a big deal, of course :) > +USE test; > +DROP PROCEDURE p_install; > +DROP PROCEDURE p_show_vars; > > === added file 'mysql-test/t/plugin_vars.test' > --- a/mysql-test/t/plugin_vars.test 1970-01-01 00:00:00 +0000 > +++ b/mysql-test/t/plugin_vars.test 2013-12-27 10:14:19 +0000 > @@ -0,0 +1,56 @@ > +--echo # > +--echo # MDEV-5345 - Deadlock between mysql_change_user(), SHOW VARIABLES and > +--echo # INSTALL PLUGIN > +--echo # > + > +# Prepare test > +delimiter |; > +CREATE PROCEDURE p_install(x INT) > +BEGIN > + DECLARE CONTINUE HANDLER FOR 1126 BEGIN END; > + WHILE x DO > + SET x= x - 1; > + INSTALL PLUGIN no_such_plugin SONAME 'no_such_object'; > + END WHILE; > +END| > + > +CREATE PROCEDURE p_show_vars(x INT) > +WHILE x DO > + SET x= x - 1; > + SHOW VARIABLES; > +END WHILE| > +delimiter ;| > + > +connect(con1, localhost, root,,); > +connect(con2, localhost, root,,); > + > +# Start test > +connection con1; > +--send CALL p_install(100); > + > +connection con2; > +--send CALL p_show_vars(100); Why did you prefer this test over a deterministic one with debug sync points? > +connection default; > + > +disable_result_log; > +let $i= 100; > +while ($i) > +{ > + change_user; > + dec $i; > +} > + > +# Cleanup > +connection con1; > +reap; > +connection con2; > +reap; > +connection default; > +enable_result_log; > + > +disconnect con1; > +disconnect con2; > +USE test; > +DROP PROCEDURE p_install; > +DROP PROCEDURE p_show_vars; > > === modified file 'sql/sql_plugin.cc' > --- a/sql/sql_plugin.cc 2013-12-12 17:14:08 +0000 > +++ b/sql/sql_plugin.cc 2013-12-27 10:14:19 +0000 > @@ -201,7 +202,7 @@ static bool initialized= 0; > write-lock on LOCK_system_variables_hash is required before modifying > the following variables/structures > */ > -static MEM_ROOT plugin_mem_root; > +static MEM_ROOT plugin_vars_mem_root; ok, so you couldn't consistently use one lock for plugin_mem_root, and had to create a second memroot. I see. another option would be to use one memroot and a dedicated lock for it. but I don't think it matters much what solution we use here :) > static uint global_variables_dynamic_size= 0; > static HASH bookmark_hash; > > @@ -1342,7 +1337,8 @@ void plugin_unlock_list(THD *thd, plugin > } > > > -static int plugin_initialize(struct st_plugin_int *plugin) > +static int plugin_initialize(MEM_ROOT *tmp_root, struct st_plugin_int *plugin, > + int *argc, char **argv, bool initialize) this looks pretty weird. plugin_initialize() function that doesn't initialize if its initialize argument is false? perhaps it's better to rename the function then? Or create another one that tests plugin options and invokes plugin_initialize() if necessary? > { > int ret= 1; > DBUG_ENTER("plugin_initialize"); > @@ -1413,6 +1421,14 @@ static int plugin_initialize(struct st_p > ret= 0; > > err: > + if (ret) > + { > + mysql_rwlock_wrlock(&LOCK_system_variables_hash); > + mysql_del_sys_var_chain(plugin->system_vars); > + mysql_rwlock_unlock(&LOCK_system_variables_hash); > + restore_pluginvar_names(plugin->system_vars); may be, you'd move these four lines in a separate function? like, cleanup_plugin_sysvars() ? > + } > + > mysql_mutex_lock(&LOCK_plugin); > plugin->state= state; > > @@ -1646,9 +1654,10 @@ int plugin_init(int *argc, char **argv, > for (i= 0; i < plugin_array.elements; i++) > { > plugin_ptr= *dynamic_element(&plugin_array, i, struct st_plugin_int **); > - if (plugin_ptr->state == PLUGIN_IS_UNINITIALIZED) > + if (plugin_ptr->plugin_dl && plugin_ptr->state == PLUGIN_IS_UNINITIALIZED) Why? > { > - if (plugin_initialize(plugin_ptr)) > + if (plugin_initialize(&tmp_root, plugin_ptr, argc, argv, > + !(flags & PLUGIN_INIT_SKIP_INITIALIZATION))) > { > plugin_ptr->state= PLUGIN_IS_DYING; > *(reap++)= plugin_ptr; Regards, Sergei

2 4

Re: [Maria-developers] Please review MDEV-5458 RQG hits 'sql/tztime.cc:799: my_time_t sec_since_epoch(int, int, int, int, int, int): Assertion `mon > 0 && mon < 13' failed.'
by Sergei Golubchik 27 Jan '14

27 Jan '14

Hi, Alexander! First, please cc: such requests to maria-developers@ instead. Thanks! On Jan 22, Alexander Barkov wrote: > > Please review a patch for a crashing bug MDEV-5458. > > Item_func_min_max::get_date() did not always honor the flags > given (such as NO_ZERO_IN_DATE and NO_ZERO_DATE) > in could erroneously return a not-NULL value instead of NULL. > > The patch moves the call for check_date_with_warn() > *AFTER* conversion from DATETIME to TIME, to make > sure that the returned value always matches the requested > flags. > > Previously it was done before conversion. The value before > conversion could be OK for the flags given, but the value > after conversion could be not OK. > > Also, I added a new function adjust_time_range_with_warn() > to avoid duplicate code. Reused it in three places. All ok. Just one comment: > === modified file 'sql/mysql_priv.h' > --- sql/mysql_priv.h 2013-12-02 11:09:34 +0000 > +++ sql/mysql_priv.h 2014-01-22 14:53:38 +0000 > @@ -2582,6 +2582,7 @@ check_date(const MYSQL_TIME *ltime, ulon > } > bool check_date_with_warn(const MYSQL_TIME *ltime, uint fuzzy_date, > timestamp_type ts_type); > +bool adjust_time_range_with_warn(MYSQL_TIME *ltime, uint dec); > int test_if_number(char *str,int *res,bool allow_wildcards); > void change_byte(uchar *,uint,char,char); > bool init_read_record(READ_RECORD *info, THD *thd, TABLE *reg_form, This is 5.3, right? Take care when merging to 5.5, there's no mysql_priv.h there, only sql_priv.h and this declaration should not end up there, but in sql_time.h Regards, Sergei

2 1

Re: [Maria-developers] Rev 4019: MDEV-5505: Assertion `! is_set()' fails on PREPARE SELECT with out of range in GROUP BY
by Sergei Golubchik 26 Jan '14

26 Jan '14

Hi, Sanja! On Jan 23, sanja(a)askmonty.org wrote: > === modified file 'sql/sql_select.cc' > --- a/sql/sql_select.cc 2014-01-21 13:27:36 +0000 > +++ b/sql/sql_select.cc 2014-01-23 12:50:54 +0000 > @@ -20463,7 +20463,8 @@ find_order_in_list(THD *thd, Item **ref_ > if (!order_item->fixed && > (order_item->fix_fields(thd, order->item) || > (order_item= *order->item)->check_cols(1) || > - thd->is_fatal_error)) > + thd->is_fatal_error || > + thd->is_error())) > return TRUE; /* Wrong field. */ Why not simply > - thd->is_fatal_error)) > + thd->is_error())) can there be a fatal error that is not an error? :) Regards, Sergei

1 0

Re: [Maria-developers] [Commits] Rev 3965: MDEV-5403 - Reduce usage of LOCK_open: tc_count in lp:maria/10.0
by Kristian Nielsen 24 Jan '14

24 Jan '14

Sergey Vojtovich <svoj(a)mariadb.org> writes: > +#ifdef __cplusplus > +template <typename T, int S> class Atomic_type_triat; > + > + > +template <typename T> class Atomic_type_triat<T, 4> > +template <typename T> class Atomic_type_triat<T, 8> > +template <typename T, typename S= Atomic_type_triat<T, sizeof(T)> > class Atomic > + T operator++() { return add(1) + 1; } [lots and lots of more template code snipped] I am vetoing this. We really, _really_ do not want to package up the atomic stuff in even more abstractions and API layers than we already have. Atomics and lock-free stuff is in itself plenty complicated. It is crucial that one can at least see easily in the code what is going on, without also having to spend effort to understand multiple layers of wrapping. As far as I can see, the only thing the templates do is hide stuff (like the data type used and the wrapping in locks on platforms without atomic support?), while what we need is to make those details clear to people reading the code, not to hide them! - Kristian.

3 5

[Maria-developers] benchmarking next steps
by Axel Schwenke 24 Jan '14

24 Jan '14

Hi, for those of you who didn't attend the Barcelona meeting last week, here are the plans for next steps in benchmarking: 1. top priority is automatic running of DBT3 for each new release of MariaDB-10.0 and MySQL-5.6. Probably also for MariaDB-5.5. I'll send out another mail to pinpoint DBT3 conditions (i.e. which table statistics to use etc.) 2. multithreaded replication slave is now in a condition that it can and should be benchmarked 3. the problem pointed out by Yoshinori in http://yoshinorimatsunobu.blogspot.de/2013/12/single-thread-performance-reg… must be checked for MariaDB. Monty announced that he will commit a patch this week that should help with that problem. 4. for the NUMA scalability problem, Kristian will try to find the definitive reason with profiling tools. Besides that ad-hoc testing of new patches from Svoj continues as before. 5. Galera benchmarking will be done by the Galera team for now. XL

5 8

[Maria-developers] MariaDB 10.1 tree
by Sergei Petrunia 22 Jan '14

22 Jan '14

Hi, Unless there are any objections, I am going to create a 10.1 tree on Launchpad. It will be based on current 10.0. BR Sergei -- Sergei Petrunia, Software Developer MariaDB | Skype: sergefp | Blog: http://s.petrunia.net/blog

2 3