From 1d4acfb668a5de5c638a794b081fb858700f2970 Mon Sep 17 00:00:00 2001 From: "sergefp@mysql.com" <> Date: Tue, 25 Apr 2006 23:33:31 +0400 Subject: [PATCH 01/10] BUG#15872: Don't run the range analyzer on "t1.keypart NOT IN (const1, ..., )", as that consumes too much memory. Instead, either create the equvalent SEL_TREE manually, or create only two ranges that strictly include the area to scan (Note: just to re-iterate: increasing NOT_IN_IGNORE_THRESHOLD will make optimization run slower for big IN-lists, but the server will not run out of memory. O(N^2) memory use has been eliminated) --- mysql-test/r/func_in.result | 103 +++++++++++++++++++++++++++++++++++- mysql-test/t/func_in.test | 94 +++++++++++++++++++++++++++++++- sql/item.cc | 10 ++++ sql/item.h | 1 + sql/item_cmpfunc.h | 77 +++++++++++++++++++++++++-- sql/opt_range.cc | 93 ++++++++++++++++++++++++++++---- 6 files changed, 364 insertions(+), 14 deletions(-) diff --git a/mysql-test/r/func_in.result b/mysql-test/r/func_in.result index 60022ae0d8f..e3257ce5fd0 100644 --- a/mysql-test/r/func_in.result +++ b/mysql-test/r/func_in.result @@ -1,4 +1,4 @@ -drop table if exists t1; +drop table if exists t1, t2; select 1 in (1,2,3); 1 in (1,2,3) 1 @@ -225,3 +225,104 @@ a 46 DROP VIEW v1; DROP TABLE t1; +create table t1 (a int); +insert into t1 values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9); +create table t2 (a int, filler char(200), key(a)); +insert into t2 select C.a*2, 'no' from t1 A, t1 B, t1 C; +insert into t2 select C.a*2+1, 'yes' from t1 C; +explain +select * from t2 where a NOT IN (0, 2,4,6,8,10,12,14,16,18); +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t2 range a a 5 NULL 12 Using where +select * from t2 where a NOT IN (0, 2,4,6,8,10,12,14,16,18); +a filler +1 yes +3 yes +5 yes +7 yes +9 yes +11 yes +13 yes +15 yes +17 yes +19 yes +explain select * from t2 force index(a) where a NOT IN (2,2,2,2,2,2); +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t2 range a a 5 NULL 912 Using where +explain select * from t2 force index(a) where a <> 2; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t2 range a a 5 NULL 912 Using where +drop table t2; +create table t2 (a datetime, filler char(200), key(a)); +insert into t2 select '2006-04-25 10:00:00' + interval C.a minute, +'no' from t1 A, t1 B, t1 C where C.a % 2 = 0; +insert into t2 select '2006-04-25 10:00:00' + interval C.a*2+1 minute, +'yes' from t1 C; +explain +select * from t2 where a NOT IN ( +'2006-04-25 10:00:00','2006-04-25 10:02:00','2006-04-25 10:04:00', +'2006-04-25 10:06:00', '2006-04-25 10:08:00'); +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t2 range a a 9 NULL 18 Using where +select * from t2 where a NOT IN ( +'2006-04-25 10:00:00','2006-04-25 10:02:00','2006-04-25 10:04:00', +'2006-04-25 10:06:00', '2006-04-25 10:08:00'); +a filler +2006-04-25 10:01:00 yes +2006-04-25 10:03:00 yes +2006-04-25 10:05:00 yes +2006-04-25 10:07:00 yes +2006-04-25 10:09:00 yes +2006-04-25 10:11:00 yes +2006-04-25 10:13:00 yes +2006-04-25 10:15:00 yes +2006-04-25 10:17:00 yes +2006-04-25 10:19:00 yes +drop table t2; +create table t2 (a varchar(10), filler char(200), key(a)); +insert into t2 select 'foo', 'no' from t1 A, t1 B; +insert into t2 select 'barbar', 'no' from t1 A, t1 B; +insert into t2 select 'bazbazbaz', 'no' from t1 A, t1 B; +insert into t2 values ('fon', '1'), ('fop','1'), ('barbaq','1'), +('barbas','1'), ('bazbazbay', '1'),('zz','1'); +explain select * from t2 where a not in('foo','barbar', 'bazbazbaz'); +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t2 range a a 13 NULL 7 Using where +drop table t2; +create table t2 (a decimal(10,5), filler char(200), key(a)); +insert into t2 select 345.67890, 'no' from t1 A, t1 B; +insert into t2 select 43245.34, 'no' from t1 A, t1 B; +insert into t2 select 64224.56344, 'no' from t1 A, t1 B; +insert into t2 values (0, '1'), (22334.123,'1'), (33333,'1'), +(55555,'1'), (77777, '1'); +explain +select * from t2 where a not in (345.67890, 43245.34, 64224.56344); +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t2 range a a 7 NULL 7 Using where +select * from t2 where a not in (345.67890, 43245.34, 64224.56344); +a filler +0.00000 1 +22334.12300 1 +33333.00000 1 +55555.00000 1 +77777.00000 1 +drop table t2; +create table t2 (a int, key(a), b int); +insert into t2 values (1,1),(2,2); +set @cnt= 1; +set @str="update t2 set b=1 where a not in ("; +select count(*) from ( +select @str:=concat(@str, @cnt:=@cnt+1, ",") +from t1 A, t1 B, t1 C, t1 D) Z; +count(*) +10000 +set @str:=concat(@str, "10000)"); +select substr(@str, 1, 50); +substr(@str, 1, 50) +update t2 set b=1 where a not in (2,3,4,5,6,7,8,9, +prepare s from @str; +execute s; +deallocate prepare s; +set @str=NULL; +drop table t2; +drop table t1; diff --git a/mysql-test/t/func_in.test b/mysql-test/t/func_in.test index 0472968f918..351d1fc2c92 100644 --- a/mysql-test/t/func_in.test +++ b/mysql-test/t/func_in.test @@ -1,6 +1,6 @@ # Initialise --disable_warnings -drop table if exists t1; +drop table if exists t1, t2; --enable_warnings # # test of IN (NULL) @@ -128,3 +128,95 @@ SELECT * FROM v1; DROP VIEW v1; DROP TABLE t1; + +# BUG#15872: Excessive memory consumption of range analysis of NOT IN +create table t1 (a int); +insert into t1 values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9); +create table t2 (a int, filler char(200), key(a)); + +insert into t2 select C.a*2, 'no' from t1 A, t1 B, t1 C; +insert into t2 select C.a*2+1, 'yes' from t1 C; + +explain +select * from t2 where a NOT IN (0, 2,4,6,8,10,12,14,16,18); +select * from t2 where a NOT IN (0, 2,4,6,8,10,12,14,16,18); + +explain select * from t2 force index(a) where a NOT IN (2,2,2,2,2,2); +explain select * from t2 force index(a) where a <> 2; + +drop table t2; + +# +# Repeat the test for DATETIME +# +create table t2 (a datetime, filler char(200), key(a)); + +insert into t2 select '2006-04-25 10:00:00' + interval C.a minute, + 'no' from t1 A, t1 B, t1 C where C.a % 2 = 0; + +insert into t2 select '2006-04-25 10:00:00' + interval C.a*2+1 minute, + 'yes' from t1 C; + +explain +select * from t2 where a NOT IN ( + '2006-04-25 10:00:00','2006-04-25 10:02:00','2006-04-25 10:04:00', + '2006-04-25 10:06:00', '2006-04-25 10:08:00'); +select * from t2 where a NOT IN ( + '2006-04-25 10:00:00','2006-04-25 10:02:00','2006-04-25 10:04:00', + '2006-04-25 10:06:00', '2006-04-25 10:08:00'); +drop table t2; + +# +# Repeat the test for CHAR(N) +# +create table t2 (a varchar(10), filler char(200), key(a)); + +insert into t2 select 'foo', 'no' from t1 A, t1 B; +insert into t2 select 'barbar', 'no' from t1 A, t1 B; +insert into t2 select 'bazbazbaz', 'no' from t1 A, t1 B; + +insert into t2 values ('fon', '1'), ('fop','1'), ('barbaq','1'), + ('barbas','1'), ('bazbazbay', '1'),('zz','1'); + +explain select * from t2 where a not in('foo','barbar', 'bazbazbaz'); + +drop table t2; + +# +# Repeat for DECIMAL +# +create table t2 (a decimal(10,5), filler char(200), key(a)); + +insert into t2 select 345.67890, 'no' from t1 A, t1 B; +insert into t2 select 43245.34, 'no' from t1 A, t1 B; +insert into t2 select 64224.56344, 'no' from t1 A, t1 B; + +insert into t2 values (0, '1'), (22334.123,'1'), (33333,'1'), + (55555,'1'), (77777, '1'); + +explain +select * from t2 where a not in (345.67890, 43245.34, 64224.56344); +select * from t2 where a not in (345.67890, 43245.34, 64224.56344); + +drop table t2; + +# Try a very big IN-list +create table t2 (a int, key(a), b int); +insert into t2 values (1,1),(2,2); + +set @cnt= 1; +set @str="update t2 set b=1 where a not in ("; +select count(*) from ( + select @str:=concat(@str, @cnt:=@cnt+1, ",") + from t1 A, t1 B, t1 C, t1 D) Z; + +set @str:=concat(@str, "10000)"); +select substr(@str, 1, 50); +prepare s from @str; +execute s; +deallocate prepare s; +set @str=NULL; + +drop table t2; +drop table t1; + diff --git a/sql/item.cc b/sql/item.cc index 8cf6fde1dbd..9410fa65408 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -1991,6 +1991,16 @@ bool Item_decimal::eq(const Item *item, bool binary_cmp) const } +void Item_decimal::set_decimal_value(my_decimal *value_par) +{ + my_decimal2decimal(value_par, &decimal_value); + decimals= (uint8) decimal_value.frac; + unsigned_flag= !decimal_value.sign(); + max_length= my_decimal_precision_to_length(decimal_value.intg + decimals, + decimals, unsigned_flag); +} + + String *Item_float::val_str(String *str) { // following assert is redundant, because fixed=1 assigned in constructor diff --git a/sql/item.h b/sql/item.h index d33e0ae34be..8b09bc14221 100644 --- a/sql/item.h +++ b/sql/item.h @@ -1442,6 +1442,7 @@ public: } uint decimal_precision() const { return decimal_value.precision(); } bool eq(const Item *, bool binary_cmp) const; + void set_decimal_value(my_decimal *value_par); }; diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h index 67f0a5f5e2e..1837603d78f 100644 --- a/sql/item_cmpfunc.h +++ b/sql/item_cmpfunc.h @@ -623,15 +623,17 @@ public: /* Functions to handle the optimized IN */ + +/* A vector of values of some type */ + class in_vector :public Sql_alloc { - protected: +public: char *base; uint size; qsort2_cmp compare; CHARSET_INFO *collation; uint count; -public: uint used_count; in_vector() {} in_vector(uint elements,uint element_length,qsort2_cmp cmp_func, @@ -647,6 +649,32 @@ public: qsort2(base,used_count,size,compare,collation); } int find(Item *item); + + /* + Create an instance of Item_{type} (e.g. Item_decimal) constant object + which type allows it to hold an element of this vector without any + conversions. + The purpose of this function is to be able to get elements of this + vector in form of Item_xxx constants without creating Item_xxx object + for every array element you get (i.e. this implements "FlyWeight" pattern) + */ + virtual Item* create_item() { return NULL; } + + /* + Store the value at position #pos into provided item object + SYNOPSIS + value_to_item() + pos Index of value to store + item Constant item to store value into. The item must be of the same + type that create_item() returns. + */ + virtual void value_to_item(uint pos, Item *item) { } + + /* Compare values number pos1 and pos2 for equality */ + bool compare_elems(uint pos1, uint pos2) + { + return test(compare(collation, base + pos1*size, base + pos2*size)); + } }; class in_string :public in_vector @@ -658,6 +686,16 @@ public: ~in_string(); void set(uint pos,Item *item); byte *get_value(Item *item); + Item* create_item() + { + return new Item_string(collation); + } + void value_to_item(uint pos, Item *item) + { + String *str=((String*) base)+pos; + Item_string *to= (Item_string*)item; + to->str_value= *str; + } }; class in_longlong :public in_vector @@ -667,6 +705,19 @@ public: in_longlong(uint elements); void set(uint pos,Item *item); byte *get_value(Item *item); + + Item* create_item() + { + /* + We're created a signed INT, this may not be correct in + general case (see BUG#19342). + */ + return new Item_int(0); + } + void value_to_item(uint pos, Item *item) + { + ((Item_int*)item)->value= ((longlong*)base)[pos]; + } }; class in_double :public in_vector @@ -676,6 +727,15 @@ public: in_double(uint elements); void set(uint pos,Item *item); byte *get_value(Item *item); + Item *create_item() + { + return new Item_float(0.0); + } + void value_to_item(uint pos, Item *item) + { + ((Item_float*)item)->value= ((double*) base)[pos]; + } + }; class in_decimal :public in_vector @@ -685,6 +745,16 @@ public: in_decimal(uint elements); void set(uint pos, Item *item); byte *get_value(Item *item); + Item *create_item() + { + return new Item_decimal(0, FALSE); + } + void value_to_item(uint pos, Item *item) + { + my_decimal *dec= ((my_decimal *)base) + pos; + Item_decimal *item_dec= (Item_decimal*)item; + item_dec->set_decimal_value(dec); + } }; @@ -864,12 +934,13 @@ public: class Item_func_in :public Item_func_opt_neg { +public: Item_result cmp_type; in_vector *array; cmp_item *in_item; bool have_null; DTCollation cmp_collation; - public: + Item_func_in(List &list) :Item_func_opt_neg(list), array(0), in_item(0), have_null(0) { diff --git a/sql/opt_range.cc b/sql/opt_range.cc index ae02bb8934c..2610e4fc1c8 100644 --- a/sql/opt_range.cc +++ b/sql/opt_range.cc @@ -3501,17 +3501,92 @@ static SEL_TREE *get_func_mm_tree(PARAM *param, Item_func *cond_func, if (inv) { - tree= get_ne_mm_tree(param, cond_func, field, - func->arguments()[1], func->arguments()[1], - cmp_type); - if (tree) + /* + We get here for conditions like "t.keypart NOT IN (....)". + + If the IN-list contains only constants (and func->array is an ordered + array of them), we construct the appropriate SEL_ARG tree manually, + because constructing it using the range analyzer (as + AND_i( t.keypart != c_i)) will cause lots of memory to be consumed + (see BUG#15872). + */ + if (func->array && func->cmp_type != ROW_RESULT) { - Item **arg, **end; - for (arg= func->arguments()+2, end= arg+func->argument_count()-2; - arg < end ; arg++) + /* + Create one Item_type constant object. We'll need it as + get_mm_parts only accepts constant values wrapped in Item_Type + objects. + We create the Item on param->mem_root which points to + per-statement mem_root (while thd->mem_root is currently pointing + to mem_root local to range optimizer). + */ + MEM_ROOT *tmp_root= param->mem_root; + param->thd->mem_root= param->old_root; + Item *value_item= func->array->create_item(); + param->thd->mem_root= tmp_root; + + if (!value_item) + break; + + /* Get a SEL_TREE for "-inf < X < c_0" interval */ + func->array->value_to_item(0, value_item); + tree= get_mm_parts(param, cond_func, field, Item_func::LT_FUNC, + value_item, cmp_type); + if (!tree) + break; +#define NOT_IN_IGNORE_THRESHOLD 1000 + SEL_TREE *tree2; + if (func->array->count < NOT_IN_IGNORE_THRESHOLD) { - tree= tree_and(param, tree, get_ne_mm_tree(param, cond_func, field, - *arg, *arg, cmp_type)); + for (uint i=1; i < func->array->count; i++) + { + if (func->array->compare_elems(i, i-1)) + { + /* Get a SEL_TREE for "-inf < X < c_i" interval */ + func->array->value_to_item(i, value_item); + tree2= get_mm_parts(param, cond_func, field, Item_func::LT_FUNC, + value_item, cmp_type); + + /* Change all intervals to be "c_{i-1} < X < c_i" */ + for (uint idx= 0; idx < param->keys; idx++) + { + SEL_ARG *new_interval; + if ((new_interval= tree2->keys[idx])) + { + SEL_ARG *last_val= tree->keys[idx]->last(); + new_interval->min_value= last_val->max_value; + new_interval->min_flag= NEAR_MIN; + } + } + tree= tree_or(param, tree, tree2); + } + } + } + else + func->array->value_to_item(func->array->count - 1, value_item); + + /* + Get the SEL_TREE for the last "c_last < X < +inf" interval + (value_item cotains c_last already) + */ + tree2= get_mm_parts(param, cond_func, field, Item_func::GT_FUNC, + value_item, cmp_type); + tree= tree_or(param, tree, tree2); + } + else + { + tree= get_ne_mm_tree(param, cond_func, field, + func->arguments()[1], func->arguments()[1], + cmp_type); + if (tree) + { + Item **arg, **end; + for (arg= func->arguments()+2, end= arg+func->argument_count()-2; + arg < end ; arg++) + { + tree= tree_and(param, tree, get_ne_mm_tree(param, cond_func, field, + *arg, *arg, cmp_type)); + } } } } From 3144d5eb48b578525cb24378fb616a1b59bf0f27 Mon Sep 17 00:00:00 2001 From: "serg@sergbook.mysql.com" <> Date: Tue, 25 Apr 2006 13:37:33 -0700 Subject: [PATCH 02/10] buffer overflow and information exposure bugs fixed (reported by Stefano Di Paola) --- sql/sql_parse.cc | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 4b32703c1ee..93f696f6d49 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -647,6 +647,11 @@ check_connections(THD *thd) char *db=0; if (thd->client_capabilities & CLIENT_CONNECT_WITH_DB) db=strend(passwd)+1; + if (strend(db ? db : passwd) - (char*)net->read_pos > pkt_len) + { + inc_host_errors(&thd->remote.sin_addr); + return ER_HANDSHAKE_ERROR; + } if (thd->client_capabilities & CLIENT_INTERACTIVE) thd->variables.net_wait_timeout= thd->variables.net_interactive_timeout; if ((thd->client_capabilities & CLIENT_TRANSACTIONS) && @@ -1002,7 +1007,17 @@ bool dispatch_command(enum enum_server_command command, THD *thd, statistic_increment(com_other, &LOCK_status); slow_command = TRUE; uint db_len = *(uchar*)packet; + if (db_len >= packet_length || db_len > NAME_LEN) + { + send_error(&thd->net, ER_UNKNOWN_COM_ERROR); + break; + } uint tbl_len = *(uchar*)(packet + db_len + 1); + if (db_len+tbl_len+2 > packet_length || tbl_len > NAME_LEN) + { + send_error(&thd->net, ER_UNKNOWN_COM_ERROR); + break; + } char* db = thd->alloc(db_len + tbl_len + 2); memcpy(db, packet + 1, db_len); char* tbl_name = db + db_len; From 078ec307aeead7209aa0d6ff3ba3a40d489a7270 Mon Sep 17 00:00:00 2001 From: "sergefp@mysql.com" <> Date: Wed, 26 Apr 2006 01:21:33 +0400 Subject: [PATCH 03/10] Fix compile failure on Win32 --- sql/item_cmpfunc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h index 1837603d78f..1cfdcef02d0 100644 --- a/sql/item_cmpfunc.h +++ b/sql/item_cmpfunc.h @@ -712,7 +712,7 @@ public: We're created a signed INT, this may not be correct in general case (see BUG#19342). */ - return new Item_int(0); + return new Item_int((longlong)0); } void value_to_item(uint pos, Item *item) { From 54c97e61501936a8e468be0c96536558b66809c1 Mon Sep 17 00:00:00 2001 From: "serg@sergbook.mysql.com" <> Date: Tue, 25 Apr 2006 17:12:06 -0700 Subject: [PATCH 04/10] after merge fix --- sql/sql_parse.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 3fc71351d74..51ef3f31b26 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -908,7 +908,7 @@ static int check_connection(THD *thd) db + passwd_len + 1 : 0; uint db_len= db ? strlen(db) : 0; - if (passwd + passwd_len + db_len > net->read_pos + pkt_len) + if (passwd + passwd_len + db_len > (char *)net->read_pos + pkt_len) { inc_host_errors(&thd->remote.sin_addr); return ER_HANDSHAKE_ERROR; @@ -1388,13 +1388,13 @@ bool dispatch_command(enum enum_server_command command, THD *thd, uint db_len= *(uchar*) packet; if (db_len >= packet_length || db_len > NAME_LEN) { - send_error(&thd->net, ER_UNKNOWN_COM_ERROR); + send_error(thd, ER_UNKNOWN_COM_ERROR); break; } uint tbl_len= *(uchar*) (packet + db_len + 1); if (db_len+tbl_len+2 > packet_length || tbl_len > NAME_LEN) { - send_error(&thd->net, ER_UNKNOWN_COM_ERROR); + send_error(thd, ER_UNKNOWN_COM_ERROR); break; } From ed3f3fa0c7bf94715d2b05f0fec0d61f05b477a2 Mon Sep 17 00:00:00 2001 From: "serg@sergbook.mysql.com" <> Date: Tue, 25 Apr 2006 22:39:59 -0700 Subject: [PATCH 05/10] after merge --- sql/sql_parse.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 25ace6bb4b0..90f99e6bf7d 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -1598,13 +1598,13 @@ bool dispatch_command(enum enum_server_command command, THD *thd, uint db_len= *(uchar*) packet; if (db_len >= packet_length || db_len > NAME_LEN) { - send_error(thd, ER_UNKNOWN_COM_ERROR); + my_message(ER_UNKNOWN_COM_ERROR, ER(ER_UNKNOWN_COM_ERROR), MYF(0)); break; } uint tbl_len= *(uchar*) (packet + db_len + 1); if (db_len+tbl_len+2 > packet_length || tbl_len > NAME_LEN) { - send_error(thd, ER_UNKNOWN_COM_ERROR); + my_message(ER_UNKNOWN_COM_ERROR, ER(ER_UNKNOWN_COM_ERROR), MYF(0)); break; } From 92825f3ada442195bf2a1645c2f8b177ed98a5c9 Mon Sep 17 00:00:00 2001 From: "bpontz@mysql.com" <> Date: Wed, 26 Apr 2006 08:26:33 +0200 Subject: [PATCH 06/10] configure.in: clone-off for 5.0.21 --- configure.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.in b/configure.in index 174c9a16d8e..74ac7ed4574 100644 --- a/configure.in +++ b/configure.in @@ -7,7 +7,7 @@ AC_INIT(sql/mysqld.cc) AC_CANONICAL_SYSTEM # The Docs Makefile.am parses this line! # remember to also change ndb version below and update version.c in ndb -AM_INIT_AUTOMAKE(mysql, 5.0.21) +AM_INIT_AUTOMAKE(mysql, 5.0.22) AM_CONFIG_HEADER(config.h) PROTOCOL_VERSION=10 @@ -19,7 +19,7 @@ SHARED_LIB_VERSION=$SHARED_LIB_MAJOR_VERSION:0:0 # ndb version NDB_VERSION_MAJOR=5 NDB_VERSION_MINOR=0 -NDB_VERSION_BUILD=21 +NDB_VERSION_BUILD=22 NDB_VERSION_STATUS="" # Set all version vars based on $VERSION. How do we do this more elegant ? From 0e331dfb92657d61e423a747aee1f6f74eb8c298 Mon Sep 17 00:00:00 2001 From: "bell@sanja.is.com.ua" <> Date: Wed, 26 Apr 2006 17:57:41 +0300 Subject: [PATCH 07/10] fixed error message text --- mysql-test/r/view.result | 2 +- sql/share/errmsg.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/mysql-test/r/view.result b/mysql-test/r/view.result index 890e5be66c6..667d10cd145 100644 --- a/mysql-test/r/view.result +++ b/mysql-test/r/view.result @@ -2606,7 +2606,7 @@ create view v2 as select * from v1; drop table t1; rename table v2 to t1; select * from v1; -ERROR HY000: `test`.`v1` contain view recursion +ERROR HY000: `test`.`v1` contains view recursion drop view t1, v1; create table t1 (a int); create function f1() returns int diff --git a/sql/share/errmsg.txt b/sql/share/errmsg.txt index 9519e77903d..1c2e073075f 100644 --- a/sql/share/errmsg.txt +++ b/sql/share/errmsg.txt @@ -5614,4 +5614,4 @@ ER_SP_NO_AGGREGATE 42000 ER_MAX_PREPARED_STMT_COUNT_REACHED 42000 eng "Can't create more than max_prepared_stmt_count statements (current value: %lu)" ER_VIEW_RECURSIVE - eng "`%-.64s`.`%-.64s` contain view recursion" + eng "`%-.64s`.`%-.64s` contains view recursion" From 2c9d6095ea5f34bc4df667574c2a5abcb8fa2761 Mon Sep 17 00:00:00 2001 From: "tomas@poseidon.ndb.mysql.com" <> Date: Wed, 26 Apr 2006 16:57:41 +0200 Subject: [PATCH 08/10] ndb: added timeout handling to alloc node id to avoid the usage of purge stale sessions --- ndb/src/mgmsrv/MgmtSrvr.cpp | 17 ++++++++++- ndb/src/mgmsrv/MgmtSrvr.hpp | 4 ++- ndb/src/mgmsrv/Services.cpp | 57 +++++++++++++++++++++++++++++-------- ndb/src/mgmsrv/Services.hpp | 1 + 4 files changed, 65 insertions(+), 14 deletions(-) diff --git a/ndb/src/mgmsrv/MgmtSrvr.cpp b/ndb/src/mgmsrv/MgmtSrvr.cpp index 9b518ba938b..c7d0c11eec4 100644 --- a/ndb/src/mgmsrv/MgmtSrvr.cpp +++ b/ndb/src/mgmsrv/MgmtSrvr.cpp @@ -2378,6 +2378,8 @@ MgmtSrvr::repCommand(Uint32* repReqId, Uint32 request, bool waitCompleted) MgmtSrvr::Allocated_resources::Allocated_resources(MgmtSrvr &m) : m_mgmsrv(m) { + m_reserved_nodes.clear(); + m_alloc_timeout= 0; } MgmtSrvr::Allocated_resources::~Allocated_resources() @@ -2396,9 +2398,22 @@ MgmtSrvr::Allocated_resources::~Allocated_resources() } void -MgmtSrvr::Allocated_resources::reserve_node(NodeId id) +MgmtSrvr::Allocated_resources::reserve_node(NodeId id, NDB_TICKS timeout) { m_reserved_nodes.set(id); + m_alloc_timeout= NdbTick_CurrentMillisecond() + timeout; +} + +bool +MgmtSrvr::Allocated_resources::is_timed_out(NDB_TICKS tick) +{ + if (m_alloc_timeout && tick > m_alloc_timeout) + { + g_eventLogger.info("Mgmt server state: nodeid %d timed out.", + get_nodeid()); + return true; + } + return false; } NodeId diff --git a/ndb/src/mgmsrv/MgmtSrvr.hpp b/ndb/src/mgmsrv/MgmtSrvr.hpp index fe1603a1953..e2eb33d1198 100644 --- a/ndb/src/mgmsrv/MgmtSrvr.hpp +++ b/ndb/src/mgmsrv/MgmtSrvr.hpp @@ -106,7 +106,8 @@ public: ~Allocated_resources(); // methods to reserve/allocate resources which // will be freed when running destructor - void reserve_node(NodeId id); + void reserve_node(NodeId id, NDB_TICKS timeout); + bool is_timed_out(NDB_TICKS tick); bool is_reserved(NodeId nodeId) { return m_reserved_nodes.get(nodeId); } bool is_reserved(NodeBitmask mask) { return !mask.bitAND(m_reserved_nodes).isclear(); } bool isclear() { return m_reserved_nodes.isclear(); } @@ -114,6 +115,7 @@ public: private: MgmtSrvr &m_mgmsrv; NodeBitmask m_reserved_nodes; + NDB_TICKS m_alloc_timeout; }; NdbMutex *m_node_id_mutex; diff --git a/ndb/src/mgmsrv/Services.cpp b/ndb/src/mgmsrv/Services.cpp index a80827abd8f..ee97235912a 100644 --- a/ndb/src/mgmsrv/Services.cpp +++ b/ndb/src/mgmsrv/Services.cpp @@ -137,6 +137,7 @@ ParserRow commands[] = { MGM_ARG("public key", String, Mandatory, "Public key"), MGM_ARG("endian", String, Optional, "Endianness"), MGM_ARG("name", String, Optional, "Name of connection"), + MGM_ARG("timeout", Int, Optional, "Timeout in seconds"), MGM_CMD("get version", &MgmApiSession::getVersion, ""), @@ -259,6 +260,15 @@ ParserRow commands[] = { MGM_END() }; +struct PurgeStruct +{ + NodeBitmask free_nodes;/* free nodes as reported + * by ndbd in apiRegReqConf + */ + BaseString *str; + NDB_TICKS tick; +}; + MgmApiSession::MgmApiSession(class MgmtSrvr & mgm, NDB_SOCKET_TYPE sock) : SocketServer::Session(sock), m_mgmsrv(mgm) { @@ -408,6 +418,7 @@ MgmApiSession::get_nodeid(Parser_t::Context &, { const char *cmd= "get nodeid reply"; Uint32 version, nodeid= 0, nodetype= 0xff; + Uint32 timeout= 20; // default seconds timeout const char * transporter; const char * user; const char * password; @@ -425,6 +436,7 @@ MgmApiSession::get_nodeid(Parser_t::Context &, args.get("public key", &public_key); args.get("endian", &endian); args.get("name", &name); + args.get("timeout", &timeout); endian_check.l = 1; if(endian @@ -464,8 +476,24 @@ MgmApiSession::get_nodeid(Parser_t::Context &, NodeId tmp= nodeid; if(tmp == 0 || !m_allocated_resources->is_reserved(tmp)){ BaseString error_string; - if (!m_mgmsrv.alloc_node_id(&tmp, (enum ndb_mgm_node_type)nodetype, - &addr, &addrlen, error_string)){ + NDB_TICKS tick= 0; + while (!m_mgmsrv.alloc_node_id(&tmp, (enum ndb_mgm_node_type)nodetype, + &addr, &addrlen, error_string)) + { + if (tick == 0) + { + // attempt to free any timed out reservations + tick= NdbTick_CurrentMillisecond(); + struct PurgeStruct ps; + m_mgmsrv.get_connected_nodes(ps.free_nodes); + // invert connected_nodes to get free nodes + ps.free_nodes.bitXORC(NodeBitmask()); + ps.str= 0; + ps.tick= tick; + m_mgmsrv.get_socket_server()-> + foreachSession(stop_session_if_timed_out,&ps); + continue; + } const char *alias; const char *str; alias= ndb_mgm_get_node_type_alias_string((enum ndb_mgm_node_type) @@ -491,7 +519,7 @@ MgmApiSession::get_nodeid(Parser_t::Context &, m_output->println("nodeid: %u", tmp); m_output->println("result: Ok"); m_output->println(""); - m_allocated_resources->reserve_node(tmp); + m_allocated_resources->reserve_node(tmp, timeout*1000); if (name) g_eventLogger.info("Node %d: %s", tmp, name); @@ -1480,14 +1508,6 @@ done: m_output->println(""); } -struct PurgeStruct -{ - NodeBitmask free_nodes;/* free nodes as reported - * by ndbd in apiRegReqConf - */ - BaseString *str; -}; - void MgmApiSession::stop_session_if_not_connected(SocketServer::Session *_s, void *data) { @@ -1495,7 +1515,20 @@ MgmApiSession::stop_session_if_not_connected(SocketServer::Session *_s, void *da struct PurgeStruct &ps= *(struct PurgeStruct *)data; if (s->m_allocated_resources->is_reserved(ps.free_nodes)) { - ps.str->appfmt(" %d", s->m_allocated_resources->get_nodeid()); + if (ps.str) + ps.str->appfmt(" %d", s->m_allocated_resources->get_nodeid()); + s->stopSession(); + } +} + +void +MgmApiSession::stop_session_if_timed_out(SocketServer::Session *_s, void *data) +{ + MgmApiSession *s= (MgmApiSession *)_s; + struct PurgeStruct &ps= *(struct PurgeStruct *)data; + if (s->m_allocated_resources->is_reserved(ps.free_nodes) && + s->m_allocated_resources->is_timed_out(ps.tick)) + { s->stopSession(); } } diff --git a/ndb/src/mgmsrv/Services.hpp b/ndb/src/mgmsrv/Services.hpp index f97223750a1..975202b96df 100644 --- a/ndb/src/mgmsrv/Services.hpp +++ b/ndb/src/mgmsrv/Services.hpp @@ -30,6 +30,7 @@ class MgmApiSession : public SocketServer::Session { + static void stop_session_if_timed_out(SocketServer::Session *_s, void *data); static void stop_session_if_not_connected(SocketServer::Session *_s, void *data); private: typedef Parser Parser_t; From edba345e568ec5b4234a1dc8e9a39f2655307e69 Mon Sep 17 00:00:00 2001 From: "tomas@poseidon.ndb.mysql.com" <> Date: Wed, 26 Apr 2006 16:57:44 +0200 Subject: [PATCH 09/10] Bug #19202 Incorrect errorhandling in select count(*) wrt temporary error - added retry handling of temporary transaction errors --- sql/ha_ndbcluster.cc | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/sql/ha_ndbcluster.cc b/sql/ha_ndbcluster.cc index bdd49b06b98..138e8c72e7a 100644 --- a/sql/ha_ndbcluster.cc +++ b/sql/ha_ndbcluster.cc @@ -5650,12 +5650,24 @@ ndb_get_table_statistics(Ndb* ndb, const char * table, { DBUG_ENTER("ndb_get_table_statistics"); DBUG_PRINT("enter", ("table: %s", table)); - NdbTransaction* pTrans= ndb->startTransaction(); - do + NdbTransaction* pTrans; + int retries= 10; + int retry_sleep= 30 * 1000; /* 30 milliseconds */ + + do { + pTrans= ndb->startTransaction(); if (pTrans == NULL) + { + if (ndb->getNdbError().status == NdbError::TemporaryError && + retries--) + { + my_sleep(retry_sleep); + continue; + } break; - + } + NdbScanOperation* pOp= pTrans->getNdbScanOperation(table); if (pOp == NULL) break; @@ -5678,8 +5690,18 @@ ndb_get_table_statistics(Ndb* ndb, const char * table, NdbTransaction::AbortOnError, TRUE); if (check == -1) + { + if (pTrans->getNdbError().status == NdbError::TemporaryError && + retries--) + { + ndb->closeTransaction(pTrans); + pTrans= 0; + my_sleep(retry_sleep); + continue; + } break; - + } + Uint32 count= 0; Uint64 sum_rows= 0; Uint64 sum_commits= 0; @@ -5713,7 +5735,7 @@ ndb_get_table_statistics(Ndb* ndb, const char * table, sum_mem, count)); DBUG_RETURN(0); - } while (0); + } while(1); if (pTrans) ndb->closeTransaction(pTrans); From ee3bdf586b128b4985730df9ff4c598b4423d0d9 Mon Sep 17 00:00:00 2001 From: "tomas@poseidon.ndb.mysql.com" <> Date: Wed, 26 Apr 2006 16:57:45 +0200 Subject: [PATCH 10/10] Bug #18550 ndbd getting "node failure handling not complete..." after graceful restart - addded more retries to wait for nodefailure to complete Bug #19039 multi node failure causes node failure handling not to complete - patch to avoid this scenario when the management server is used to perform the stop - wait for NF_COMPLETE_REP in management server before returning ndb: allocate nodeid - only retry on retryable error --- ndb/include/mgmapi/mgmapi.h | 8 +- ndb/src/common/mgmcommon/ConfigRetriever.cpp | 6 +- ndb/src/kernel/vm/Configuration.cpp | 3 +- ndb/src/mgmapi/mgmapi.cpp | 13 +- ndb/src/mgmsrv/MgmtSrvr.cpp | 146 ++++++++++++++----- ndb/src/mgmsrv/MgmtSrvr.hpp | 6 +- ndb/src/mgmsrv/Services.cpp | 17 ++- 7 files changed, 148 insertions(+), 51 deletions(-) diff --git a/ndb/include/mgmapi/mgmapi.h b/ndb/include/mgmapi/mgmapi.h index c6c0fdebb65..0f3d75d50fb 100644 --- a/ndb/include/mgmapi/mgmapi.h +++ b/ndb/include/mgmapi/mgmapi.h @@ -232,6 +232,12 @@ extern "C" { /** Could not connect to socker */ NDB_MGM_COULD_NOT_CONNECT_TO_SOCKET = 1011, + /* Alloc node id failures */ + /** Generic error, retry may succeed */ + NDB_MGM_ALLOCID_ERROR = 1101, + /** Non retriable error */ + NDB_MGM_ALLOCID_CONFIG_MISMATCH = 1102, + /* Service errors - Start/Stop Node or System */ /** Start failed */ NDB_MGM_START_FAILED = 2001, @@ -999,7 +1005,7 @@ extern "C" { void ndb_mgm_destroy_configuration(struct ndb_mgm_configuration *); int ndb_mgm_alloc_nodeid(NdbMgmHandle handle, - unsigned version, int nodetype); + unsigned version, int nodetype, int log_event); /** * End Session diff --git a/ndb/src/common/mgmcommon/ConfigRetriever.cpp b/ndb/src/common/mgmcommon/ConfigRetriever.cpp index fb3531d81f6..c2b3e8235eb 100644 --- a/ndb/src/common/mgmcommon/ConfigRetriever.cpp +++ b/ndb/src/common/mgmcommon/ConfigRetriever.cpp @@ -349,12 +349,14 @@ ConfigRetriever::allocNodeId(int no_retries, int retry_delay_in_seconds) if(!ndb_mgm_connect(m_handle, 0, 0, 0)) goto next; - res= ndb_mgm_alloc_nodeid(m_handle, m_version, m_node_type); + res= ndb_mgm_alloc_nodeid(m_handle, m_version, m_node_type, + no_retries == 0 /* only log last retry */); if(res >= 0) return _ownNodeId= (Uint32)res; next: - if (no_retries == 0) + int error = ndb_mgm_get_latest_error(m_handle); + if (no_retries == 0 || error == NDB_MGM_ALLOCID_CONFIG_MISMATCH) break; no_retries--; NdbSleep_SecSleep(retry_delay_in_seconds); diff --git a/ndb/src/kernel/vm/Configuration.cpp b/ndb/src/kernel/vm/Configuration.cpp index 425907e24c6..b73a82df66b 100644 --- a/ndb/src/kernel/vm/Configuration.cpp +++ b/ndb/src/kernel/vm/Configuration.cpp @@ -286,7 +286,8 @@ Configuration::fetch_configuration(){ if (globalData.ownId) cr.setNodeId(globalData.ownId); - globalData.ownId = cr.allocNodeId(2 /*retry*/,3 /*delay*/); + globalData.ownId = cr.allocNodeId(globalData.ownId ? 10 : 2 /*retry*/, + 3 /*delay*/); if(globalData.ownId == 0){ ERROR_SET(fatal, NDBD_EXIT_INVALID_CONFIG, diff --git a/ndb/src/mgmapi/mgmapi.cpp b/ndb/src/mgmapi/mgmapi.cpp index b02367a8870..f99478a8cea 100644 --- a/ndb/src/mgmapi/mgmapi.cpp +++ b/ndb/src/mgmapi/mgmapi.cpp @@ -1868,7 +1868,8 @@ const char *ndb_mgm_get_connectstring(NdbMgmHandle handle, char *buf, int buf_sz extern "C" int -ndb_mgm_alloc_nodeid(NdbMgmHandle handle, unsigned int version, int nodetype) +ndb_mgm_alloc_nodeid(NdbMgmHandle handle, unsigned int version, int nodetype, + int log_event) { CHECK_HANDLE(handle, 0); CHECK_CONNECTED(handle, 0); @@ -1888,9 +1889,11 @@ ndb_mgm_alloc_nodeid(NdbMgmHandle handle, unsigned int version, int nodetype) args.put("endian", (endian_check.c[sizeof(long)-1])?"big":"little"); if (handle->m_name) args.put("name", handle->m_name); + args.put("log_event", log_event); const ParserRow reply[]= { MGM_CMD("get nodeid reply", NULL, ""), + MGM_ARG("error_code", Int, Optional, "Error code"), MGM_ARG("nodeid", Int, Optional, "Error message"), MGM_ARG("result", String, Mandatory, "Error message"), MGM_END() @@ -1903,14 +1906,16 @@ ndb_mgm_alloc_nodeid(NdbMgmHandle handle, unsigned int version, int nodetype) nodeid= -1; do { const char * buf; - if(!prop->get("result", &buf) || strcmp(buf, "Ok") != 0){ + if (!prop->get("result", &buf) || strcmp(buf, "Ok") != 0) + { const char *hostname= ndb_mgm_get_connected_host(handle); unsigned port= ndb_mgm_get_connected_port(handle); BaseString err; + Uint32 error_code= NDB_MGM_ALLOCID_ERROR; err.assfmt("Could not alloc node id at %s port %d: %s", hostname, port, buf); - setError(handle, NDB_MGM_COULD_NOT_CONNECT_TO_SOCKET, __LINE__, - err.c_str()); + prop->get("error_code", &error_code); + setError(handle, error_code, __LINE__, err.c_str()); break; } Uint32 _nodeid; diff --git a/ndb/src/mgmsrv/MgmtSrvr.cpp b/ndb/src/mgmsrv/MgmtSrvr.cpp index c7d0c11eec4..d40eaab7bd5 100644 --- a/ndb/src/mgmsrv/MgmtSrvr.cpp +++ b/ndb/src/mgmsrv/MgmtSrvr.cpp @@ -507,9 +507,10 @@ MgmtSrvr::MgmtSrvr(SocketServer *socket_server, if (_ownNodeId == 0) // we did not get node id from other server { NodeId tmp= m_config_retriever->get_configuration_nodeid(); + int error_code; if (!alloc_node_id(&tmp, NDB_MGM_NODE_TYPE_MGM, - 0, 0, error_string)){ + 0, 0, error_code, error_string)){ ndbout << "Unable to obtain requested nodeid: " << error_string.c_str() << endl; require(false); @@ -1118,31 +1119,16 @@ int MgmtSrvr::sendSTOP_REQ(const Vector &node_ids, const NFCompleteRep * const rep = CAST_CONSTPTR(NFCompleteRep, signal->getDataPtr()); #ifdef VM_TRACE - ndbout_c("Node %d fail completed", rep->failedNodeId); + ndbout_c("sendSTOP_REQ Node %d fail completed", rep->failedNodeId); #endif + nodes.clear(rep->failedNodeId); // clear the failed node + if (singleUserNodeId == 0) + stoppedNodes.set(rep->failedNodeId); break; } case GSN_NODE_FAILREP:{ const NodeFailRep * const rep = CAST_CONSTPTR(NodeFailRep, signal->getDataPtr()); - NodeBitmask failedNodes; - failedNodes.assign(NodeBitmask::Size, rep->theNodes); -#ifdef VM_TRACE - { - ndbout << "Failed nodes:"; - for (unsigned i = 0; i < 32*NodeBitmask::Size; i++) - if(failedNodes.get(i)) - ndbout << " " << i; - ndbout << endl; - } -#endif - failedNodes.bitAND(nodes); - if (!failedNodes.isclear()) - { - nodes.bitANDC(failedNodes); // clear the failed nodes - if (singleUserNodeId == 0) - stoppedNodes.bitOR(failedNodes); - } break; } default: @@ -1263,11 +1249,47 @@ int MgmtSrvr::restartNodes(const Vector &node_ids, abort, false, true, - nostart, + true, initialStart); + + if (ret) + return ret; + if (stopCount) *stopCount = nodes.count(); - return ret; + + // start up the nodes again + int waitTime = 12000; + NDB_TICKS maxTime = NdbTick_CurrentMillisecond() + waitTime; + for (unsigned i = 0; i < node_ids.size(); i++) + { + NodeId nodeId= node_ids[i]; + enum ndb_mgm_node_status s; + s = NDB_MGM_NODE_STATUS_NO_CONTACT; +#ifdef VM_TRACE + ndbout_c("Waiting for %d not started", nodeId); +#endif + while (s != NDB_MGM_NODE_STATUS_NOT_STARTED && waitTime > 0) + { + Uint32 startPhase = 0, version = 0, dynamicId = 0, nodeGroup = 0; + Uint32 connectCount = 0; + bool system; + const char *address; + status(nodeId, &s, &version, &startPhase, + &system, &dynamicId, &nodeGroup, &connectCount, &address); + NdbSleep_MilliSleep(100); + waitTime = (maxTime - NdbTick_CurrentMillisecond()); + } + } + + if (nostart) + return 0; + + for (unsigned i = 0; i < node_ids.size(); i++) + { + int result = start(node_ids[i]); + } + return 0; } /* @@ -1918,7 +1940,8 @@ MgmtSrvr::alloc_node_id(NodeId * nodeId, enum ndb_mgm_node_type type, struct sockaddr *client_addr, SOCKET_SIZE_TYPE *client_addr_len, - BaseString &error_string) + int &error_code, BaseString &error_string, + int log_event) { DBUG_ENTER("MgmtSrvr::alloc_node_id"); DBUG_PRINT("enter", ("nodeid=%d, type=%d, client_addr=%d", @@ -1927,6 +1950,7 @@ MgmtSrvr::alloc_node_id(NodeId * nodeId, if (*nodeId == 0) { error_string.appfmt("no-nodeid-checks set in management server.\n" "node id must be set explicitly in connectstring"); + error_code = NDB_MGM_ALLOCID_CONFIG_MISMATCH; DBUG_RETURN(false); } DBUG_RETURN(true); @@ -1951,8 +1975,10 @@ MgmtSrvr::alloc_node_id(NodeId * nodeId, if(NdbMutex_Lock(m_configMutex)) { + // should not happen error_string.appfmt("unable to lock configuration mutex"); - return false; + error_code = NDB_MGM_ALLOCID_ERROR; + DBUG_RETURN(false); } ndb_mgm_configuration_iterator iter(* _config->m_configValues, CFG_SECTION_NODE); @@ -2023,6 +2049,7 @@ MgmtSrvr::alloc_node_id(NodeId * nodeId, "or specifying unique host names in config file.", id_found, tmp); NdbMutex_Unlock(m_configMutex); + error_code = NDB_MGM_ALLOCID_CONFIG_MISMATCH; DBUG_RETURN(false); } if (config_hostname == 0) { @@ -2031,6 +2058,7 @@ MgmtSrvr::alloc_node_id(NodeId * nodeId, "or specifying unique host names in config file,\n" "or specifying just one mgmt server in config file.", tmp); + error_code = NDB_MGM_ALLOCID_CONFIG_MISMATCH; DBUG_RETURN(false); } id_found= tmp; // mgmt server matched, check for more matches @@ -2072,8 +2100,9 @@ MgmtSrvr::alloc_node_id(NodeId * nodeId, char tmp_str[128]; m_reserved_nodes.getText(tmp_str); - g_eventLogger.info("Mgmt server state: nodeid %d reserved for ip %s, m_reserved_nodes %s.", - id_found, get_connect_address(id_found), tmp_str); + g_eventLogger.info("Mgmt server state: nodeid %d reserved for ip %s, " + "m_reserved_nodes %s.", + id_found, get_connect_address(id_found), tmp_str); DBUG_RETURN(true); } @@ -2093,26 +2122,48 @@ MgmtSrvr::alloc_node_id(NodeId * nodeId, type_c_string.assfmt("%s(%s)", alias, str); } - if (*nodeId == 0) { + if (*nodeId == 0) + { if (found_matching_id) + { if (found_matching_type) + { if (found_free_node) + { error_string.appfmt("Connection done from wrong host ip %s.", (client_addr)? - inet_ntoa(((struct sockaddr_in *) + inet_ntoa(((struct sockaddr_in *) (client_addr))->sin_addr):""); + error_code = NDB_MGM_ALLOCID_ERROR; + } else + { error_string.appfmt("No free node id found for %s.", type_string.c_str()); + error_code = NDB_MGM_ALLOCID_ERROR; + } + } else + { error_string.appfmt("No %s node defined in config file.", type_string.c_str()); + error_code = NDB_MGM_ALLOCID_CONFIG_MISMATCH; + } + } else + { error_string.append("No nodes defined in config file."); - } else { + error_code = NDB_MGM_ALLOCID_CONFIG_MISMATCH; + } + } + else + { if (found_matching_id) + { if (found_matching_type) - if (found_free_node) { + { + if (found_free_node) + { // have to split these into two since inet_ntoa overwrites itself error_string.appfmt("Connection with id %d done from wrong host ip %s,", *nodeId, inet_ntoa(((struct sockaddr_in *) @@ -2120,27 +2171,44 @@ MgmtSrvr::alloc_node_id(NodeId * nodeId, error_string.appfmt(" expected %s(%s).", config_hostname, r_config_addr ? "lookup failed" : inet_ntoa(config_addr)); - } else + error_code = NDB_MGM_ALLOCID_CONFIG_MISMATCH; + } + else + { error_string.appfmt("Id %d already allocated by another node.", *nodeId); + error_code = NDB_MGM_ALLOCID_ERROR; + } + } else + { error_string.appfmt("Id %d configured as %s, connect attempted as %s.", *nodeId, type_c_string.c_str(), type_string.c_str()); + error_code = NDB_MGM_ALLOCID_CONFIG_MISMATCH; + } + } else + { error_string.appfmt("No node defined with id=%d in config file.", *nodeId); + error_code = NDB_MGM_ALLOCID_CONFIG_MISMATCH; + } } - g_eventLogger.warning("Allocate nodeid (%d) failed. Connection from ip %s. " - "Returned error string \"%s\"", - *nodeId, - client_addr != 0 ? inet_ntoa(((struct sockaddr_in *)(client_addr))->sin_addr) : "", - error_string.c_str()); - - NodeBitmask connected_nodes2; - get_connected_nodes(connected_nodes2); + if (log_event || error_code == NDB_MGM_ALLOCID_CONFIG_MISMATCH) { + g_eventLogger.warning("Allocate nodeid (%d) failed. Connection from ip %s." + " Returned error string \"%s\"", + *nodeId, + client_addr != 0 + ? inet_ntoa(((struct sockaddr_in *) + (client_addr))->sin_addr) + : "", + error_string.c_str()); + + NodeBitmask connected_nodes2; + get_connected_nodes(connected_nodes2); BaseString tmp_connected, tmp_not_connected; for(Uint32 i = 0; i < MAX_NODES; i++) { diff --git a/ndb/src/mgmsrv/MgmtSrvr.hpp b/ndb/src/mgmsrv/MgmtSrvr.hpp index e2eb33d1198..007494a277d 100644 --- a/ndb/src/mgmsrv/MgmtSrvr.hpp +++ b/ndb/src/mgmsrv/MgmtSrvr.hpp @@ -434,8 +434,10 @@ public: */ bool getNextNodeId(NodeId * _nodeId, enum ndb_mgm_node_type type) const ; bool alloc_node_id(NodeId * _nodeId, enum ndb_mgm_node_type type, - struct sockaddr *client_addr, SOCKET_SIZE_TYPE *client_addr_len, - BaseString &error_string); + struct sockaddr *client_addr, + SOCKET_SIZE_TYPE *client_addr_len, + int &error_code, BaseString &error_string, + int log_event = 1); /** * diff --git a/ndb/src/mgmsrv/Services.cpp b/ndb/src/mgmsrv/Services.cpp index ee97235912a..555c2082480 100644 --- a/ndb/src/mgmsrv/Services.cpp +++ b/ndb/src/mgmsrv/Services.cpp @@ -138,6 +138,7 @@ ParserRow commands[] = { MGM_ARG("endian", String, Optional, "Endianness"), MGM_ARG("name", String, Optional, "Name of connection"), MGM_ARG("timeout", Int, Optional, "Timeout in seconds"), + MGM_ARG("log_event", Int, Optional, "Log failure in cluster log"), MGM_CMD("get version", &MgmApiSession::getVersion, ""), @@ -425,6 +426,8 @@ MgmApiSession::get_nodeid(Parser_t::Context &, const char * public_key; const char * endian= NULL; const char * name= NULL; + Uint32 log_event= 1; + bool log_event_version; union { long l; char c[sizeof(long)]; } endian_check; args.get("version", &version); @@ -437,6 +440,8 @@ MgmApiSession::get_nodeid(Parser_t::Context &, args.get("endian", &endian); args.get("name", &name); args.get("timeout", &timeout); + /* for backwards compatability keep track if client uses new protocol */ + log_event_version= args.get("log_event", &log_event); endian_check.l = 1; if(endian @@ -476,11 +481,15 @@ MgmApiSession::get_nodeid(Parser_t::Context &, NodeId tmp= nodeid; if(tmp == 0 || !m_allocated_resources->is_reserved(tmp)){ BaseString error_string; + int error_code; NDB_TICKS tick= 0; + /* only report error on second attempt as not to clog the cluster log */ while (!m_mgmsrv.alloc_node_id(&tmp, (enum ndb_mgm_node_type)nodetype, - &addr, &addrlen, error_string)) + &addr, &addrlen, error_code, error_string, + tick == 0 ? 0 : log_event)) { - if (tick == 0) + /* NDB_MGM_ALLOCID_CONFIG_MISMATCH is a non retriable error */ + if (tick == 0 && error_code != NDB_MGM_ALLOCID_CONFIG_MISMATCH) { // attempt to free any timed out reservations tick= NdbTick_CurrentMillisecond(); @@ -492,6 +501,7 @@ MgmApiSession::get_nodeid(Parser_t::Context &, ps.tick= tick; m_mgmsrv.get_socket_server()-> foreachSession(stop_session_if_timed_out,&ps); + error_string = ""; continue; } const char *alias; @@ -500,6 +510,9 @@ MgmApiSession::get_nodeid(Parser_t::Context &, nodetype, &str); m_output->println(cmd); m_output->println("result: %s", error_string.c_str()); + /* only use error_code protocol if client knows about it */ + if (log_event_version) + m_output->println("error_code: %d", error_code); m_output->println(""); return; }