- developers - lists.mariadb.org

[Maria-developers] storage engine index usage
by Andras Szabo 06 Feb '15

06 Feb '15

Hi Folks, A new question has risen during the development of our storage engine. In short the engine is providing a read-only view to the server filesystem. One can issue (implemented via assisted discovery): CREATE TABLE etc ENGINE=fsview ROOT=‘/etc’; Then it’s possible to tell: SELECT name, fsize FROM etc ORDER BY fsize DESC LIMIT 10 resulting in: +-----------------+--------+ | name | fsize | +-----------------+--------+ | ld.so.cache | 129417 | | mailcap | 44732 | | fb.modes | 24501 | | mime.types | 24066 | | brltty.conf | 22478 | | services | 19558 | | devscripts.conf | 18037 | | ltrace.conf | 14867 | | login.defs | 10551 | | sensors3.conf | 10344 | +-----------------+--------+ So far so good (the listing is not recursive). An additional feature is to access subdirectories of the root (this is working via automated discovery): SELECT name, fsize FROM `etc/init.d` This is not terribly wrong, but we’re thinking about the possiblity of improving it somehow that: SELECT name, fsize FROM etc WHERE path = “init.d”; However we’re in a bit of trouble how we could improve the storage engine to be able to handle this. Of course we would not like to do a "deep enumeration" of the root directory (full table scan) . So somehow we would like to access/use the information from the where clause. Can anybody recommend a way to solve that? -- Andras Szabo Sent with Airmail

3 2

Re: [Maria-developers] ping
by Sergei Golubchik 05 Feb '15

05 Feb '15

Hi, Vicențiu! Next time, please, cc: (or to: ) your commit to commits(a)mariadb.org. Or use the post-commit trigger from http://bazaar.launchpad.net/~maria-captains/mariadb-tools/trunk/files/head:… > commit 4ede7cdbe51332d1b07def8830974877ebcee936 > Author: Vicențiu Ciorbaru <cvicentiu(a)gmail.com> > Date: Wed Feb 4 23:08:05 2015 +0000 > > MDEV-6918 Create a way to see a user's default role. > > Added an extra column to i_s_applicable_roles, named IS_DEFAULT. > The column displays which role is the default role for the user > querying the table. Looks good, thanks! A couple of minor comments, see below. Ok to push after that. > diff --git a/mysql-test/suite/roles/i_s_applicable_roles_is_default.test b/mysql-test/suite/roles/i_s_applicable_roles_is_default.test > new file mode 100644 > index 0000000..1782f7a > --- /dev/null > +++ b/mysql-test/suite/roles/i_s_applicable_roles_is_default.test > @@ -0,0 +1,61 @@ I'd suggest to start the test with --enable_connect_log it'll make the result file a bit easier to follow. > +create user foo; > +create role role1; > +create role role2; > +create role role3; > + > diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc > index 96fff6d..45ce43d 100644 > --- a/sql/sql_acl.cc > +++ b/sql/sql_acl.cc > @@ -10349,6 +10349,17 @@ applicable_roles_insert(ACL_USER_BASE *grantee, ACL_ROLE *role, void *ptr) > else > table->field[2]->store(STRING_WITH_LEN("NO"), cs); > > + /* Default role is only valid when looking at a role granted to a user. */ > + if (!is_role) > + { > + if (!is_role && data->user->default_rolename.length && you don't need a second !is_role under if(!is_role). I suppose that's a remnant of the first version of your patch, when you returned NO for roles granted to roles :) > + !strcmp(data->user->default_rolename.str, role->user.str)) > + table->field[3]->store(STRING_WITH_LEN("YES"), cs); > + else > + table->field[3]->store(STRING_WITH_LEN("NO"), cs); > + table->field[3]->set_notnull(); > + } > + > if (schema_table_store_record(table->in_use, table)) > return -1; > return 0; Regards, Sergei

1 0

[Maria-developers] Simplify and stabilize build tests in downstream distributions?
by Otto Kekäläinen 05 Feb '15

05 Feb '15

Hello Elena, I asked you on IRC in December about if the mtr run, as it is now called from debian/rules, is optimal and sensible for in-distro build time testing? Canonical staff did these changes to MySQL 5.6, do you think we should follow suite? https://github.com/basak/debian-mysql/commit/6eb941e0f6fc9de35468dc59605208… - see only line 142 https://github.com/basak/debian-mysql/commit/4710f5a3b429c640176fd45b7c4b70… https://github.com/basak/debian-mysql/commit/78d6a4ac03f2e4d1553cc35574fc78…

2 2

Re: [Maria-developers] [Commits] 7320cbf: MDEV-7177 - Server hangs on shutdown after InnoDB error (main.plugin_loaderr
by Sergei Golubchik 04 Feb '15

04 Feb '15

Hi, Sergey! On Feb 02, svoj(a)mariadb.org wrote: > revision-id: 7320cbf85ddfaf7ef76c8f03718819377fd14318 > parent(s): 11deb416fe12ebfd65a1f089c7eb6087c192e2dd > committer: Sergey Vojtovich > branch nick: 10.1 > timestamp: 2015-02-02 15:41:49 +0400 > message: > > MDEV-7177 - Server hangs on shutdown after InnoDB error (main.plugin_loaderr > fails in buildbot) > > There was a race condition in timer functionality of query timeouts. > > Race was as following: > main thread: init_thr_timers() > timer handler thread: my_thread_init() > main thread: end_thr_timer()/timer_thread_state= ABORTING > timer handler thread: timer_thread_state= RUNNING, continue normal op > main thread: waits indefinitely for timer handler thread to go down > > The original idea of the fix is to set RUNNING state in main thread, before > starting timer handler thread. But it didn't survive further cleanups: > - removed "timer_thread_state" and used "thr_timer_inited" for this purpose > - removed unused "timer_thread_running" > - removed code responisble for "timer handler thread" shutdown synchronization, > use pthread_join() instead. That looks ok to me. The only concern is - in end_thr_timer(), the old code had a timeout waiting for a timer thread to die. pthread_join() doesn't. I don't know if that's a problem, perhaps not. Regards, Sergei

2 1

Re: [Maria-developers] [Commits] 11deb41: MDEV-7499 - System variables have broken default values on big endian
by Sergei Golubchik 04 Feb '15

04 Feb '15

Hi, Sergey! On Jan 30, svoj(a)mariadb.org wrote: > revision-id: 11deb416fe12ebfd65a1f089c7eb6087c192e2dd > parent(s): edf34f38ac4fad7996bf19cd9ac669d2a6825400 > committer: Sergey Vojtovich > branch nick: 10.1 > timestamp: 2015-01-30 16:13:28 +0400 > message: > > MDEV-7499 - System variables have broken default values on big endian > > INFORMATION_SCHEMA.SYSTEM_VARIABLES.DEFAULT_VALUE had broken values on > big endian. > > Default value is internally stored as longlong, while I_S references it's > pointer (longlong *) according to variable type (e.g. int, my_bool, etc). This > works well on little endian, but on big endian we always get 0 for such > variables. Ok to push, thanks! One suggestion below > diff --git a/sql/sql_plugin.cc b/sql/sql_plugin.cc > index 46849e2..1928df5 100644 > --- a/sql/sql_plugin.cc > +++ b/sql/sql_plugin.cc > @@ -3254,7 +3254,29 @@ static int pluginvar_sysvar_flags(const st_mysql_sys_var *p) > uchar* sys_var_pluginvar::real_value_ptr(THD *thd, enum_var_type type) > { > if (type == OPT_DEFAULT) > - return (uchar*)&option.def_value; > + { > + switch (plugin_var->flags & PLUGIN_VAR_TYPEMASK) { > + case PLUGIN_VAR_BOOL: > + thd->sys_var_tmp.my_bool_value= option.def_value; > + return (uchar*) &thd->sys_var_tmp.my_bool_value; > + case PLUGIN_VAR_INT: > + thd->sys_var_tmp.int_value= option.def_value; > + return (uchar*) &thd->sys_var_tmp.int_value; > + case PLUGIN_VAR_LONG: > + case PLUGIN_VAR_ENUM: > + thd->sys_var_tmp.long_value= option.def_value; > + return (uchar*) &thd->sys_var_tmp.long_value; > + case PLUGIN_VAR_LONGLONG: > + case PLUGIN_VAR_SET: > + case PLUGIN_VAR_DOUBLE: > + return (uchar*) &option.def_value; PLUGIN_VAR_LONGLONG and PLUGIN_VAR_SET - ok, but for PLUGIN_VAR_DOUBLE I'd either copy it to thd->sys_var_tmp too or add an assert, like this case PLUGIN_VAR_DOUBLE: compile_time_assert(sizeof(double) == sizeof(option.def_value)); /* fall through */ case PLUGIN_VAR_LONGLONG: case PLUGIN_VAR_SET: return (uchar*) &option.def_value; More for documentation purposes than for protection agains a wrong sizeof(double). It explains why you don't copy the value. > + case PLUGIN_VAR_STR: > + thd->sys_var_tmp.ptr_value= (void*) option.def_value; > + return (uchar*) &thd->sys_var_tmp.ptr_value; > + default: > + DBUG_ASSERT(0); > + } > + } > Regards, Sergei

2 1

Re: [Maria-developers] [Commits] 101d7eb: MDEV-4987: Sort by domain_id when list of GTIDs are output
by Kristian Nielsen 03 Feb '15

03 Feb '15

nirbhay(a)mariadb.com writes: > revision-id: 101d7eb963816514362da8a98fc7db7135181910 > committer: Nirbhay Choubey > timestamp: 2015-01-31 21:48:14 -0500 > message: > > MDEV-4987: Sort by domain_id when list of GTIDs are output > > Added logic to sort gtid list based on domain_id before > populating them in string. Added a test case. Thanks! The patch looks ok to push after fixing / considering below in-line comments: > diff --git a/mysql-test/suite/rpl/t/rpl_gtid_sort.test b/mysql-test/suite/rpl/t/rpl_gtid_sort.test > new file mode 100644 > index 0000000..d3b8c6d > --- /dev/null > +++ b/mysql-test/suite/rpl/t/rpl_gtid_sort.test > +--connection server_2 > +SHOW VARIABLES LIKE '%GTID%'; > +#SET GLOBAL gtid_slave_pos=""; This commented-out SET looks like some left-over, probably you meant to remove it? > diff --git a/sql/rpl_gtid.cc b/sql/rpl_gtid.cc > index e5620ec..a0471f3 100644 > --- a/sql/rpl_gtid.cc > +++ b/sql/rpl_gtid.cc > @@ -247,11 +247,13 @@ > { > my_hash_init(&hash, &my_charset_bin, 32, offsetof(element, domain_id), > sizeof(uint32), NULL, rpl_slave_state_free_element, HASH_UNIQUE); > + my_init_dynamic_array(&gtid_cache, sizeof(rpl_gtid), 8, 8, MYF(0)); The name 'cache' confused me slightly, as I think of a cache as something that stores some state between invocations. But it is actually just a temporary buffer used for sorting. A suggestion for a better name could be 'gtid_sort_array'. > rpl_slave_state::~rpl_slave_state() > { > + delete_dynamic(&gtid_cache); > } The other parts of rpl_slave_state are freed in rpl_slave_state::deinit(). I think this is because there is a global variable of this class, and destructors in global variables can cause various issues. So better put this delete_dynamic() into deinit() as well (for consistency, if nothing else). > @@ -705,7 +707,13 @@ class Gtid_db_intact : public Table_check_intact > return sub_id; > } > > +/* A callback used in sorting of gtid list based on domain_id. */ > +static int rpl_gtid_cmp_cb(const void *id1, const void *id2) > +{ > + return (((rpl_gtid *)id1)->domain_id - ((rpl_gtid *)id2)->domain_id); > +} I think this is wrong, you can get signed/unsigned integer overflow (eg. it will consider 0xffffffff as being less than 0x00000000). We might be able to do the subtraction in longlong with a cast (though that would still be susceptible to overflow when converting to the int return value, right?), but it's probably clearer to just do: uint32 d1= ((rpl_gtid *)id1)->domain_id; uint32 d2= ((rpl_gtid *)id2)->domain_id; if (d1 < d2) return -1; else if (d1 > f2) return 1; else return 0; You should probably also modify the test case to have a domain id > 0x80000000, just to check. Otherwise looks good, thanks! - Kristian.

2 2

[Maria-developers] custom storage backend rnd_next() implementation and caching questions
by Andras Szabo 03 Feb '15

03 Feb '15

Hi Everyone, I am developing a custom mariadb storage backend which is intended to give filesystem related information via mariadb tables. For example one could tell: CREATE TEMPORARY TABLE IF NOT EXISTS `/var/log` ENGINE=fsview; which would create a table (using the assisted discovery method) with the following schema: MariaDB [test]> describe `/var`; +-----------+--------------+------+-----+---------+-------+ | Field | Type | Null | Key | Default | Extra | +-----------+--------------+------+-----+---------+-------+ | file_name | varchar(255) | YES | | NULL | | | contents | longblob | YES | | NULL | | +-----------+--------------+------+-----+---------+———+ However i have some implementation issues: 1. I am having trouble implementing rnd_next() in the backend: int ha_fsview::rnd_next(uchar *buf) { DBUG_ENTER(__PRETTY_FUNCTION__); if (!dirp) { DBUG_RETURN(HA_ERR_INTERNAL_ERROR); } if (struct dirent* entry = readdir(dirp)) { uchar *pos = buf; *pos = 0; ++pos; // file name (1bytes size + str) *pos = strlen(entry->d_name); ++pos; size_t bytes_to_copy = strlen(entry->d_name); memcpy(pos, entry->d_name, bytes_to_copy); pos+=bytes_to_copy; DBUG_PRINT("info", ("fsview filename: %s", entry->d_name)); // content (4 bytes size + content ptr) static const std::string fake_content = "hello world"; int s = fake_content.size(); memcpy(pos, &s, sizeof(s)); pos+=sizeof(s); const char* c = fake_content.c_str(); memcpy(pos, &c, sizeof(c)); pos+=sizeof(c); DBUG_RETURN(0); } else { DBUG_RETURN(HA_ERR_END_OF_FILE); } } My problem is that filenames (first field) is ok, but i cannot really see the expected content “hello world” in the second field. I am assuming that i am doing something wrong, but i cannot really find the reason. Could somebody help me, please? 2. If i am issuing: SELECT * FROM `/var`; twice, then the server is not initiating a table scan again on my backend (most likely results from the first run are stored in some cache). So if filesystem contents are changed between the two statements, then it won’t be reflected in the resultset. Of course if i do a: SELECT SQL_NO_CACHE * FROM `/var`; then everything is fine, however i would not like to put the burden on the user to specify the option to not cache the results. My question is whether it’s possible to specify in the storage backend that the results from the tables should not be cached. Thank for the help in advance -- Andras Szabo Sent with Airmail

3 7

Re: [Maria-developers] [Commits] Rev 4577: MDEV-5719: Wrong result with GROUP BY and LEFT OUTER JOIN in lp:~maria-captains/maria/10.0
by Sergey Petrunia 29 Jan '15

29 Jan '15

Hi Sergei, == TL;DR == I have debugged that LEFT JOIN problem. The fix is not pretty, but correct. I will now push it into the 10.0 tree. == Details == So, the commit changes - return (table->null_row ? TRUE : - null_ptr ? MY_TEST(null_ptr[row_offset] & null_bit) : 0); + return real_maybe_null() ? + MY_TEST(null_ptr[row_offset] & null_bit) : table->null_row; The patch changes the behavior for NULLable fields. Suppose * real_maybe_null()= TRUE * table->null_row==TRUE // the table has a NULL-complemented row * (null_ptr[row_offset] & null_bit) == FALSE // however the value // is not null. the old code will return TRUE the new code will return FALSE The new code may seem wrong at first glance (it's a NULL-complemented row, why we return FALSE from is_null()?) However, if we look where table->null_row is set to true, we will find that it is done here: inline void mark_as_null_row(TABLE *table) { table->null_row=1; table->status|=STATUS_NULL_ROW; bfill(table->null_flags,table->s->null_bytes,255); } The function also sets all NULL-bits, so normally one can assume that (table->null_row=TRUE) => ((null_ptr[..] & null_bit) == TRUE) However, there is a case when the above does not hold. It happens when we run GROUP BY query, and GROUP BY is satisfied by scanning an index which produces rows in group order. In that case, the following can occur: consider the query from the testcase: SELECT ... FROM table_one a LEFT JOIN table_two b ON a.oid=b.oid GROUP BY a.oid * we read from a row (1,3) * we make index lookup in table b and find row (3). * row combination a(1,3)-b(3) passes the WHERE clause, GROUP BY execution code records it as current row. * there are no other matches in table b, so we get back to reading table a. * we read row a(1,4) * we look for matches in table b. there are no matches, so we produce a NULL-complemented row b(NULLs) * row combination a(1,4)-b(NULLs) is passed to the GROUP BY runtime. The runtime sees that we are now in a different group, and figures that it should pass the row combination representing the previous group into query output. * It attempts to pass the row combination a(1,3)-b(3) (*) into query output. However, the current row for table b is already the NULL-complemented row. the row combination (*) is stored in the "result_field" objects (and not in "regular field" objects which refer to table->record[0]). It seems wrong that we look at result_field (which points to a row that's different from table->record[0]), and at the same time we are looking at table->null_row, which refers to the record in table->record[0]. However, "result_field" storage has no place for table->null_row bit. The fix basically means: "for NULL-able fields, field's NULL-bit should override table->null_row". Considering the above, it is correct (albeit ugly). Now, what about GROUP BY and NOT NULL fields? I discovered the following: if a table has table->maybe_null, then Field objects that refer to such table and are pointed by Item_field::result_field, are always NULLable. (code-wise, check out setup_copy_fields(), these calls: item->result_field=field->new_field(thd->mem_root,field->table, 1); item->result_field->move_field(copy->to_ptr,copy->to_null_ptr,1); result_field object of NOT NULL column is null-able. On Fri, Jan 23, 2015 at 08:25:59AM -0800, Nirbhay Choubey wrote: > At lp:~maria-captains/maria/10.0 > > ------------------------------------------------------------ > revno: 4577 > revision-id: nirbhay(a)mariadb.com-20150123162551-h88uxn780ivj5k2n > parent: sergii(a)pisem.net-20150119153032-fqrmxjr13z0p1ly0 > committer: Nirbhay Choubey <nirbhay(a)mariadb.com> > branch nick: b5719-10.0 > timestamp: Fri 2015-01-23 11:25:51 -0500 > message: > MDEV-5719: Wrong result with GROUP BY and LEFT OUTER JOIN > > Merged revision 5224 from mysql-5.6 and added a test case. > .. > revno: 5224 > committer: Sergey Glukhov <sergey.glukhov(a)oracle.com> > branch nick: mysql-5.6 > timestamp: Wed 2013-06-19 14:24:08 +0400 > message: > Bug#16620047 INCORRECT QUERY RESULT (AFTER SERVER UPGRADE) > .. > === modified file 'mysql-test/r/group_by_innodb.result' > --- a/mysql-test/r/group_by_innodb.result 2014-01-26 20:49:11 +0000 > +++ b/mysql-test/r/group_by_innodb.result 2015-01-23 16:25:51 +0000 > @@ -57,3 +57,26 @@ > NULL 11.1,22.2 > DROP TABLE t1; > End of 5.5 tests > +# > +# MDEV-5719: Wrong result with GROUP BY and LEFT OUTER JOIN > +# > +CREATE TABLE table_one (oidGroup INT, oid INT PRIMARY KEY)ENGINE=INNODB; > +INSERT INTO table_one VALUES (1,1),(1,2),(1,3),(1,4); > +CREATE TABLE table_two (oid INT PRIMARY KEY)ENGINE=INNODB; > +INSERT INTO table_two VALUES (3); > +SELECT a.oidGroup, a.oid, b.oid FROM table_one a LEFT JOIN table_two b ON > +a.oid=b.oid WHERE a.oidGroup=1; > +oidGroup oid oid > +1 1 NULL > +1 2 NULL > +1 3 3 > +1 4 NULL > +SELECT a.oidGroup, a.oid, b.oid FROM table_one a LEFT JOIN table_two b ON > +a.oid=b.oid WHERE a.oidGroup=1 GROUP BY a.oid; > +oidGroup oid oid > +1 1 NULL > +1 2 NULL > +1 3 3 > +1 4 NULL > +DROP TABLE table_one, table_two; > +# End of tests > > === modified file 'mysql-test/t/group_by_innodb.test' > --- a/mysql-test/t/group_by_innodb.test 2014-01-26 20:49:11 +0000 > +++ b/mysql-test/t/group_by_innodb.test 2015-01-23 16:25:51 +0000 > @@ -67,3 +67,22 @@ > > --echo End of 5.5 tests > > +--echo # > +--echo # MDEV-5719: Wrong result with GROUP BY and LEFT OUTER JOIN > +--echo # > +CREATE TABLE table_one (oidGroup INT, oid INT PRIMARY KEY)ENGINE=INNODB; > +INSERT INTO table_one VALUES (1,1),(1,2),(1,3),(1,4); > + > +CREATE TABLE table_two (oid INT PRIMARY KEY)ENGINE=INNODB; > +INSERT INTO table_two VALUES (3); > + > +# Returns a value > +SELECT a.oidGroup, a.oid, b.oid FROM table_one a LEFT JOIN table_two b ON > +a.oid=b.oid WHERE a.oidGroup=1; > + > +SELECT a.oidGroup, a.oid, b.oid FROM table_one a LEFT JOIN table_two b ON > +a.oid=b.oid WHERE a.oidGroup=1 GROUP BY a.oid; > + > +DROP TABLE table_one, table_two; > + > +--echo # End of tests > > === modified file 'sql/field.h' > --- a/sql/field.h 2014-10-20 12:42:00 +0000 > +++ b/sql/field.h 2015-01-23 16:25:51 +0000 > @@ -656,21 +656,28 @@ > inline bool is_null(my_ptrdiff_t row_offset= 0) const > { > /* > + If the field is NULLable, it returns NULLity based > + on null_ptr[row_offset] value. Otherwise it returns > + NULL flag depending on TABLE::null_row value. > + > The table may have been marked as containing only NULL values > for all fields if it is a NULL-complemented row of an OUTER JOIN > or if the query is an implicitly grouped query (has aggregate > functions but no GROUP BY clause) with no qualifying rows. If > - this is the case (in which TABLE::null_row is true), the field > - is considered to be NULL. > + this is the case (in which TABLE::null_row is true) and the > + field is not nullable, the field is considered to be NULL. > + > + Do not change the order of testing. Fields may be associated > + with a TABLE object without being part of the current row. > + For NULL value check to work for these fields, they must > + have a valid null_ptr, and this pointer must be checked before > + TABLE::null_row. > + > Note that if a table->null_row is set then also all null_bits are > set for the row. > - > - Otherwise, if the field is NULLable, it has a valid null_ptr > - pointer, and its NULLity is recorded in the "null_bit" bit of > - null_ptr[row_offset]. > */ > - return (table->null_row ? TRUE : > - null_ptr ? MY_TEST(null_ptr[row_offset] & null_bit) : 0); > + return real_maybe_null() ? > + MY_TEST(null_ptr[row_offset] & null_bit) : table->null_row; > } > inline bool is_real_null(my_ptrdiff_t row_offset= 0) const > { return null_ptr ? (null_ptr[row_offset] & null_bit ? 1 : 0) : 0; } > > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits -- BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

2 2

[Maria-developers] Dynamic columns support in libmaria client library?
by Peter Laursen 28 Jan '15

28 Jan '15

This page https://mariadb.com/kb/en/mariadb/documentation/nosql/dynamic-columns-api/ does not mention anything about it. it just says *The API is a part of libmysql C client library. In order to use it, one needs to include this header file* *#include <mysql/ma_dyncol.h>* *and link against libmysql.* Now can ma_dyncol.h be linked against libmaria as well? There are 2 questions actually 1) technical: will it work with libmaria? 2) legal: can a client program using libmaria under LGPL license include ma_dyncol.h? -- Peter -- Webyog

3 5

Re: [Maria-developers] [Commits] 51fa027: Improve performance for calculating memory allocation
by Sergei Golubchik 25 Jan '15

25 Jan '15

Hi, Monty! On Jan 23, Michael Widenius wrote: > revision-id: 51fa0272febd782674b3d44828718e805c44c1a8 > parent(s): ab440b0fb7302d707ba0ba41382bf911404db1cb > committer: Michael Widenius > branch nick: maria-10.1 > timestamp: 2015-01-23 14:32:53 +0200 > message: > > Improve performance for calculating memory allocation See my comments below: > diff --git a/include/mysql/plugin.h b/include/mysql/plugin.h > index 16b0c6e..9ae0fe8 100644 > --- a/include/mysql/plugin.h > +++ b/include/mysql/plugin.h > @@ -190,6 +196,7 @@ struct st_mysql_show_var { > > #define SHOW_VAR_FUNC_BUFF_SIZE (256 * sizeof(void*)) > typedef int (*mysql_show_var_func)(MYSQL_THD, struct st_mysql_show_var*, char *); > +typedef int (*mysql_show_var_extended_func)(MYSQL_THD, struct st_mysql_show_var*, enum enum_var_type, char *); I'd put scope the last making mysql_show_var_extended_func backward compatible with mysql_show_var_func. this makes some code simpler, see below. > > /* > diff --git a/sql/sql_show.cc b/sql/sql_show.cc > index 044b60d..8eed7b9 100644 > --- a/sql/sql_show.cc > +++ b/sql/sql_show.cc > @@ -3115,9 +3115,16 @@ static bool show_status_array(THD *thd, const char *wild, > if var->type is SHOW_FUNC or SHOW_SIMPLE_FUNC, call the function. > Repeat as necessary, if new var is again one of the above > */ > - for (var=variables; var->type == SHOW_FUNC || > - var->type == SHOW_SIMPLE_FUNC; var= &tmp) > - ((mysql_show_var_func)(var->value))(thd, &tmp, buff); > + for (var=variables; > + var->type == SHOW_FUNC || var->type == SHOW_SIMPLE_FUNC || > + var->type == SHOW_EXTENDED_FUNC; > + var= &tmp) > + { > + if (var->type == SHOW_EXTENDED_FUNC) > + ((mysql_show_var_extended_func)(var->value))(thd, &tmp, scope, buff); > + else > + ((mysql_show_var_func)(var->value))(thd, &tmp, buff); You won't need this if() when scope is the last argument. In fact I'm still thinking whether it would be a good idea to add scope to mysql_show_var_func and SHOW_FUNC, without introducing SHOW_EXTENDED_FUNC at all. This wouldn't break any existing plugin, as it's perfectly binary compatible extension. Also, see below. > + } > > SHOW_TYPE show_type=var->type; > if (show_type == SHOW_ARRAY) > diff --git a/sql/sql_class.cc b/sql/sql_class.cc > index 0d49407..d200af6 100644 > --- a/sql/sql_class.cc > +++ b/sql/sql_class.cc > @@ -1747,6 +1749,16 @@ void add_to_status(STATUS_VAR *to_var, STATUS_VAR *from_var) > to_var->binlog_bytes_written+= from_var->binlog_bytes_written; > to_var->cpu_time+= from_var->cpu_time; > to_var->busy_time+= from_var->busy_time; > + to_var->local_memory_used+= from_var->local_memory_used; why? it's not additive, right? and global_status_var.local_memory_used is never shown anyway. > + > + /* > + Update global_memory_used. We have to do this with atomic_add as the > + global value can change outside of LOCK_status. > + */ > + // workaround for gcc 4.2.4-1ubuntu4 -fPIE (from DEB_BUILD_HARDENING=1) > + int64 volatile * volatile ptr= &to_var->global_memory_used; I'd rather remove this workaround. 4.2.4 is ancient, and quite probably this bug doesn't happen with MY_MEMORY_ORDER_RELAXED at all > + my_atomic_add64_explicit(ptr, from_var->global_memory_used, > + MY_MEMORY_ORDER_RELAXED); > } > > /* > @@ -1784,6 +1796,11 @@ void add_diff_to_status(STATUS_VAR *to_var, STATUS_VAR *from_var, > dec_var->binlog_bytes_written; > to_var->cpu_time+= from_var->cpu_time - dec_var->cpu_time; > to_var->busy_time+= from_var->busy_time - dec_var->busy_time; > + > + /* > + We don't need to accumulate memory_used as these are not after > + this function is called. Sorry, I cannot parse that :( > + */ > } > > #define SECONDS_TO_WAIT_FOR_KILL 2 > diff --git a/sql/mysqld.cc b/sql/mysqld.cc > index ea53e47..c4e1dc3 100644 > --- a/sql/mysqld.cc > +++ b/sql/mysqld.cc > @@ -3972,19 +3972,24 @@ static void my_malloc_size_cb_func(long long size, my_bool is_thread_specific) > if (thd) > { > DBUG_PRINT("info", ("memory_used: %lld size: %lld", > - (longlong) thd->status_var.memory_used, size)); > + (longlong) thd->status_var.local_memory_used, > + size)); > - thd->status_var.memory_used+= size; > + thd->status_var.local_memory_used+= size; > - DBUG_ASSERT((longlong) thd->status_var.memory_used >= 0); > + DBUG_ASSERT((longlong) thd->status_var.local_memory_used >= 0); > } > } > } > + else if (likely(thd)) > + thd->status_var.global_memory_used+= size; Hmm, ok. So, you think that current_thd is cheaper than updating a global variable? > + else > + { > // workaround for gcc 4.2.4-1ubuntu4 -fPIE (from DEB_BUILD_HARDENING=1) > - int64 volatile * volatile ptr=&global_status_var.memory_used; > + int64 volatile * volatile ptr=&global_status_var.global_memory_used; I would've removed this workaround too > my_atomic_add64_explicit(ptr, size, MY_MEMORY_ORDER_RELAXED); > + } > } > } > > - > static int init_common_variables() > { > umask(((~my_umask) & 0666)); > @@ -8028,6 +8033,21 @@ static int show_default_keycache(THD *thd, SHOW_VAR *var, char *buff) > return 0; > } > > + > +static int show_memory_used(THD *thd, SHOW_VAR *var, enum enum_var_type scope, > + char *buff) > +{ > + var->type= SHOW_LONGLONG; > + var->value= buff; > + if (scope == OPT_GLOBAL) > + *(longlong*) buff= (global_status_var.local_memory_used + > + global_status_var.global_memory_used); > + else > + *(longlong*) buff= thd->status_var.local_memory_used; > + return 0; I'm surprised you haven't added SHOW_EXTENDED_SIMPLE_FUNC :) That would've make a very confusing name though. May be we should indeed add scope as the last argument to mysql_show_var_func and reuse both SHOW_SIMPLE_FUNC and SHOW_FUNC? I'm starting to think it's a good thing to do. Makes the API simpler and the number of choices to understand - smaller. If you do that - please don't forget to increment MARIA_PLUGIN_INTERFACE_VERSION. > +} > + > + > #ifndef DBUG_OFF > static int debug_status_func(THD *thd, SHOW_VAR *var, char *buff) > { Regards, Sergei

1 0