- developers - lists.mariadb.org

Re: [Maria-developers] Please review: EXPLAIN UPDATE patch + refactoring
by Sergei Petrunia 28 Jun '13

28 Jun '13

Now with attachment. On Fri, Jun 28, 2013 at 09:18:56PM +0400, Sergei Petrunia wrote: > Hello Igor, > > Please review the attached patch against current 10.0-base which contains > - EXPLAIN UPDATE/DELETE support > - "Query Plan Footprint" refactoring. > > The tree is at lp:~maria-captains/maria/10.0-base-explain-update-r2 > > BR Sergei -- Sergei Petrunia, Software Developer Monty Program AB, http://askmonty.org Blog: http://s.petrunia.net/blog

1 0

Re: [Maria-developers] Rev 3768: Changed acl_roles to be stored into a HASH
by Sergei 28 Jun '13

28 Jun '13

Hi, Vicențiu! A couple of small notes: On Jun 27, noreply(a)launchpad.net wrote: > ------------------------------------------------------------ > revno: 3768 > committer: Vicențiu Ciorbaru <cvicentiu(a)gmail.com> > branch nick: trunk > timestamp: Fri 2013-06-28 01:20:45 +0300 > message: > Changed acl_roles to be stored into a HASH. > > === modified file 'sql/sql_acl.cc' > --- sql/sql_acl.cc 2013-06-25 21:49:50 +0000 > +++ sql/sql_acl.cc 2013-06-27 22:20:45 +0000 > @@ -51,6 +51,7 @@ > #include "hostname.h" > #include "sql_db.h" > #include "sql_array.h" > +#include "sql_hset.h" You should probably include hash.h not sql_hset.h because you don't use Hash_set<> template. > @@ -520,6 +521,13 @@ > return (uchar*) entry->key; > } > > +uchar* acl_role_get_key(ACL_USER *entry, size_t *length, > + my_bool not_used __attribute__((unused))) > +{ > + *length=(uint) (entry->user ? strlen(entry->user) : 0); You know, we generally try to avoid strlens, especially in a code that's used often, like here. Please, replace char *user; in the ACL_USER with LEX_STRING user; > + return (uchar*) entry->user; > +} > + > #define IP_ADDR_STRLEN (3 + 1 + 3 + 1 + 3 + 1 + 3) > #define ACL_KEY_LENGTH (IP_ADDR_STRLEN + 1 + NAME_LEN + \ > 1 + USERNAME_LENGTH + 1) Regards, Sergei

1 0

Re: [Maria-developers] Review request: mdev-4634
by Alexander Barkov 27 Jun '13

27 Jun '13

Hi Sergei, Please review a small patch that fixes the exact problem reported in 4634 (and nothing else). Thanks. (the other problems from the original big patch are now reported separately, with some of them already fixed). On 06/13/2013 12:08 PM, Sergei Golubchik wrote: > Hi, Alexander! > > On Jun 13, Alexander Barkov wrote: >>> Hi, Alexander! >>> >>> This looks like a lot of changes for what you explained the reason of >>> the bug was. There will be lots of "whys" below... >> >> The fix itself was quite small. However, an attempt >> to write a reasonable test case revealed a few other bugs. >> I just tried to fixing everything in a single patch. > > I understood this and it was okay. > > But I still wanted to know what change fixes what bug and what test > case. > >> Okey. Let's do everything separately. >> These are separate reports for the problems >> found during writing a test case for 4634: >> https://mariadb.atlassian.net/browse/MDEV-4652 >> https://mariadb.atlassian.net/browse/MDEV-4653 >> https://mariadb.atlassian.net/browse/MDEV-4654 >> >> Here's a new one, found today: >> https://mariadb.atlassian.net/browse/MDEV-4651 > > Okay, as you prefer :) > > Regards, > Sergei >

2 1

[Maria-developers] query cache issue
by Roberto Spadim 27 Jun '13

27 Jun '13

guys, i sent some (many) MDEV issues to have a better control over query cache i want to comment here to don't stay with this idea in my mind only... i created some labels in JIRA: querycache - relative to query cache qc_info - relative to qc_info plugin querycache_client - relative to implement query cache at client side (not in server) querycache_control - relative to DELETE queries in cache, DELETE tables in cache, LOCK/UNLOCK QUERY CACHE querycache_insert - relative to prune of queries that should be or shouldn't be cacheable querycache_lowmen - realtive to what we could do if we get low men in query cache querycache_result - features relative to when should or shouldn't hit query cache when query is cached if they will be or not implemented is another history is labels ok? should i remove it? -- Roberto Spadim

2 5

Re: [Maria-developers] Rev 3766: Added separation between roles and users in the mysql.user table
by Sergei 26 Jun '13

26 Jun '13

Hi, Vicențiu! On Jun 25, noreply(a)launchpad.net wrote: > ------------------------------------------------------------ > revno: 3766 > committer: Vicențiu Ciorbaru <cvicentiu(a)gmail.com> > branch nick: trunk > timestamp: Tue 2013-06-25 23:52:30 +0300 > message: > Added separation between roles and users in the mysql.user table > modified: > sql/sql_acl.cc > > === modified file 'sql/sql_acl.cc' > --- sql/sql_acl.cc 2013-06-23 12:25:04 +0000 > +++ sql/sql_acl.cc 2013-06-25 20:52:30 +0000 > @@ -542,7 +542,16 @@ > #endif /* HAVE_OPENSSL && !EMBEDDED_LIBRARY */ > #define NORMAL_HANDSHAKE_SIZE 6 > > -static DYNAMIC_ARRAY acl_hosts, acl_users, acl_dbs, acl_proxy_users; > +static DYNAMIC_ARRAY acl_hosts, acl_users, acl_dbs, acl_proxy_users, acl_roles; > +/* XXX > + ***** Potential optimization ***** > + role_grants could potentially be a HASH with keys as usernames and values > + as a DYNAMIC_ARRAY of pointers to granted roles > + XXX > + ***** Implementation choice for now ***** > + An array representing mappings between acl_users and acl_roles; > +*/ Agree about a hash. Hosts, users and dbs are stored in an array, because they support wildcards in MariaDB, not only exact matches. And very specific sort order, from most specific to least specific. But we only need exact matches for role names, so you can put all roles in a hash (see HASH or Hash_set). > +static DYNAMIC_ARRAY role_grants; Same. Do you need an array here? Is the order of entries important? I don't think it is. In fact, you probably may store role<->user relationships directly in the ACL_USER structure. For a user - list of granted roles. For a role - list of users it is granted to. Then it should be easy to check if a given user can SET a given role. > static MEM_ROOT mem, memex; > static bool initialized=0; > static bool allow_all_hosts=1; > @@ -876,11 +891,20 @@ > while (!(read_record_info.read_record(&read_record_info))) > { > ACL_USER user; > + bool is_role= FALSE; > bzero(&user, sizeof(user)); > update_hostname(&user.host, get_field(&mem, table->field[0])); > user.user= get_field(&mem, table->field[1]); > - DBUG_PRINT("info", ("user_entry %s", user.user)); > - if (check_no_resolve && hostname_requires_resolving(user.host.hostname)) > + > + /* If the user entry is a role, skip password and hostname checks > + A user can not log in with a role so some checks are not necessary > + */ > + if (!user.host.hostname) { > + is_role= TRUE; I'm sorry, but it wouldn't do. Apparently it is possible to create a user with the empty host name: MariaDB [test]> create user foo@''; Query OK, 0 rows affected (0.00 sec) MariaDB [test]> select user,host from mysql.user; +------+-----------+ | user | host | +------+-----------+ | foo | | Looking at compare_hostname() function, one can see that empty hostname means "any hostname is ok", it's a wildcard. It means, you cannot use empty hostnames to distinguish between roles and users. Which is a pity, I'd also prefer to use an empty hostname for that. You can use a NULL hostname (you'd need ALTER TABLE to make the column nullable) or a separate column, like, Is_role ENUM('N','Y'). > + } > + > + if (!is_role && check_no_resolve && > + hostname_requires_resolving(user.host.hostname)) Regards, Sergei

1 0

Re: [Maria-developers] Rev 3517: MySQL 5.6.10 performance schema: merge of host_cache table
by Sergei Golubchik 25 Jun '13

25 Jun '13

Hi, Timour! Just few comments On Jun 25, timour(a)askmonty.org wrote: > ------------------------------------------------------------ > revno: 3517 > revision-id: timour(a)askmonty.org-20130621115451-tyxgeeroqyxwz7gn > parent: monty(a)askmonty.org-20130623091543-zqu5dvhry6okna6g > committer: timour(a)askmonty.org > branch nick: 10.0-perf-schema-merge > timestamp: Fri 2013-06-21 14:54:51 +0300 > message: > MySQL 5.6.10 performance schema: merge of host_cache table > === modified file 'sql/hostname.cc' > --- a/sql/hostname.cc 2013-01-15 18:13:32 +0000 > +++ b/sql/hostname.cc 2013-06-21 11:54:51 +0000 > @@ -24,7 +24,7 @@ > Hostnames are checked with reverse name lookup and checked that they > doesn't resemble an IP address. > */ > - > +#include "my_global.h" > #include "sql_priv.h" > #include "hostname.h" > #include "my_global.h" you don't need to include my_global.h twice here > === modified file 'sql/sql_acl.cc' > --- a/sql/sql_acl.cc 2013-06-17 23:01:34 +0000 > +++ b/sql/sql_acl.cc 2013-06-21 11:54:51 +0000 > @@ -8730,7 +8737,6 @@ static int server_mpvio_read_packet(MYSQ > err: > if (mpvio->status == MPVIO_EXT::FAILURE) > { > - inc_host_errors(mpvio->thd->security_ctx->ip); > if (!mpvio->thd->is_error()) > { > if (mpvio->make_it_fail) > @@ -8890,6 +8896,10 @@ static int do_auth_once(THD *thd, const > else > { > /* Server cannot load the required plugin. */ > + Host_errors errors; > + errors.m_no_auth_plugin= 1; > + // merge-todo: struct MPVIO_EXT’ has no member named ‘ip’ > + // inc_host_errors(mpvio->ip, &errors); see above, the line you've removed in a previous chunk, used inc_host_errors(mpvio->thd->security_ctx->ip); > my_error(ER_PLUGIN_IS_NOT_LOADED, MYF(0), auth_plugin_name->str); > res= CR_ERROR; > } > @@ -8913,8 +8923,6 @@ static int do_auth_once(THD *thd, const > Perform the handshake, authorize the client and update thd sctx variables. > > @param thd thread handle > - @param connect_errors number of previous failed connect attemps > - from this host really? you've removed it from the comment, but not from the function, so now acl_authenticate() has one undocumented argument. > @param com_change_user_pkt_len size of the COM_CHANGE_USER packet > (without the first, command, byte) or 0 > if it's not a COM_CHANGE_USER (that is, if > @@ -9017,8 +9025,27 @@ bool acl_authenticate(THD *thd, uint con > > if (res > CR_OK && mpvio.status != MPVIO_EXT::SUCCESS) > { > + Host_errors errors; > DBUG_ASSERT(mpvio.status == MPVIO_EXT::FAILURE); > - > + switch (res) > + { > + case CR_AUTH_PLUGIN_ERROR: > + errors.m_auth_plugin= 1; > + break; > + case CR_AUTH_HANDSHAKE: > + errors.m_handshake= 1; > + break; > + case CR_AUTH_USER_CREDENTIALS: > + errors.m_authentication= 1; > + break; > + case CR_ERROR: > + default: > + /* Unknown of unspecified auth plugin error. */ > + errors.m_auth_plugin= 1; > + break; > + } > + // merge-todo: struct MPVIO_EXT’ has no member named ‘ip’ > + // inc_host_errors(mpvio.ip, &errors); same as above. And personally, I'd rather make a method errors.set_error(res); and move the switch inside. But you may decide to stay closer to 5.6 code. > if (!thd->is_error()) > login_failed_error(thd); > DBUG_RETURN(1); > @@ -9256,23 +9283,20 @@ static int native_password_authenticate( > #endif > > if (pkt_len == 0) /* no password */ > - DBUG_RETURN(info->auth_string[0] ? CR_ERROR : CR_OK); > + DBUG_RETURN(mpvio->acl_user->salt_len != 0 ? CR_AUTH_USER_CREDENTIALS : CR_OK); Why did you change the condition? Our pluggable auth code is not identical to MySQL's, so not all changes can be copied verbatim from there. I think the old condition was ok. > info->password_used= PASSWORD_USED_YES; > if (pkt_len == SCRAMBLE_LENGTH) > { > if (!mpvio->acl_user->salt_len) > - DBUG_RETURN(CR_ERROR); > + DBUG_RETURN(CR_AUTH_USER_CREDENTIALS); > > - if (check_scramble(pkt, thd->scramble, mpvio->acl_user->salt)) > - DBUG_RETURN(CR_ERROR); > - else > - DBUG_RETURN(CR_OK); > + DBUG_RETURN(check_scramble(pkt, thd->scramble, mpvio->acl_user->salt) ? > + CR_AUTH_USER_CREDENTIALS : CR_OK); could you change it back please and only replace the error code? if() is easier to debug, than a function call inside DBUG_RETURN(). I suppose that's why we have it that way. > } > > - inc_host_errors(mpvio->thd->security_ctx->ip); > my_error(ER_HANDSHAKE_ERROR, MYF(0)); > - DBUG_RETURN(CR_ERROR); > + DBUG_RETURN(CR_AUTH_HANDSHAKE); > } Regards, Sergei

1 0

[Maria-developers] Debug interface
by Roberto Spadim 25 Jun '13

25 Jun '13

hi guys, what ide could i use to debug easily in linux mariadb version? i'm using gdb via command line, but it's very complicate, i'm a visual studio user, but a console interface is nice, if i could put some break points and F5 (run),F8 (run one step) there's another place to ask about it? -- Roberto Spadim

3 2

[Maria-developers] RFC: Pausing a query thread
by Marian Marinov 25 Jun '13

25 Jun '13

Hi guys, I work with hosting companies and all of my clients have the same irritating problem, one query taking a lot of time to finish and consuming a lot of resources. So I have solved the same issue but with processes on the machine by simply pausing(kill -STOP) these processes when the load is high and continuing(kill -CONT) them when the server load goes down. I want to implement the same logic within MariaDB, so I can STOP/CONT the thread that is executing this query. My idea is to implement this in the 'KILL query_id' function, so it will accept one more argument, which will be either STOP or CONT. If the parameter is not present, the default behavior should remain. What do you think about my approach? Marian

6 18

Re: [Maria-developers] Help with MDEV-4419, ENUM optimization
by Roberto Spadim 24 Jun '13

24 Jun '13

hummm :) nice, i will study it =) it give me a lot of speed, since some system are closed source and i can't rewrite the query, just change the mysql server or put a mysql proxy

1 5

[Maria-developers] users table
by Roberto Spadim 24 Jun '13

24 Jun '13

hi guys, what's the better way to have a shared mysql schema? i'm talking about creating a user in one database and this user be created in many databases (in the same server) i'm trying with symbolic links in data directory, but sure i have some problems that i need same server version (ok no problem) and flush tables in all servers after add an user i don't know if i could do a two replications since it's like a master-master storage when one database add a user all databases need that information any idea? -- Roberto Spadim

4 4