December 2011 - developers - lists.mariadb.org

Re: [Maria-developers] [Commits] Rev 3020: MWL#192: Non-blocking client API for libmysqlclient. in http://bazaar.launchpad.net/~maria-captains/maria/5.2
by Michael Widenius 09 Jan '12

09 Jan '12

Hi! >>>>> "knielsen" == knielsen <knielsen(a)knielsen-hq.org> writes: knielsen> At http://bazaar.launchpad.net/~maria-captains/maria/5.2 knielsen> ------------------------------------------------------------ knielsen> revno: 3020 knielsen> revision-id: knielsen(a)knielsen-hq.org-20110920104925-huxke80yuq1zrvcx knielsen> parent: monty(a)askmonty.org-20110816160131-4kju8si1nqqnohxg knielsen> committer: knielsen(a)knielsen-hq.org knielsen> branch nick: tmp2 knielsen> timestamp: Tue 2011-09-20 12:49:25 +0200 knielsen> message: knielsen> MWL#192: Non-blocking client API for libmysqlclient. knielsen> All client functions that can block on I/O have alternate _start() and knielsen> _cont() versions that do not block but return control back to the knielsen> application, which can then issue I/O wait in its own fashion and later knielsen> call back into the library to continue the operation. knielsen> Works behind the scenes by spawning a co-routine/fiber to run the knielsen> blocking operation and suspend it while waiting for I/O. This knielsen> co-routine/fiber use is invisible to applications. knielsen> For i368/x86_64 on GCC, uses very fast assembler co-routine support. On knielsen> Windows uses native Win32 Fibers. Falls back to POSIX ucontext on other knielsen> platforms. Assembler routines for more platforms are relatively easy to knielsen> add by extending mysys/my_context.c, eg. similar to the Lua lcoco knielsen> library. knielsen> For testing, mysqltest and mysql_client_test are extended with the knielsen> option --non-blocking-api. This causes the programs to use the knielsen> non-blocking API for database access. mysql-test-run.pl has a similar knielsen> option --non-blocking-api that uses this, as well as additional knielsen> testcases. knielsen> An example program tests/async_queries.c is included that uses the new knielsen> non-blocking API with libevent to show how, in a single-threaded knielsen> program, to issue many queries in parallel against a database. <cut> +++ client/async_example.c 2011-09-20 10:49:25 +0000 @@ -0,0 +1,207 @@ +/* + Copyright 2011 Kristian Nielsen and Monty Program Ab. + + Experiments with non-blocking libmysql. + + This is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 2 of the License, or + (at your option) any later version. + + This is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this. If not, see <http://www.gnu.org/licenses/>. +*/ Please change license to LGPL for this and the other new files (so we can add them to our new LGPL client library) + + +#ifndef __WIN__ +#include <poll.h> +#else +#include <WinSock2.h> +#endif You don't need to incldue Winsock2.h; This is automaticly included by mysql.h. +#include <stdlib.h> +#include <stdio.h> +#include <mysql.h> <cut> === modified file 'client/mysqltest.cc' --- client/mysqltest.cc 2011-05-18 13:17:26 +0000 +++ client/mysqltest.cc 2011-09-20 10:49:25 +0000 @@ -60,6 +60,12 @@ #define SIGNAL_FMT "signal %d" #endif === modified file 'include/Makefile.am' --- include/Makefile.am 2011-05-03 16:10:10 +0000 +++ include/Makefile.am 2011-09-20 10:49:25 +0000 @@ -46,7 +46,7 @@ atomic/rwlock.h atomic/x86-gcc.h \ atomic/generic-msvc.h \ atomic/gcc_builtins.h my_libwrap.h my_stacktrace.h \ - wqueue.h waiting_threads.h + wqueue.h waiting_threads.h my_context.h EXTRA_DIST = mysql.h.pp mysql/plugin_auth.h.pp mysql/client_plugin.h.pp CMakeLists.txt === added file 'include/my_context.h' --- include/my_context.h 1970-01-01 00:00:00 +0000 +++ include/my_context.h 2011-09-20 10:49:25 +0000 @@ -0,0 +1,222 @@ +/* + Copyright 2011 Kristian Nielsen + + Experiments with non-blocking libmysql. + + This is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 2 of the License, or + (at your option) any later version. + + This is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this. If not, see <http://www.gnu.org/licenses/>. +*/ + +/* + Simple API for spawning a co-routine, to be used for async libmysqlclient. + + Idea is that by implementing this interface using whatever facilities are + available for given platform, we can use the same code for the generic + libmysqlclient-async code. + + (This particular implementation uses Posix ucontext swapcontext().) +*/ + +#ifdef __WIN__ +#define MY_CONTEXT_USE_WIN32_FIBERS 1 +#elif defined(__GNUC__) && __GNUC__ >= 3 && defined(__x86_64__) +#define MY_CONTEXT_USE_X86_64_GCC_ASM +#elif defined(__GNUC__) && __GNUC__ >= 3 && defined(__i386__) +#define MY_CONTEXT_USE_I386_GCC_ASM +#else +#define MY_CONTEXT_USE_UCONTEXT +#endif + +#ifdef MY_CONTEXT_USE_WIN32_FIBERS +struct my_context { + void (*user_func)(void *); + void *user_arg; + void *app_fiber; + void *lib_fiber; + int return_value; +#ifndef DBUG_OFF + void *dbug_state; +#endif +}; +#endif + + +#ifdef MY_CONTEXT_USE_UCONTEXT +#include <ucontext.h> + +struct my_context { + void (*user_func)(void *); + void *user_data; + void *stack; + size_t stack_size; + ucontext_t base_context; + ucontext_t spawned_context; + int active; +#ifdef HAVE_VALGRIND_VALGRIND_H + unsigned int valgrind_stack_id; +#endif +#ifndef DBUG_OFF + void *dbug_state; +#endif +}; +#endif + + +#ifdef MY_CONTEXT_USE_X86_64_GCC_ASM +#include <stdint.h> + +struct my_context { + uint64_t save[9]; + void *stack_top; + void *stack_bot; +#ifdef HAVE_VALGRIND_VALGRIND_H + unsigned int valgrind_stack_id; +#endif +#ifndef DBUG_OFF + void *dbug_state; +#endif +}; +#endif + + +#ifdef MY_CONTEXT_USE_I386_GCC_ASM +#include <stdint.h> + +struct my_context { + uint64_t save[7]; + void *stack_top; + void *stack_bot; +#ifdef HAVE_VALGRIND_VALGRIND_H + unsigned int valgrind_stack_id; +#endif +#ifndef DBUG_OFF + void *dbug_state; +#endif +}; +#endif + + +/* + Initialize an asynchroneous context object. + Returns 0 on success, non-zero on failure. +*/ +extern int my_context_init(struct my_context *c, size_t stack_size); + +/* Free an asynchroneous context object, deallocating any resources used. */ +extern void my_context_destroy(struct my_context *c); + +/* + Spawn an asynchroneous context. The context will run the supplied user + function, passing the supplied user data pointer. + + The context must have been initialised with my_context_init() prior to + this call. + + The user function may call my_context_yield(), which will cause this + function to return 1. Then later my_context_continue() may be called, which + will resume the asynchroneous context by returning from the previous + my_context_yield() call. + + When the user function returns, this function returns 0. + + In case of error, -1 is returned. +*/ +extern int my_context_spawn(struct my_context *c, void (*f)(void *), void *d); + +/* + Suspend an asynchroneous context started with my_context_spawn. + + When my_context_yield() is called, execution immediately returns from the + last my_context_spawn() or my_context_continue() call. Then when later + my_context_continue() is called, execution resumes by returning from this + my_context_yield() call. + + Returns 0 if ok, -1 in case of error. +*/ +extern int my_context_yield(struct my_context *c); + +/* + Resume an asynchroneous context. The context was spawned by + my_context_spawn(), and later suspended inside my_context_yield(). + + The asynchroneous context may be repeatedly suspended with + my_context_yield() and resumed with my_context_continue(). + + Each time it is suspended, this function returns 1. When the originally + spawned user function returns, this function returns 0. + + In case of error, -1 is returned. +*/ +extern int my_context_continue(struct my_context *c); + + +struct mysql_async_context { + /* + This is set to the value that should be returned from foo_start() or + foo_cont() when a call is suspended. + It is also set to the event(s) that triggered when a suspended call is + resumed, eg. whether we woke up due to connection completed or timeout + in mysql_real_connect_cont(). + */ + unsigned int ret_status; + /* + This is set to the result of the whole asynchronous operation when it + completes. It uses a union, as different calls have different return + types. + */ + union { + void *r_ptr; + const void *r_const_ptr; + int r_int; + my_bool r_my_bool; + } ret_result; + /* + The timeout value, for suspended calls that need to wake up on a timeout + (eg. mysql_real_connect_start(). + */ + unsigned int timeout_value; + /* + This flag is set when we are executing inside some asynchronous call + foo_start() or foo_cont(). It is used to decide whether to use the + synchronous or asynchronous version of calls that may block such as + recv(). + + Note that this flag is not set when a call is suspended, eg. after + returning from foo_start() and before re-entering foo_cont(). + */ + my_bool active; + /* + This flag is set when an asynchronous operation is in progress, but + suspended. Ie. it is set when foo_start() or foo_cont() returns because + the operation needs to block, suspending the operation. + + It is used to give an error (rather than crash) if the application + attempts to call some foo_cont() method when no suspended operation foo is + in progress. + */ + my_bool suspended; + /* + If non-NULL, this is a pointer to a callback hook that will be invoked with + the user data argument just before the context is suspended, and just after + it is resumed. + */ + void (*suspend_resume_hook)(my_bool suspend, void *user_data); + void *suspend_resume_hook_user_data; + /* + This is used to save the execution contexts so that we can suspend an + operation and switch back to the application context, to resume the + suspended context later when the application re-invokes us with + foo_cont(). + */ + struct my_context async_context; +}; === modified file 'include/my_dbug.h' --- include/my_dbug.h 2011-02-20 16:51:43 +0000 +++ include/my_dbug.h 2011-09-20 10:49:25 +0000 @@ -83,6 +83,8 @@ extern void _db_unlock_file_(void); extern FILE *_db_fp_(void); extern void _db_flush_(); +extern void dbug_swap_code_state(void **code_state_store); +extern void dbug_free_code_state(void **code_state_store); Please define a macros DBUG_FREE_CODE_STATE / DBUG_SWAP_CODE_STATE that are mapped to the aboe if DBUG is defined and to "do {} while (0)" otherwise. This allows you to replace: #ifndef DBUG_OFF dbug_free_code_state(&c->dbug_state); #endif with DBUG_FREE_CODE_STATE(&c->dbug_state); (It's always nice to not have a lot of #ifdef in the code). +++ include/mysql.h 2011-09-20 10:49:25 +0000 void *extension; } MYSQL_PARAMETERS; +/* + Flag bits, the asynchronous methods return a combination of these ORed + together to let the application know when to resume the suspended operation. +*/ +typedef enum { + MYSQL_WAIT_READ= 1, /* Wait for data to be available on socket to read */ + /* mysql_get_socket_fd() will return socket descriptor*/ + MYSQL_WAIT_WRITE= 2, /* Wait for socket to be ready to write data */ + MYSQL_WAIT_EXCEPT= 4, /* Wait for select() to mark exception on socket */ + MYSQL_WAIT_TIMEOUT= 8 /* Wait until timeout occurs. Value of timeout can be */ + /* obtained from mysql_get_timeout_value() */ +} MYSQL_ASYNC_STATUS; + Why have enum's instead of defines ? (As these are bits and you are not going to use 'switch' on these, defines makes more sense). #ifdef USE_OLD_FUNCTIONS MYSQL * STDCALL mysql_connect(MYSQL *mysql, const char *host, const char *user, const char *passwd); +int STDCALL mysql_connect_start(MYSQL **ret, MYSQL *mysql, + const char *host, const char *user, + const char *passwd); +int STDCALL mysql_connect_cont(MYSQL **ret, MYSQL *mysql, + int status); int STDCALL mysql_create_db(MYSQL *mysql, const char *DB); +int STDCALL mysql_create_db_start(int *ret, MYSQL *mysql, + const char *DB); +int STDCALL mysql_create_db_cont(int *ret, MYSQL *mysql, + int status); +int STDCALL mysql_drop_db(MYSQL *mysql, const char *DB); +int STDCALL mysql_drop_db_start(int *ret, MYSQL *mysql, + const char *DB); +int STDCALL mysql_drop_db_cont(int *ret, MYSQL *mysql, int status); int STDCALL mysql_drop_db(MYSQL *mysql, const char *DB); #define mysql_reload(mysql) mysql_refresh((mysql),REFRESH_GRANT) #endif If USE_OLD_FUNCTIONS is defined, you don't have to provide the new api. These are deprecated functions that we don't want people to use so there is no reason to provide 'better' versions of them... === modified file 'include/sql_common.h' --- include/sql_common.h 2010-04-01 09:04:26 +0000 +++ include/sql_common.h 2011-09-20 10:49:25 +0000 @@ -26,11 +26,18 @@ extern const char *cant_connect_sqlstate; extern const char *not_error_sqlstate; + +struct mysql_async_context; + struct st_mysql_options_extention { char *plugin_dir; char *default_auth; }; +struct st_mysql_extension { + struct mysql_async_context *async_context; +}; + Why provide 'st_mysql_extension' in a public file? Isn't it enough to have this only in a local file in the client directory? (We don't want anyone to access this structure in any clients) === modified file 'include/violite.h' --- include/violite.h 2010-01-29 10:42:31 +0000 +++ include/violite.h 2011-09-20 10:49:25 +0000 @@ -194,6 +194,8 @@ char *read_pos; /* start of unfetched data in the read buffer */ char *read_end; /* end of unfetched data */ + struct mysql_async_context *async_context; /* For non-blocking API */ + uint read_timeout, write_timeout; /* function pointers. They are similar for socket/SSL/whatever */ void (*viodelete)(Vio*); int (*vioerrno)(Vio*); Hm, I haven't thought about that it's safe to add new things to struct st_vio without breaking the interface. That's good and useful to know! <cut> <cut> === added file 'mysql-test/t/non_blocking_api.test' --- mysql-test/t/non_blocking_api.test 1970-01-01 00:00:00 +0000 +++ mysql-test/t/non_blocking_api.test 2011-09-20 10:49:25 +0000 @@ -0,0 +1,18 @@ +# Test mixing the use of blocking and non-blocking API in a single connection. + +--enable_non_blocking_api +connect (con_nonblock,localhost,root,,test); +--disable_non_blocking_api +connect (con_normal,localhost,root,,test); + Usually all test starts with drop table if exists t1; This is mostly useful when you want to run the tests against an external server. Not critical but good practice to do... <cut> === added file 'mysys/my_context.c' --- mysys/my_context.c 1970-01-01 00:00:00 +0000 +++ mysys/my_context.c 2011-09-20 10:49:25 +0000 @@ -0,0 +1,749 @@ +/* + Copyright 2011 Kristian Nielsen + + Experiments with non-blocking libmysql. + Add the information text after the copyright. (Is the comment still relevant?) + This is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 2 of the License, or + (at your option) any later version. <cut> + Implementation of async context spawning using Posix ucontext and + swapcontext(). +*/ + +#include <stdio.h> +#include <errno.h> + You can remove the above as mysys_priv.h will include these anyway. +#include "mysys_priv.h" +#include "my_context.h" + +#ifdef HAVE_VALGRIND_VALGRIND_H +#include <valgrind/valgrind.h> +#endif Shouldn't this also be depending on the HAVE_valgrind define? <cut> +int +my_context_init(struct my_context *c, size_t stack_size) +{ + if (2*sizeof(int) < sizeof(void *)) + { + fprintf(stderr, + "Error: Unable to store pointer in 2 ints on this architecture\n"); + return -1; + } Please make the above a compile time error instead. #if SIZEOF_CHARP < SIZEOF_INT*2 #error Error: Unable to store pointer in 2 ints on this architecture #endif I think it would be a good idea to do here: bzero(*c, sizeof(*c)); This is better than assuming that 'c' is zeroed on entry. + if (!(c->stack= malloc(stack_size))) + return -1; /* Out of memory */ + c->stack_size= stack_size; +#ifdef HAVE_VALGRIND_VALGRIND_H + c->valgrind_stack_id= + VALGRIND_STACK_REGISTER(c->stack, ((unsigned char *)(c->stack))+stack_size); +#endif +#ifndef DBUG_OFF + c->dbug_state= NULL; +#endif If we do the bzero, you can remove the above + return 0; +} <cut> +int +my_context_init(struct my_context *c, size_t stack_size) +{ + if (!(c->stack_bot= malloc(stack_size))) + return -1; /* Out of memory */ Add bzero of 'c' here. <cut> +int +my_context_init(struct my_context *c, size_t stack_size) +{ Add bzero() here. <cut> +int +my_context_init(struct my_context *c, size_t stack_size) +{ Add bzero here. +#ifndef DBUG_OFF + c->dbug_state= NULL; +#endif + c->lib_fiber= CreateFiber(stack_size, my_context_trampoline, c); + if (c->lib_fiber) + return 0; + else + return -1; Remove else or do: return c->lib_fiber ? 0 : -1; +} <cut> === modified file 'sql-common/client.c' --- sql-common/client.c 2011-03-18 15:03:43 +0000 +++ sql-common/client.c 2011-09-20 10:49:25 +0000 @@ -108,6 +108,7 @@ #include "client_settings.h" #include <sql_common.h> #include <mysql/client_plugin.h> +#include "my_context.h" Should probably be: #include <my_context.h> +#define mysql_extension_get(MYSQL, X) \ + ((MYSQL)->extension ? (MYSQL)->extension->X : NULL) +#define mysql_extension_set(MYSQL, X, VAL) \ + if (!(MYSQL)->extension) \ + (MYSQL)->extension= (struct st_mysql_extension *) \ + my_malloc(sizeof(struct st_mysql_extension), \ + MYF(MY_WME | MY_ZEROFILL)); \ + (MYSQL)->extension->X= VAL; + In 5.3 and 5.5 we have the following macro: #define extension_set(OPTS, X, VAL) \ if (!(OPTS)->extension) \ (OPTS)->extension= (struct st_mysql_options_extention *) \ my_malloc(sizeof(struct st_mysql_options_extention), \ MYF(MY_WME | MY_ZEROFILL)); \ (OPTS)->extension->X= VAL; Why not also store your new extension in mysql->option->extension instead of mysql->extensions ? This would allow you to reuse the existing macros.. +/* + Fetch the context for asynchronous API calls, allocating a new one if + necessary. +*/ +#define STACK_SIZE (4096*15) + +struct mysql_async_context * +mysql_get_async_context(MYSQL *mysql) +{ + struct mysql_async_context *b; + if ((b= mysql_extension_get(mysql, async_context))) + return b; + + if (!(b= (struct mysql_async_context *) + my_malloc(sizeof(*b), MYF(MY_ZEROFILL)))) + { + set_mysql_error(mysql, CR_OUT_OF_MEMORY, unknown_sqlstate); + return NULL; + } Remove the MY_ZEROFILL and let my_context_init() do it. (Makes the function cleaner). + if (my_context_init(&b->async_context, STACK_SIZE)) + { + my_free(b, MYF(0)); + return NULL; + } <cut> /************************************************************************** Get column lengths of the current row @@ -2537,6 +2577,26 @@ } +static int +connect_sync_or_async(MYSQL *mysql, NET *net, my_socket fd, + const struct sockaddr *name, uint namelen) +{ + extern int my_connect_async(struct mysql_async_context *b, my_socket fd, + const struct sockaddr *name, uint namelen, + uint timeout); Please move the above definitioon to header file that is also read by the file the defines the function. (Otherwise someone can change the function without you getting a compiler error). + struct mysql_async_context *actxt= mysql_extension_get(mysql, async_context); + + if (actxt && actxt->active) + { + my_bool old_mode; + vio_blocking(net->vio, FALSE, &old_mode); + return my_connect_async(actxt, fd, name, namelen, + mysql->options.connect_timeout); + } + else Remove else + return my_connect(fd, name, namelen, mysql->options.connect_timeout); +} + <cut> my_bool mysql_reconnect(MYSQL *mysql) { <cut> + if ((ctxt= mysql_extension_get(mysql, async_context)) && ctxt->active) + { + hook_data.orig_mysql= mysql; + hook_data.new_mysql= &tmp_mysql; + hook_data.orig_vio= mysql->net.vio; + my_context_install_suspend_resume_hook(ctxt, my_suspend_hook, &hook_data); + } if (!mysql_real_connect(&tmp_mysql,mysql->host,mysql->user,mysql->passwd, mysql->db, mysql->port, mysql->unix_socket, mysql->client_flag | CLIENT_REMEMBER_OPTIONS)) { + if (ctxt) + my_context_install_suspend_resume_hook(ctxt, NULL, NULL); Change the above to: res= mysql_real_connect(&tmp_mysql,mysql->host,mysql->user,mysql->passwd, mysql->db, mysql->port, mysql->unix_socket, mysql->client_flag | CLIENT_REMEMBER_OPTIONS); if (ctxt) my_context_install_suspend_resume_hook(ctxt, NULL, NULL); if (!res) .... This way you only need to call my_context_install_suspend_resume_hook(ctxt, NULL, NULL) once. mysql->net.last_errno= tmp_mysql.net.last_errno; strmov(mysql->net.last_error, tmp_mysql.net.last_error); strmov(mysql->net.sqlstate, tmp_mysql.net.sqlstate); @@ -3109,13 +3220,18 @@ if (mysql_set_character_set(&tmp_mysql, mysql->charset->csname)) { DBUG_PRINT("error", ("mysql_set_character_set() failed")); + tmp_mysql.extension= NULL; bzero((char*) &tmp_mysql.options,sizeof(tmp_mysql.options)); mysql_close(&tmp_mysql); + if (ctxt) + my_context_install_suspend_resume_hook(ctxt, NULL, NULL); Please remove the above call. mysql->net.last_errno= tmp_mysql.net.last_errno; strmov(mysql->net.last_error, tmp_mysql.net.last_error); strmov(mysql->net.sqlstate, tmp_mysql.net.sqlstate); DBUG_RETURN(1); } + if (ctxt) + my_context_install_suspend_resume_hook(ctxt, NULL, NULL); Please remove the above call. <cut> +static void +mysql_close_free_extension(MYSQL *mysql) +{ + if (mysql->extension) + { + if (mysql->extension->async_context) + { + my_context_destroy(&mysql->extension->async_context->async_context); + my_free(mysql->extension->async_context, MYF(0)); + } + my_free(mysql->extension, MYF(0)); + mysql->extension= NULL; + } +} I still think it would be better to use mysql->option.extension as in this case you can do the above with 2 lines of code and we would not need another function. <discussions over IRC about this....> The reason for not using mysql->options.extension was only becasue mysql_real_connect() freed the options on failure. This code is not needed as they will be freed by mysql_close() anyway. So I suggest you remove the code: if (!(client_flag & CLIENT_REMEMBER_OPTIONS)) mysql_close_free_options(mysql); from mysql_real_connect() and change to use mysql->options.extension instead static void mysql_close_free(MYSQL *mysql) { my_free((uchar*) mysql->host_info,MYF(MY_ALLOW_ZERO_PTR)); @@ -3304,6 +3439,33 @@ (As some clients call this after mysql_real_connect() fails) */ +/* + mysql_close() can actually block, at least in theory, if the socket buffer + is full when sending the COM_QUIT command. + + On the other hand, the latter part of mysql_close() needs to free the stack + used for non-blocking operation of blocking stuff, so that later part can + _not_ be done non-blocking. + + Therefore, mysql_pre_close() is used to run the parts of mysql_close() that + may block. It can be called before mysql_close(), and in that case + mysql_close() is guaranteed not to need to block. +*/ Consider changing the name to 'mysql_slow_part_of_close()' and make it static if possible. +void mysql_pre_close(MYSQL *mysql) +{ + if (!mysql) + return; Remove test; impossible with current code, see belove in mysql_close. + /* If connection is still up, send a QUIT message */ + if (mysql->net.vio != 0) + { + free_old_query(mysql); + mysql->status=MYSQL_STATUS_READY; /* Force command */ + mysql->reconnect=0; + simple_command(mysql,COM_QUIT,(uchar*) 0,0,1); + end_server(mysql); /* Sets mysql->net.vio= 0 */ + } +} + void STDCALL mysql_close(MYSQL *mysql) { DBUG_ENTER("mysql_close"); @@ -3311,16 +3473,9 @@ + +my_socket STDCALL +mysql_get_socket(const MYSQL *mysql) +{ + if (mysql->net.vio) + return mysql->net.vio->sd; + else remove else + return INVALID_SOCKET; +} +++ sql-common/mysql_async.c 2011-09-20 10:49:25 +0000 +#ifdef __WIN__ +/* + Windows does not support MSG_DONTWAIT for send()/recv(). So we need to ensure + that the socket is non-blocking at the start of every operation. +*/ +#define WIN_SET_NONBLOCKING(mysql) { \ + my_bool old_mode__; \ + if ((mysql)->net.vio) vio_blocking((mysql)->net.vio, FALSE, &old_mode__); \ + } old_mode__ -> old_mode +#else +#define WIN_SET_NONBLOCKING(mysql) +#endif + +extern struct mysql_async_context *mysql_get_async_context(MYSQL *mysql); Move to header file. +/* Asynchronous connect(); socket must already be set non-blocking. */ +int +my_connect_async(struct mysql_async_context *b, my_socket fd, + const struct sockaddr *name, uint namelen, uint timeout) +{ + int res; +#ifdef __WIN__ + int s_err_size; +#else + socklen_t s_err_size; +#endif It would be nice to just define socklen_t for windows, but this isn't critical. You could have a DBUG_ASSERT here to check if socket is really in non blocking. + + /* + Start to connect asynchronously. + If this will block, we suspend the call and return control to the + application context. The application will then resume us when the socket + polls ready for write, indicating that the connection attempt completed. + */ + res= connect(fd, name, namelen); +#ifdef __WIN__ + if (res != 0) + { + int wsa_err= WSAGetLastError(); + if (wsa_err != WSAEWOULDBLOCK) + return res; +#else + if (res < 0) To not have only one { inside the #ifdef / #else I suggest that you move if (res != 0) { before the +#ifdef __WIN__. This works on both windows and Unix This will make the number of { balance + { + if (errno != EINPROGRESS && errno != EALREADY && errno != EAGAIN) + return res; +#endif + b->timeout_value= timeout; + b->ret_status= MYSQL_WAIT_WRITE | + (timeout ? MYSQL_WAIT_TIMEOUT : 0); +#ifdef __WIN__ + b->ret_status|= MYSQL_WAIT_EXCEPT; +#endif How about setting b->ret_status to 0 before res= connect() ? Then you can move the above code to the previous #ifdef __WIN__ and change the setting of b->ret_status to use |= This removes the extra ifdef __WIN__ and also ensures that ret_status is 0 if evertything went well. (not required but still better than a random value when trying to understand what's going in). + if (b->suspend_resume_hook) + (*b->suspend_resume_hook)(TRUE, b->suspend_resume_hook_user_data); + my_context_yield(&b->async_context); + if (b->suspend_resume_hook) + (*b->suspend_resume_hook)(FALSE, b->suspend_resume_hook_user_data); + if (b->ret_status & MYSQL_WAIT_TIMEOUT) + return -1; + + s_err_size= sizeof(int); Better to use (safer and more descriptive): s_err_size= sizeof(res); + if (getsockopt(fd, SOL_SOCKET, SO_ERROR, (char*) &res, &s_err_size) != 0) + return -1; <cut> + +ssize_t +my_recv_async(struct mysql_async_context *b, int fd, + unsigned char *buf, size_t size, uint timeout) +{ + ssize_t res; + + for (;;) + { + res= recv(fd, buf, size, +#ifdef __WIN__ + 0 +#else + MSG_DONTWAIT +#endif + ); Better to do before function start: #ifdef __WIN__ #define MSG_DONTWAIT 0 #endif and remove the above #ifdef in middle of call. Another option is to use the IF_WIN() macro: IF_WIN(0, MSG_DONTWAIT) + if (res >= 0 || +#ifdef __WIN__ + WSAGetLastError() != WSAEWOULDBLOCK +#else + (errno != EAGAIN && errno != EINTR) +#endif Please define macro: #define IS_BLOCKING_ERROR IF_WIN(WSAGetLastError() != WSAEWOULDBLOCK, (errno != EAGAIN && errno != EINTR)) and use that instead of having an ifdef in the middle of the code. + ) + return res; + b->ret_status= MYSQL_WAIT_READ; + if (timeout) + { + b->ret_status|= MYSQL_WAIT_TIMEOUT; + b->timeout_value= timeout; + } + if (b->suspend_resume_hook) + (*b->suspend_resume_hook)(TRUE, b->suspend_resume_hook_user_data); + my_context_yield(&b->async_context); + if (b->suspend_resume_hook) + (*b->suspend_resume_hook)(FALSE, b->suspend_resume_hook_user_data); The following call was quite confusion as we are checking a variable that we where just setting. <phone call> The reasons is that b->ret_status is both an in an out parameter. We agree to do one of the following to make the code easier to understand: - Change to use different bits for in and out status. - Change to use different variables for in and out status. + if (b->ret_status & MYSQL_WAIT_TIMEOUT) + return -1; + } +} + Add empty row here +ssize_t +my_send_async(struct mysql_async_context *b, int fd, + const unsigned char *buf, size_t size, uint timeout) +{ + ssize_t res; + + for (;;) + { + res= send(fd, buf, size, +#ifdef __WIN__ + 0 +#else + MSG_DONTWAIT +#endif + ); + if (res >= 0 || +#ifdef __WIN__ + WSAGetLastError() != WSAEWOULDBLOCK +#else + (errno != EAGAIN && errno != EINTR) +#endif + ) See comments for previous function how to get rid of the ifdefs' <cut> +#ifdef HAVE_OPENSSL +int +my_ssl_read_async(struct mysql_async_context *b, SSL *ssl, + void *buf, int size) +{ + int res, ssl_err; + + for (;;) + { + res= SSL_read(ssl, buf, size); + if (res >= 0) + return res; + ssl_err= SSL_get_error(ssl, res); + if (ssl_err == SSL_ERROR_WANT_READ) + b->ret_status= MYSQL_WAIT_READ; + else if (ssl_err == SSL_ERROR_WANT_WRITE) + b->ret_status= MYSQL_WAIT_WRITE; + else + return res; + if (b->suspend_resume_hook) + (*b->suspend_resume_hook)(TRUE, b->suspend_resume_hook_user_data); + my_context_yield(&b->async_context); + if (b->suspend_resume_hook) + (*b->suspend_resume_hook)(FALSE, b->suspend_resume_hook_user_data); + } +} + +int +my_ssl_write_async(struct mysql_async_context *b, SSL *ssl, + const void *buf, int size) +{ + int res, ssl_err; + + for (;;) + { + res= SSL_write(ssl, buf, size); + if (res >= 0) + return res; + ssl_err= SSL_get_error(ssl, res); + if (ssl_err == SSL_ERROR_WANT_READ) + b->ret_status= MYSQL_WAIT_READ; + else if (ssl_err == SSL_ERROR_WANT_WRITE) + b->ret_status= MYSQL_WAIT_WRITE; + else + return res; + if (b->suspend_resume_hook) + (*b->suspend_resume_hook)(TRUE, b->suspend_resume_hook_user_data); + my_context_yield(&b->async_context); + if (b->suspend_resume_hook) + (*b->suspend_resume_hook)(FALSE, b->suspend_resume_hook_user_data); + } +} 90 % of the above functions are the same: I would do it this way: static my_bool my_ssl_async_check_result(int *res, struct mysql_async_context *b) { int ssl_err; if (*res >= 0) return 1; ssl_err= SSL_get_error(ssl, *res); if (ssl_err == SSL_ERROR_WANT_READ) b->ret_status= MYSQL_WAIT_READ; else if (ssl_err == SSL_ERROR_WANT_WRITE) b->ret_status= MYSQL_WAIT_WRITE; else return 1; if (b->suspend_resume_hook) (*b->suspend_resume_hook)(TRUE, b->suspend_resume_hook_user_data); my_context_yield(&b->async_context); if (b->suspend_resume_hook) (*b->suspend_resume_hook)(FALSE, b->suspend_resume_hook_user_data); return 0; } And then: my_ssl_read_async(struct mysql_async_context *b, SSL *ssl, void *buf, int size) { int res; for (;;) { res= SSL_read(ssl, buf, size); if (my_ssl_async_check_result(res, b)) return res; } } <cut> +#endif /* HAVE_OPENSSL */ + +unsigned int STDCALL +mysql_get_timeout_value(const MYSQL *mysql) +{ + if (mysql->extension && mysql->extension->async_context) + return mysql->extension->async_context->timeout_value; + else Remove else + return 0; +} + Add empty line +/* + Now create non-blocking definitions for all the calls that may block. + + Each call FOO gives rise to FOO_start() that prepares the MYSQL object for + doing non-blocking calls that can suspend operation mid-way, and then starts + the call itself. And a FOO_start_internal trampoline to assist with running + the real call in a co-routine that can be suspended. And a FOO_cont() that + can continue a suspended operation. +*/ + +#define MK_ASYNC_CALLS(call__, decl_args__, invoke_args__, cont_arg__, mysql_val__, parms_mysql_val__, parms_assign__, ret_type__, err_val__, ok_val__, extra1__) \ +static void \ +call__ ## _start_internal(void *d) \ +{ \ As discussed on IRC, it would be good to have the functions declarations outside of the macro; This makes it easier to make breakpoints in the code and help debugging. + struct call__ ## _params *parms; \ + ret_type__ ret; \ + struct mysql_async_context *b; \ + \ + parms= (struct call__ ## _params *)d; \ + b= (parms_mysql_val__)->extension->async_context; \ + \ + ret= call__ invoke_args__; \ + b->ret_result. ok_val__ = ret; \ + b->ret_status= 0; \ +} \ +int STDCALL \ +call__ ## _start decl_args__ \ +{ \ + int res; \ + struct mysql_async_context *b; \ + struct call__ ## _params parms; \ + \ + extra1__ \ + if (!(b= mysql_get_async_context((mysql_val__)))) \ + { \ + *ret= err_val__; \ + return 0; \ + } \ Instead of calling mysql_get_async_context, another option would be to do: mysql_options(mysql, MYSQL_OPT_ASYNC_IO, (char*) &STACK_SIZE) And if STACK_SIZE is 0, then we allocate our default stack size then you can change the above code to: b= mysql_val__->options.extension->async_context; If it doesn't exist, it's ok to crash on access. (Same as if mysql is 0) + parms_assign__ \ + \ + b->active= 1; \ + res= my_context_spawn(&b->async_context, call__ ## _start_internal, &parms);\ + b->active= 0; \ Set also b->suspended= 0 here. (Makes the rest of the code shorter) + if (res < 0) \ + { \ + set_mysql_error((mysql_val__), CR_OUT_OF_MEMORY, unknown_sqlstate); \ How do we know the reason is out of memory? (Just curious) + b->suspended= 0; \ + *ret= err_val__; \ + return 0; \ + } \ + else if (res > 0) \ Remove else + { \ + /* Suspended. */ \ + b->suspended= 1; \ + return b->ret_status; \ + } \ + else \ Remove else and {} + { \ + /* Finished. */ \ + b->suspended= 0; \ + *ret= b->ret_result. ok_val__; \ + return 0; \ + } \ +} \ If you change to test for (res > 0) first, you can combine the two return 0 to one row and thus remove some of the { }. Makes the code shorter (at least to read): b->active= b->suspended= 0; if (res > 0) { b->suspended= 1; return b->ret_status; } if (res < 0) { set_mysql_error((mysql_val__), CR_OUT_OF_MEMORY, unknown_sqlstate); *ret= err_val__; } else *ret= b->ret_result. ok_val__; /* Finished. */ return 0; +int STDCALL \ +call__ ## _cont(ret_type__ *ret, cont_arg__, int ready_status) \ +{ \ + int res; \ + struct mysql_async_context *b; \ + \ + b= (mysql_val__)->extension->async_context; \ + if (!b || !b->suspended) \ Remove test for !b; You can assume it's exists. (We can never protect aginst wrong usage) + { \ + set_mysql_error((mysql_val__), CR_COMMANDS_OUT_OF_SYNC, unknown_sqlstate);\ + *ret= err_val__; \ + return 0; \ + } \ + \ + b->active= 1; \ + b->ret_status= ready_status; \ + res= my_context_continue(&b->async_context); \ + b->active= 0; \ + if (res < 0) \ + { \ + set_mysql_error((mysql_val__), CR_OUT_OF_MEMORY, unknown_sqlstate); \ + b->suspended= 0; \ + *ret= err_val__; \ + return 0; \ + } \ + else if (res > 0) \ + { \ + /* Suspended. */ \ + return b->ret_status; \ + } \ + else \ + { \ + /* Finished. */ \ + b->suspended= 0; \ + *ret= b->ret_result. ok_val__; \ + return 0; \ + } \ You can do the same 'suspend and check for error' optimization I did in previous function to reduce the above code with 4 lines. +} + +#define MK_ASYNC_CALLS_VOID_RETURN(call__, decl_args__, invoke_args__, cont_arg__, mysql_val__, parms_mysql_val__, parms_assign__, extra1__) \ +static void \ +call__ ## _start_internal(void *d) \ +{ \ + struct call__ ## _params *parms; \ + struct mysql_async_context *b; \ + \ + parms= (struct call__ ## _params *)d; \ + b= (parms_mysql_val__)->extension->async_context; \ + \ + call__ invoke_args__; \ + b->ret_status= 0; \ +} \ +int STDCALL \ +call__ ## _start decl_args__ \ +{ \ + int res; \ + struct mysql_async_context *b; \ + struct call__ ## _params parms; \ + \ + extra1__ \ + if (!(b= mysql_get_async_context((mysql_val__)))) \ + { \ + return 0; \ + } \ You can remove test if we know that b always exists. At least you can remove '{' + parms_assign__ \ + \ + b->active= 1; \ + res= my_context_spawn(&b->async_context, call__ ## _start_internal, &parms);\ + b->active= 0; \ + if (res < 0) \ + { \ + set_mysql_error((mysql_val__), CR_OUT_OF_MEMORY, unknown_sqlstate); \ + b->suspended= 0; \ + return 0; \ + } \ + else if (res > 0) \ + { \ + /* Suspended. */ \ + b->suspended= 1; \ + return b->ret_status; \ + } \ + else \ + { \ + /* Finished. */ \ + b->suspended= 0; \ + return 0; \ + } \ +} \ See previous comment of how to remove 4 code lines. +int STDCALL \ +call__ ## _cont(cont_arg__, int ready_status) \ +{ \ + int res; \ + struct mysql_async_context *b; \ + \ + b= (mysql_val__)->extension->async_context; \ + if (!b || !b->suspended) \ Assume !b exists + { \ + set_mysql_error((mysql_val__), CR_COMMANDS_OUT_OF_SYNC, unknown_sqlstate);\ + return 0; \ + } \ + \ + b->active= 1; \ + b->ret_status= ready_status; \ + res= my_context_continue(&b->async_context); \ + b->active= 0; \ + if (res < 0) \ + { \ + set_mysql_error((mysql_val__), CR_OUT_OF_MEMORY, unknown_sqlstate); \ + b->suspended= 0; \ + return 0; \ + } \ + else if (res > 0) \ + { \ + /* Suspended. */ \ + return b->ret_status; \ + } \ + else \ + { \ + /* Finished. */ \ + b->suspended= 0; \ + return 0; \ + } \ +} Remove extra lines above. Add comment here (and for other similar structs): /* This struct is used by the next MK_ASYNC_CALL */ + +struct mysql_real_connect_params { + MYSQL *mysql; + const char *host; + const char *user; + const char *passwd; + const char *db; + unsigned int port; + const char *unix_socket; + unsigned long client_flags; +}; +MK_ASYNC_CALLS( + mysql_real_connect, + (MYSQL **ret, MYSQL *mysql, const char *host, const char *user, + const char *passwd, const char *db, unsigned int port, + const char *unix_socket, unsigned long client_flags), + (parms->mysql, parms->host, parms->user, parms->passwd, parms->db, + parms->port, parms->unix_socket, parms->client_flags), <cut> +mysql_close_start(MYSQL *sock) +{ + int res; + + /* It is legitimate to have NULL sock argument, which will do nothing. */ + if (sock) + { + res= mysql_pre_close_start(sock); + /* If we need to block, return now and do the rest in mysql_close_cont(). */ + if (res) + return res; + } + mysql_close(sock); + return 0; +} +int STDCALL +mysql_close_cont(MYSQL *sock, int ready_status) +{ + int res; + + res= mysql_pre_close_cont(sock, ready_status); + if (res) + return res; + mysql_close(sock); + return 0; +} You should use the above mysql_close functions also in client/async_example.cc +#ifdef USE_OLD_FUNCTIONS Forget these! We should remove them soon anyway... <cut> === added file 'tests/async_queries.c' +void +add_query(const char *q) +{ + struct query_entry *e; + char *q2; + size_t len; + + e= malloc(sizeof(*e)); + q2= strdup(q); Would be better if you used my_malloc / my_strdup here. (To get things safemalloc checked). + if (!e || !q2) + fatal(NULL, "Out of memory"); + + /* Remove any trailing newline. */ + len= strlen(q2); + if (q2[len] == '\n') + q2[len--]= '\0'; + if (q2[len] == '\r') + q2[len--]= '\0'; + + e->next= NULL; + e->query= q2; + e->index= query_counter++; + *tail_ptr= e; + tail_ptr= &e->next; +} <cut> === modified file 'vio/viosocket.c' --- vio/viosocket.c 2011-05-14 16:42:07 +0000 +++ vio/viosocket.c 2011-09-20 10:49:25 +0000 @@ -21,6 +21,7 @@ */ #include "vio_priv.h" +#include "my_context.h" int vio_errno(Vio *vio __attribute__((unused))) { @@ -31,18 +32,34 @@ size_t vio_read(Vio * vio, uchar* buf, size_t size) { size_t r; + extern ssize_t my_recv_async(struct mysql_async_context *b, int fd, + unsigned char *buf, size_t size, uint timeout); Move to include file. DBUG_ENTER("vio_read"); DBUG_PRINT("enter", ("sd: %d buf: 0x%lx size: %u", vio->sd, (long) buf, (uint) size)); /* Ensure nobody uses vio_read_buff and vio_read simultaneously */ DBUG_ASSERT(vio->read_end == vio->read_pos); + if (vio->async_context && vio->async_context->active) + r= my_recv_async(vio->async_context, vio->sd, buf, size, vio->read_timeout); + else + { <cut> === modified file 'vio/viossl.c' @@ -21,6 +21,7 @@ */ #include "vio_priv.h" +#include "my_context.h" #ifdef HAVE_OPENSSL @@ -90,11 +91,16 @@ size_t vio_ssl_read(Vio *vio, uchar* buf, size_t size) { size_t r; + extern int my_ssl_read_async(struct mysql_async_context *b, SSL *ssl, + void *buf, int size); Move to extern file (Same for other extern declarations in this file). <cut> Good and intresting work! Regards, Monty

4 9

[Maria-developers] IO statistic in slow log
by jiangchengyao 09 Jan '12

09 Jan '12

Hi, all I write a patch to show physical & logical IO statistics in slow log. Now it only support XtraDB storage engine, but it is easy to realize in other engine. You may found it like this in slow log: # Time: 111227 16:29:54 # User@Host: root[root] @ localhost [::1] # Query_time: 0.310018 Lock_time: 0.203012 Rows_sent: 1 Rows_examined: 30000 Logical_IO: 30145 Physical_IO: 50 use tpcc; SET timestamp=1324974594; select count(1) from history; I think this feature is much helpful for DBA or Developer to know what the real IO one SQL has. In addition, it is simple, only a few changes should be made. The branch is: https://code.launchpad.net/~jiangchengyao/+junk/mariadb Regards, David 2011-12-30 jiangchengyao

3 2

[Maria-developers] win auth plugin
by sergii＠pisem.net 31 Dec '11

31 Dec '11

Hi. Attached is the patch for the windows authentication plugin. To avoid code duplication I've reused MySQL's windows authentication plugin and only wrote the server part. This plugin is different from MySQL's, because (as all our plugins) it does not do user name mapping and only preforms the authentication - that is, only verifies that the user really is who he says he is. Regards, Sergei

3 3

[Maria-developers] WL#247 New (by Igor): Make the optimizer use extended keys
by worklog-noreply＠askmonty.org 31 Dec '11

31 Dec '11

----------------------------------------------------------------------- WORKLOG TASK -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- TASK...........: Make the optimizer use extended keys CREATION DATE..: Sat, 31 Dec 2011, 05:49 SUPERVISOR.....: IMPLEMENTOR....: Igor COPIES TO......: Igor Monty Psergey Sanja Sergei Timour CATEGORY.......: Server-BackLog TASK ID........: 247 (http://askmonty.org/worklog/?tid=247) VERSION........: Server-5.5 STATUS.........: Assigned PRIORITY.......: 60 WORKED HOURS...: 0 ESTIMATE.......: 60 (hours remain) ORIG. ESTIMATE.: 60 PROGRESS NOTES: DESCRIPTION: Currently the optimizer mostly does not use hidden components of InnoDB keys. Meanwhile these components contain fields of the primary key and could be used to to generate more efficient execution plans. In particular using these components in many cases allow to generate execution plans that that employs index only look-ups. This task will support usage of extended keys for: - ref and eq-ref accesses - range scans - index-merge scans - loose scans - min/max optimization ESTIMATED WORK TIME ESTIMATED COMPLETION DATE ----------------------------------------------------------------------- WorkLog (v4.0.0)

1 0

[Maria-developers] Problems with packages in 5.3 buildbot
by Kristian Nielsen 30 Dec '11

30 Dec '11

I learned that there were many test failures with packages in 5.3. I think it may be due to the version in configure.in not being increased after last release. I pushed a commit that changes it to 5.3.4-rc, we will see if it helps. - Kristian.

1 0

Re: [Maria-developers] License issues
by Henrik Ingo 30 Dec '11

30 Dec '11

I'm assuming you left out the list in accident and won't mind that I quote you a little: On Thu, Dec 29, 2011 at 10:22 PM, Sergei Golubchik <serg(a)askmonty.org> wrote: > On Dec 29, Henrik Ingo wrote: >> (I'm pretty sure readline developers didn't quite realize what they >> were doing here. Their original goal with their choice of license was >> to create an advantage for GPL licensed apps, now they are doing >> exactly the opposite.) > > I think it was intentional. > > readline is a gnu project, hosted on gnu.org. > It looks like an FSF policy to use GPLv3. Other gnu software than I've > checked is also on GPLv3 (in particular, libbfd - that is used by > binutils, gdb, etc - so it's kind of fundamental for development chain). Yes, I'm aware of this. However, most FSF *libraries* are licensed LGPL. Only readline and a few others (apparently then libbfd, if you say so) are GPL. The purpose of this is to make those libraries exclusively available to GPL software, or also compatible FOSS software. (eg. a BSD licensed application could use a GPL licensed library as long as source code of the app is in fact made available.) > I might be wrong, but I think using GPLv3 for all (or most) GNU > projects, including essencial libraries, is quite intentional. Well, it is intentional for FSF in the sense that it is policy. For the libraries that use LGPL it is also quite fine to force everyone to use them under LGPLv3, as it won't have similar bad effects on the apps that use those libraries. However, in the case of readline and other libraries that are GPL, this has the effect of hurting projects that are GPLv2 only, while it in fact improves the situation for non-copyleft projects such as those under Apache license. (Apache license is considered by FSF to be incompatible with GPLv2 but compatible with GPLv3.) I very much doubt that this particular consequence was intentional. To support GPL licensed software, the FSF should in this case have licensed readline as both GPLv2 and GPLv3. henrik -- henrik.ingo(a)avoinelama.fi +358-40-8211286 skype: henrik.ingo irc: hingo www.openlife.cc My LinkedIn profile: http://www.linkedin.com/profile/view?id=9522559

2 1

[Maria-developers] License issues
by Maarten Vanraes 30 Dec '11

30 Dec '11

Hi, I'm a packager for mageia, and i'm working to get branch 5.5 submitted on our system, replacing mysql. However, we have readline v6 on mageia, which is GPLv3, and serg added the NOT_FOR_DISTRIBUTION variable, which makes it build a bundled readline v5. well, it builds for us too, but later i get a conflict situation on our system between mariadb-common and mariadb-client (i didn't look deep into it). but normally we try to use as much system shared libraries as possible, also, what about security fixes + bug fixes in this bundled readline v5, etc... i donno how long v5 will be maintained by gnu, this is not an ideal situation. i've checked and noted that mariadb server is GPLv2only and mariadb client has a FLOSS exception... which doesn't mention GPLv3 so i'm a bit at a loss here, ideally we want to use system shared readline v6 on our distribution... it's a bit of a PITA for me, tbh, i mean, it's all open source, i don't really get why this is all so difficult... :-( I guess the easiest way for us is if mariadb was GPLv2+ we could go ahead and use readline v6. I guess i'm asking for a way to proceed for us. Regards, AL13N

4 6

Re: [Maria-developers] [Commits] Rev 3077: Fixed LP bug #904345. in file:///home/igor/maria/maria-5.2-bug904345/
by Oleksandr Byelkin 27 Dec '11

27 Dec '11

Hi! 27.12.2011 20:46, Igor Babaev пишет: > At file:///home/igor/maria/maria-5.2-bug904345/ > > ------------------------------------------------------------ > revno: 3077 > revision-id: igor(a)askmonty.org-20111227184559-v7hlo1h98dz8ui32 > parent: wlad(a)montyprogram.com-20111223140257-k34gmwhs5msbc57k > committer: Igor Babaev<igor(a)askmonty.org> > branch nick: maria-5.2-bug904345 > timestamp: Tue 2011-12-27 10:45:59 -0800 > message: > Fixed LP bug #904345. > > The MIN/MAX optimizer code from the function opt_sum_query erroneously > did not take into account conjunctive conditions that did not depend on > any table, yet were not identified as constant items. These could be > items containing rand() or PS/SP parameters. These items are supposed > to be evaluated at the execution phase. That's why if such conditions > can be extracted from the WHERE condition the MIN/MAX optimization is > not applied as currently it is always done at the optimization phase. > > (In 5.3 expensive subqueries are also evaluated only at the execution > phase. So, if a constant condition with such subquery can be extracted > from the WHERE clause the MIN/MAX optimization should not be applied > in 5.3.) > > IF an IN/ALL/SOME predicate with a constant left part is transformed > into an EXISTS subquery the resulting subquery should not be considered > uncacheable if the right part of the predicate is not uncacheable. > > Backported the function dbug_print_item() from 5.3. The function is used > only for debugging. [skip] > === modified file 'sql/item.cc' > --- a/sql/item.cc 2011-11-24 16:48:58 +0000 > +++ b/sql/item.cc 2011-12-27 18:45:59 +0000 > @@ -7915,6 +7915,22 @@ > ((TABLE_LIST *)data)->hide_view_error(thd); > } > > +/* Debugger help function */ > +static char dbug_item_print_buf[256]; > + > +const char *dbug_print_item(Item *item) > +{ > + char *buf= dbug_item_print_buf; > + String str(buf, sizeof(dbug_item_print_buf),&my_charset_bin); > + str.length(0); > + if (!item) > + return "(Item*)NULL"; > + item->print(&str ,QT_ORDINARY); > + if (str.c_ptr() == buf) > + return buf; > + else > + return "Couldn't fit into buffer"; > +} This should be put under #ifndef DBUG_OFF Everything else looks OK to be pushed. [skip]

1 0

Re: [Maria-developers] [Commits] Rev 3170: After-merge fixes for 5.5 merge. in http://bazaar.launchpad.net/~maria-captains/maria/5.5
by Kristian Nielsen 27 Dec '11

27 Dec '11

Hi Monty, While fixing test failures in the MariaDB 5.5 tree, I encountered problems in sql_string, related to changes you did there in 5.2. I managed to come up with some solution, but please see below patch and see if you can suggest something better. Two issues: 1. You added realloc_with_extra() / realloc_with_extra_if_needed(), and used them to replace calls to realloc(). But realloc() as part of its operation puts a terminating '\0' at the end of the allocation; with your change this is now at a different (and not very useful) position in the buffer. My change tries to put it in the original place, but it is not terribly elegant, and perhaps your original change needs to be rethought. 2. I found a terrible mess (IMHO): We have two distict, subtly differing, sql_string.{h,cc}, one in /sql/ and one in /client/. We even seem to have a requirement for ABI compatibility between the two, since mysql_embedded uses client/sql_string.h with sql/sql_string.cc :-( And in fact your change seems to break such ABI compatibility, so I suspect mysql_embedded might be subtly broken in 5.2. But why do we need to have two sql_string in the first place? Especially given that they are apparently sufficiently close to be ABI compatible ... can't we eliminate on of them to get at least a little sanity into the sql_string story? - Kristian. knielsen(a)knielsen-hq.org writes: > At http://bazaar.launchpad.net/~maria-captains/maria/5.5 > > ------------------------------------------------------------ > revno: 3170 > revision-id: knielsen(a)knielsen-hq.org-20111209144027-yf5as4v9lb6jt3s7 > parent: knielsen(a)knielsen-hq.org-20111208170848-pijq4i8ceqwcl19q > committer: knielsen(a)knielsen-hq.org > branch nick: mariadb-5.5 > timestamp: Fri 2011-12-09 15:40:27 +0100 > message: > After-merge fixes for 5.5 merge. > > Fix typo causing too low timeout value for wait_for_slave_param.inc. > Fix wrong/missing '\0' termination in String:realloc() after Monty's > optimisations to reduce reallocation calls. > === modified file 'mysql-test/include/wait_for_slave_param.inc' > --- a/mysql-test/include/wait_for_slave_param.inc 2010-12-20 14:15:01 +0000 > +++ b/mysql-test/include/wait_for_slave_param.inc 2011-12-09 14:40:27 +0000 > @@ -79,7 +79,7 @@ if ($_slave_check_configured == 'No such > > # mysqltest doesn't provide any better way to multiply by 10 > --let $_wait_for_slave_param_zero= 0 > ---let $_slave_timeout_counter= $_slave_timeout$zero > +--let $_slave_timeout_counter= $_slave_timeout$_wait_for_slave_param_zero > --let $_slave_continue= 1 > while ($_slave_continue) > { > > === modified file 'sql/rpl_handler.cc' > --- a/sql/rpl_handler.cc 2011-11-03 18:17:05 +0000 > +++ b/sql/rpl_handler.cc 2011-12-09 14:40:27 +0000 > @@ -397,7 +397,7 @@ int Binlog_transmit_delegate::before_sen > > int ret= 0; > FOREACH_OBSERVER(ret, before_send_event, thd, > - (&param, (uchar *)packet->c_ptr(), > + (&param, (uchar *)packet->c_ptr_safe(), > packet->length(), > log_file+dirname_length(log_file), log_pos)); > return ret; > @@ -411,7 +411,7 @@ int Binlog_transmit_delegate::after_send > > int ret= 0; > FOREACH_OBSERVER(ret, after_send_event, thd, > - (&param, packet->c_ptr(), packet->length())); > + (&param, packet->c_ptr_safe(), packet->length())); > return ret; > } > > > === modified file 'sql/sql_string.cc' > --- a/sql/sql_string.cc 2011-11-03 18:17:05 +0000 > +++ b/sql/sql_string.cc 2011-12-09 14:40:27 +0000 > @@ -80,8 +80,12 @@ bool String::real_alloc(uint32 length) > no allocation occured. > > @retval true An error occured when attempting to allocate memory. > + > + The _internal() version is just to share code between realloc() and > + realloc_with_extra(), so that we can put the NULL termination at the right > + position. > */ > -bool String::realloc(uint32 alloc_length) > +bool String::realloc_internal(uint32 alloc_length) > { > if (Alloced_length <= alloc_length) > { > @@ -109,6 +113,13 @@ bool String::realloc(uint32 alloc_length > Ptr= new_ptr; > Alloced_length= len; > } > + return FALSE; > +} > + > +bool String::realloc(uint32 alloc_length) > +{ > + if (realloc_internal(alloc_length)) > + return TRUE; > Ptr[alloc_length]=0; // This make other funcs shorter > return FALSE; > } > > === modified file 'sql/sql_string.h' > --- a/sql/sql_string.h 2011-11-03 18:17:05 +0000 > +++ b/sql/sql_string.h 2011-12-09 14:40:27 +0000 > @@ -255,12 +255,18 @@ public: > return real_alloc(arg_length); > } > bool real_alloc(uint32 arg_length); // Empties old string > +private: > + bool realloc_internal(uint32 arg_length); > +public: > bool realloc(uint32 arg_length); > bool realloc_with_extra(uint32 arg_length) > { > if (extra_alloc < 4096) > extra_alloc= extra_alloc*2+128; > - return realloc(arg_length + extra_alloc); > + if (realloc_internal(arg_length + extra_alloc)) > + return TRUE; > + Ptr[arg_length]= 0; > + return FALSE; > } > bool realloc_with_extra_if_needed(uint32 arg_length) > { > > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits

3 2

[Maria-developers] Thread pool (request for review)
by Vladislav Vaintroub 27 Dec '11

27 Dec '11

Hello, Worklog#246 http://askmonty.org/worklog/Server-BackLog/?tid=246 is ready to review. The worklog is implemented in lp:~maria-captains/maria/5.5-threadpool, and if someone prefers to have a one big diff, it is in the attachment(diff.diff) It would be great if Monty, Serg (optionally Kristian if there is interest) could take a some time to review it in the near future. The worklog has extensive description of the implementation, might be helpful to consult it before reviewing. The branch currently has thread-handling=pool-if-threads as default, I found it easier for testing. Based on results of benchmark (see below), it seems to be generally performing well. So maybe we should use it as default, beta version of 5.5 for better testing coverage . Maybe we can keep it as new default after testing default, if there are no complains. Benchmarking: I also run some sysbench on 3 machines (2 Linuxes - fb-maria1 with 16 cores and pitbull with 24 cores, and windows one with 8 cores but good IO), results in https://docs.google.com/spreadsheet/ccc?key=0AqvHLyfRWjmbdDY0U1MzZGlXY3VGdUx xcC1pUlBZeHc I used sysbench 0.4 to run some of "unit" oltp tests (point-select and update-nokey), as well as mere "classic" OLTP-readonly and OLTP-readwrite. All tests are run with number of concurrent clients ranging from 16 to 4096, with warm cache, with sysbench table having 1Mio records. The results seem to be quite different on all of the machines tested (the machines are very different, in terms of cores, IO etc) , yet threadpool has a positive effect on all 3 machines, and the positive effect seems to grow with the number of cores. Some words about how I ran the benchmarks 1. Benchmark client and server used different CPUs - (I used 'taskset' (Linux), or 'start /affinity' (Windows) to run benchmark client on #CPUs/4, the rest of CPUs are used by the server). On Linuxes, I used --thread_pool_size=<N> where N is number of cores I dedicate to server. 2.I used innodb_flush_log_at_trx_commit=0 and innodb_flush_method= ALL_O_DIRECT to avoid extensive fsyncing, which I think is ok for the purposes of the testing for this WL. 3. Every "write" benchmark (oltp_rw and update_nokey) started with new server (i.e I kill mysqld, remove innodb files, and restart mysqld for each test),whereas every "read" benchmark reused the same running server instance. Starting afresh with a new server on write benchmarks is done mailnly to eliminate the effects of the purge lag. I attach config file sysbench.ini is the one I used on fb-maria1 In case I missed to say it before, Happy Xmas and Happy New Year! Wlad PS. Elena, we need to have some plan on testing it. PPS. Kristian, tried to integrate the 5.5-threadpool branch into Buildbot, and stuck. Can you help?

2 2