Commit 1ee2d817 authored by Hakan Kuecuekyilmaz's avatar Hakan Kuecuekyilmaz

Fix for

    mysqlslap: setting --engine does not get replicated
    http://bugs.mysql.com/bug.php?id=46967

and
    mysqlslap: specifying --engine and --create does not
    work with --engine=<storage_engine>:<option>
    https://bugs.launchpad.net/maria/+bug/429773

Problems were that an --engine=<storage_engine> was translated
to a "set storage_engine = <storage_engine>", wich is _not_
replicated. A --engine=<storage_engine>:<option> was not always
properly parsed and in some cases crashed.

Fixed by eliminating "set storage_engine = ..." and adding
proper DDL generation. Initialized an unitialized buffer to
prevent crashes and fixed to use proper pointer for in
case of --engine=<storage_engine>:<option> being the last
element in list of --engines.

Also cleaned up code for better readability.

Q: Should MySQL's replication actually replicate a
   "set storage_engine = ..." command or not?
A: No it should not. It is documented here:
   http://dev.mysql.com/doc/refman/5.1/en/replication-features-variables.html
   ...
   "The storage_engine system variable is not replicated, which is a
   good thing for replication between different storage engines." ...

Before the patch, mysqlslap was behaving this way:

+-------------------------------+--------+-------------+
|                               | single | replication |
+-------------------------------+--------+-------------+
| Before patch                                         | 
+-------------------------------+--------+-------------+
| --engine[1]                                          |
+-----+-------------------------+--------+-------------+
| 1.1 | eng1                    |  OK    |   Not OK    |
| 1.2 | eng1,eng2               |  OK    |   Not OK    |
| 1.3 | eng1,eng2,eng3          |  OK    |   Not OK    |
| 1.4 | memory:option           |  OK    |   Not OK    |
| 1.5 | memory:option,eng1      |  OK    |   Not OK    |
| 1.6 | eng1,memory:option      | Not OK |   Not OK    |
| 1.7 | memory:option,eng1,eng2 | Crash  |   Not OK    |
| 1.8 | eng1,memory:option,eng2 |  OK    |   Not OK    |
| 1.9 | eng1,eng2,memory:option | Not OK |   Not OK    |
+-----+-------------------------+--------+-------------+
+-------------------------------+--------+-------------+
| --create --engine[2]                                 |
+-----+-------------------------+--------+-------------+
| 2.1 | eng1                    |  OK    |   Not OK    |
| 2.2 | eng1,eng2               |  OK    |   Not OK    |
| 2.3 | eng1,eng2,eng3          |  OK    |   Not OK    |
| 2.4 | memory:option           | Not OK |   Not OK    |
| 2.5 | memory:option,eng1      | Not OK |   Not OK    |
| 2.6 | eng1,memory:option      | Not OK |   Not OK    |
| 2.7 | memory:option,eng1,eng2 | Crash  |   Not OK    |
| 2.8 | eng1,memory:option,eng2 | Not OK |   Not OK    |
| 2.9 | eng1,eng2,memory:option | Not OK |   Not OK    |
+-----+-------------------------+--------+-------------+

After my final patch, mysqlslap now runs like this:

+-------------------------------+--------+-------------+
|                               | single | replication |
+-------------------------------+--------+-------------+
| After third patch                                    | 
+-------------------------------+--------+-------------+
| --engine[1]                                          |
+-----+-------------------------+--------+-------------+
| 1.1 | eng1                    |  OK    |  OK         |
| 1.2 | eng1,eng2               |  OK    |  OK         |
| 1.3 | eng1,eng2,eng3          |  OK    |  OK         |
| 1.4 | memory:option           |  OK    |  OK         |
| 1.5 | memory:option,eng1      |  OK    |  OK         |
| 1.6 | eng1,memory:option      |  OK    |  OK         |
| 1.7 | memory:option,eng1,eng2 |  OK    |  OK         |
| 1.8 | eng1,memory:option,eng2 |  OK    |  OK         |
| 1.9 | eng1,eng2,memory:option |  OK    |  OK         |
+-----+-------------------------+--------+-------------+
+-------------------------------+--------+-------------+
| --create --engine[2]                                 |
+-----+-------------------------+--------+-------------+
| 2.1 | eng1                    |  OK    |  OK         |
| 2.2 | eng1,eng2               |  OK    |  OK         |
| 2.3 | eng1,eng2,eng3          |  OK    |  OK         |
| 2.4 | memory:option           |  OK    |  OK         |
| 2.5 | memory:option,eng1      |  OK    |  OK         |
| 2.6 | eng1,memory:option      |  OK    |  OK         |
| 2.7 | memory:option,eng1,eng2 |  OK    |  OK         |
| 2.8 | eng1,memory:option,eng2 |  OK    |  OK         |
| 2.9 | eng1,eng2,memory:option |  OK    |  OK         |
+-----+-------------------------+--------+-------------+
parent d50e99c9
...@@ -448,7 +448,16 @@ void concurrency_loop(MYSQL *mysql, uint current, option_string *eptr) ...@@ -448,7 +448,16 @@ void concurrency_loop(MYSQL *mysql, uint current, option_string *eptr)
/* First we create */ /* First we create */
if (create_statements) if (create_statements)
{
/*
If we have an --engine option, then we indicate
create_schema() to add the engine type to the DDL.
*/
if (eptr)
create_statements->type= CREATE_TABLE_TYPE;
create_schema(mysql, create_schema_string, create_statements, eptr); create_schema(mysql, create_schema_string, create_statements, eptr);
}
/* /*
If we generated GUID we need to build a list of them from creation that If we generated GUID we need to build a list of them from creation that
...@@ -588,7 +597,9 @@ static struct my_option my_long_options[] = ...@@ -588,7 +597,9 @@ static struct my_option my_long_options[] =
"Detach (close and reopen) connections after X number of requests.", "Detach (close and reopen) connections after X number of requests.",
(uchar**) &detach_rate, (uchar**) &detach_rate, 0, GET_UINT, REQUIRED_ARG, (uchar**) &detach_rate, (uchar**) &detach_rate, 0, GET_UINT, REQUIRED_ARG,
0, 0, 0, 0, 0, 0}, 0, 0, 0, 0, 0, 0},
{"engine", 'e', "Storage engine to use for creating the table.", {"engine", 'e', "Comma separated list of storage engines to use for creating the table."
"The test is run for each engine. You can also specify an option for an engine"
"after a `:', like memory:max_row=2300",
(uchar**) &default_engine, (uchar**) &default_engine, 0, (uchar**) &default_engine, (uchar**) &default_engine, 0,
GET_STR, REQUIRED_ARG, 0, 0, 0, 0, 0, 0}, GET_STR, REQUIRED_ARG, 0, 0, 0, 0, 0, 0},
{"host", 'h', "Connect to host.", (uchar**) &host, (uchar**) &host, 0, GET_STR, {"host", 'h', "Connect to host.", (uchar**) &host, (uchar**) &host, 0, GET_STR,
...@@ -958,6 +969,7 @@ build_update_string(void) ...@@ -958,6 +969,7 @@ build_update_string(void)
ptr->type= UPDATE_TYPE_REQUIRES_PREFIX ; ptr->type= UPDATE_TYPE_REQUIRES_PREFIX ;
else else
ptr->type= UPDATE_TYPE; ptr->type= UPDATE_TYPE;
strmov(ptr->string, update_string.str); strmov(ptr->string, update_string.str);
dynstr_free(&update_string); dynstr_free(&update_string);
DBUG_RETURN(ptr); DBUG_RETURN(ptr);
...@@ -973,8 +985,8 @@ build_update_string(void) ...@@ -973,8 +985,8 @@ build_update_string(void)
static statement * static statement *
build_insert_string(void) build_insert_string(void)
{ {
char buf[HUGE_STRING_LENGTH]; char buf[HUGE_STRING_LENGTH];
unsigned int col_count; unsigned int col_count;
statement *ptr; statement *ptr;
DYNAMIC_STRING insert_string; DYNAMIC_STRING insert_string;
DBUG_ENTER("build_insert_string"); DBUG_ENTER("build_insert_string");
...@@ -1064,8 +1076,8 @@ build_insert_string(void) ...@@ -1064,8 +1076,8 @@ build_insert_string(void)
static statement * static statement *
build_select_string(my_bool key) build_select_string(my_bool key)
{ {
char buf[HUGE_STRING_LENGTH]; char buf[HUGE_STRING_LENGTH];
unsigned int col_count; unsigned int col_count;
statement *ptr; statement *ptr;
static DYNAMIC_STRING query_string; static DYNAMIC_STRING query_string;
DBUG_ENTER("build_select_string"); DBUG_ENTER("build_select_string");
...@@ -1117,6 +1129,7 @@ build_select_string(my_bool key) ...@@ -1117,6 +1129,7 @@ build_select_string(my_bool key)
ptr->type= SELECT_TYPE_REQUIRES_PREFIX; ptr->type= SELECT_TYPE_REQUIRES_PREFIX;
else else
ptr->type= SELECT_TYPE; ptr->type= SELECT_TYPE;
strmov(ptr->string, query_string.str); strmov(ptr->string, query_string.str);
dynstr_free(&query_string); dynstr_free(&query_string);
DBUG_RETURN(ptr); DBUG_RETURN(ptr);
...@@ -1175,8 +1188,6 @@ get_options(int *argc,char ***argv) ...@@ -1175,8 +1188,6 @@ get_options(int *argc,char ***argv)
exit(1); exit(1);
} }
if (auto_generate_sql && num_of_query && auto_actual_queries) if (auto_generate_sql && num_of_query && auto_actual_queries)
{ {
fprintf(stderr, fprintf(stderr,
...@@ -1217,6 +1228,7 @@ get_options(int *argc,char ***argv) ...@@ -1217,6 +1228,7 @@ get_options(int *argc,char ***argv)
num_int_cols= atoi(str->string); num_int_cols= atoi(str->string);
if (str->option) if (str->option)
num_int_cols_index= atoi(str->option); num_int_cols_index= atoi(str->option);
option_cleanup(str); option_cleanup(str);
} }
...@@ -1229,6 +1241,7 @@ get_options(int *argc,char ***argv) ...@@ -1229,6 +1241,7 @@ get_options(int *argc,char ***argv)
num_char_cols_index= atoi(str->option); num_char_cols_index= atoi(str->option);
else else
num_char_cols_index= 0; num_char_cols_index= 0;
option_cleanup(str); option_cleanup(str);
} }
...@@ -1463,6 +1476,7 @@ get_options(int *argc,char ***argv) ...@@ -1463,6 +1476,7 @@ get_options(int *argc,char ***argv)
if (tty_password) if (tty_password)
opt_password= get_tty_password(NullS); opt_password= get_tty_password(NullS);
DBUG_RETURN(0); DBUG_RETURN(0);
} }
...@@ -1477,6 +1491,7 @@ static int run_query(MYSQL *mysql, const char *query, int len) ...@@ -1477,6 +1491,7 @@ static int run_query(MYSQL *mysql, const char *query, int len)
if (verbose >= 3) if (verbose >= 3)
printf("%.*s;\n", len, query); printf("%.*s;\n", len, query);
return mysql_real_query(mysql, query, len); return mysql_real_query(mysql, query, len);
} }
...@@ -1592,18 +1607,6 @@ create_schema(MYSQL *mysql, const char *db, statement *stmt, ...@@ -1592,18 +1607,6 @@ create_schema(MYSQL *mysql, const char *db, statement *stmt,
} }
} }
if (engine_stmt)
{
len= snprintf(query, HUGE_STRING_LENGTH, "set storage_engine=`%s`",
engine_stmt->string);
if (run_query(mysql, query, len))
{
fprintf(stderr,"%s: Cannot set default engine: %s\n", my_progname,
mysql_error(mysql));
exit(1);
}
}
count= 0; count= 0;
after_create= stmt; after_create= stmt;
...@@ -1617,8 +1620,21 @@ create_schema(MYSQL *mysql, const char *db, statement *stmt, ...@@ -1617,8 +1620,21 @@ create_schema(MYSQL *mysql, const char *db, statement *stmt,
{ {
char buffer[HUGE_STRING_LENGTH]; char buffer[HUGE_STRING_LENGTH];
snprintf(buffer, HUGE_STRING_LENGTH, "%s %s", ptr->string, snprintf(buffer, HUGE_STRING_LENGTH, "%s Engine = %s %s",
engine_stmt->option); ptr->string, engine_stmt->string, engine_stmt->option);
if (run_query(mysql, buffer, strlen(buffer)))
{
fprintf(stderr,"%s: Cannot run query %.*s ERROR : %s\n",
my_progname, (uint)ptr->length, ptr->string, mysql_error(mysql));
exit(1);
}
}
else if (engine_stmt && engine_stmt->string && ptr->type == CREATE_TABLE_TYPE)
{
char buffer[HUGE_STRING_LENGTH];
snprintf(buffer, HUGE_STRING_LENGTH, "%s Engine = %s",
ptr->string, engine_stmt->string);
if (run_query(mysql, buffer, strlen(buffer))) if (run_query(mysql, buffer, strlen(buffer)))
{ {
fprintf(stderr,"%s: Cannot run query %.*s ERROR : %s\n", fprintf(stderr,"%s: Cannot run query %.*s ERROR : %s\n",
...@@ -1652,6 +1668,7 @@ drop_schema(MYSQL *mysql, const char *db) ...@@ -1652,6 +1668,7 @@ drop_schema(MYSQL *mysql, const char *db)
{ {
char query[HUGE_STRING_LENGTH]; char query[HUGE_STRING_LENGTH];
int len; int len;
DBUG_ENTER("drop_schema"); DBUG_ENTER("drop_schema");
len= snprintf(query, HUGE_STRING_LENGTH, "DROP SCHEMA IF EXISTS `%s`", db); len= snprintf(query, HUGE_STRING_LENGTH, "DROP SCHEMA IF EXISTS `%s`", db);
...@@ -1940,17 +1957,27 @@ parse_option(const char *origin, option_string **stmt, char delm) ...@@ -1940,17 +1957,27 @@ parse_option(const char *origin, option_string **stmt, char delm)
uint count= 0; /* We know that there is always one */ uint count= 0; /* We know that there is always one */
for (tmp= *sptr= (option_string *)my_malloc(sizeof(option_string), for (tmp= *sptr= (option_string *)my_malloc(sizeof(option_string),
MYF(MY_ZEROFILL|MY_FAE|MY_WME)); MYF(MY_ZEROFILL|MY_FAE|MY_WME));
(retstr= strchr(ptr, delm)); (retstr= strchr(ptr, delm));
tmp->next= (option_string *)my_malloc(sizeof(option_string), tmp->next= (option_string *)my_malloc(sizeof(option_string),
MYF(MY_ZEROFILL|MY_FAE|MY_WME)), MYF(MY_ZEROFILL|MY_FAE|MY_WME)),
tmp= tmp->next) tmp= tmp->next)
{ {
char buffer[HUGE_STRING_LENGTH]; /*
Initialize buffer, because otherwise an
--engine=<storage_engine>:<option>,<eng1>,<eng2>
will crash.
*/
char buffer[HUGE_STRING_LENGTH]= "";
char *buffer_ptr; char *buffer_ptr;
count++; count++;
strncpy(buffer, ptr, (size_t)(retstr - ptr)); strncpy(buffer, ptr, (size_t)(retstr - ptr));
/*
Handle --engine=memory:max_row=200 cases, or more general speaking
--engine=<storage_engine>:<options>, which will be translated to
Engine = storage_engine option.
*/
if ((buffer_ptr= strchr(buffer, ':'))) if ((buffer_ptr= strchr(buffer, ':')))
{ {
char *option_ptr; char *option_ptr;
...@@ -1971,13 +1998,15 @@ parse_option(const char *origin, option_string **stmt, char delm) ...@@ -1971,13 +1998,15 @@ parse_option(const char *origin, option_string **stmt, char delm)
tmp->length= (size_t)(retstr - ptr); tmp->length= (size_t)(retstr - ptr);
} }
/* Skip delimiter delm */
ptr+= retstr - ptr + 1; ptr+= retstr - ptr + 1;
if (isspace(*ptr)) if (isspace(*ptr))
ptr++; ptr++;
count++; count++;
} }
if (ptr != origin+length) if (ptr != origin + length)
{ {
char *origin_ptr; char *origin_ptr;
...@@ -1986,7 +2015,7 @@ parse_option(const char *origin, option_string **stmt, char delm) ...@@ -1986,7 +2015,7 @@ parse_option(const char *origin, option_string **stmt, char delm)
char *option_ptr; char *option_ptr;
tmp->length= (size_t)(origin_ptr - ptr); tmp->length= (size_t)(origin_ptr - ptr);
tmp->string= my_strndup(origin, tmp->length, MYF(MY_FAE)); tmp->string= my_strndup(ptr, tmp->length, MYF(MY_FAE));
option_ptr= (char *)ptr + 1 + tmp->length; option_ptr= (char *)ptr + 1 + tmp->length;
...@@ -2036,7 +2065,7 @@ parse_delimiter(const char *script, statement **stmt, char delm) ...@@ -2036,7 +2065,7 @@ parse_delimiter(const char *script, statement **stmt, char delm)
if (ptr != script+length) if (ptr != script+length)
{ {
tmp->string= my_strndup(ptr, (uint)((script + length) - ptr), tmp->string= my_strndup(ptr, (uint)((script + length) - ptr),
MYF(MY_FAE)); MYF(MY_FAE));
tmp->length= (size_t)((script + length) - ptr); tmp->length= (size_t)((script + length) - ptr);
count++; count++;
} }
...@@ -2094,6 +2123,7 @@ print_conclusions_csv(conclusions *con) ...@@ -2094,6 +2123,7 @@ print_conclusions_csv(conclusions *con)
{ {
char buffer[HUGE_STRING_LENGTH]; char buffer[HUGE_STRING_LENGTH];
const char *ptr= auto_generate_sql_type ? auto_generate_sql_type : "query"; const char *ptr= auto_generate_sql_type ? auto_generate_sql_type : "query";
snprintf(buffer, HUGE_STRING_LENGTH, snprintf(buffer, HUGE_STRING_LENGTH,
"%s,%s,%ld.%03ld,%ld.%03ld,%ld.%03ld,%d,%llu\n", "%s,%s,%ld.%03ld,%ld.%03ld,%ld.%03ld,%d,%llu\n",
con->engine ? con->engine : "", /* Storage engine we ran against */ con->engine ? con->engine : "", /* Storage engine we ran against */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment