From a1c23807530c6ffd5d400f71d9226bca5b91fb62 Mon Sep 17 00:00:00 2001 From: Monty Date: Sun, 6 Feb 2022 16:05:48 +0200 Subject: MENT-328 Retry BACKUP STAGE BLOCK DDL in case of deadlocks MENT-328 wrongly assumed that the backup failed because of warnings from mariabackup about not found files. This is normal (and the error message should be deleted). randgen failed because mariabackup didn't retry BACKUP STAGE BLOCK DDL if it failed with a deadlock. To simplify things, I implemented the retry loop in the server as this particular deadlock should be quickly resolved. --- sql/backup.cc | 37 ++++++++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 9 deletions(-) (limited to 'sql') diff --git a/sql/backup.cc b/sql/backup.cc index 539dc9c31f4..c856048b49a 100644 --- a/sql/backup.cc +++ b/sql/backup.cc @@ -233,8 +233,12 @@ static bool backup_flush(THD *thd) This will probably require a callback from the InnoDB code. */ +/* Retry to get inital lock for 0.1 + 0.5 + 2.25 + 11.25 + 56.25 = 70.35 sec */ +#define MAX_RETRY_COUNT 5 + static bool backup_block_ddl(THD *thd) { + uint sleep_time; DBUG_ENTER("backup_block_ddl"); kill_delayed_threads(); @@ -275,17 +279,32 @@ static bool backup_block_ddl(THD *thd) block new DDL's, in addition to all previous blocks We didn't do this lock above, as we wanted DDL's to be executed while we wait for non transactional tables (which may take a while). + + We do this lock in a loop as we can get a deadlock if there are multi-object + ddl statements like + RENAME TABLE t1 TO t2, t3 TO t3 + and the MDL happens in the middle of it. */ - if (thd->mdl_context.upgrade_shared_lock(backup_flush_ticket, - MDL_BACKUP_WAIT_DDL, - thd->variables.lock_wait_timeout)) + sleep_time= 100; // Start with 0.1 seconds + for (uint i= 0 ; i <= MAX_RETRY_COUNT ; i++) { - /* - Could be a timeout. Downgrade lock to what is was before this function - was called so that this function can be called again - */ - backup_flush_ticket->downgrade_lock(MDL_BACKUP_FLUSH); - DBUG_RETURN(1); + if (!thd->mdl_context.upgrade_shared_lock(backup_flush_ticket, + MDL_BACKUP_WAIT_DDL, + thd->variables.lock_wait_timeout)) + break; + if (thd->get_stmt_da()->sql_errno() != ER_LOCK_DEADLOCK || thd->killed || + i == MAX_RETRY_COUNT) + { + /* + Could be a timeout. Downgrade lock to what is was before this function + was called so that this function can be called again + */ + backup_flush_ticket->downgrade_lock(MDL_BACKUP_FLUSH); + DBUG_RETURN(1); + } + thd->clear_error(); // Forget the DEADLOCK error + my_sleep(sleep_time); + sleep_time*= 5; // Wait a bit longer next time } DBUG_RETURN(0); } -- cgit v1.2.1 From d314bd266491baf0954d13fa51dc22b730a6f4d1 Mon Sep 17 00:00:00 2001 From: Monty Date: Wed, 2 Feb 2022 14:09:21 +0200 Subject: MDEV-27442 Wrong result upon query with DISTINCT and EXISTS subquery The problem was that get_best_group_min_max() did not check if fields used by the "group_min_max optimization" where used in sub queries. Because of this, it did not detect that a key (b,a) was used in the WHERE clause for the statement: SELECT DISTINCT b FROM t1 WHERE EXISTS ( SELECT 1 FROM DUAL WHERE a > 1 ). Fixed by also traversing the sub queries when checking if a field is used. This disables group_min_max_optimization for the above query. Reviewer: Sergei Petrunia --- sql/opt_range.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'sql') diff --git a/sql/opt_range.cc b/sql/opt_range.cc index ae24a257aaa..e3d26896900 100644 --- a/sql/opt_range.cc +++ b/sql/opt_range.cc @@ -14000,7 +14000,7 @@ get_best_group_min_max(PARAM *param, SEL_TREE *tree, double read_time) key_part_range[1]= last_part; /* Check if cur_part is referenced in the WHERE clause. */ - if (join->conds->walk(&Item::find_item_in_field_list_processor, 0, + if (join->conds->walk(&Item::find_item_in_field_list_processor, true, key_part_range)) { cause= "keypart reference from where clause"; -- cgit v1.2.1 From 38058c04a4fc021f381f8000e40ed23bd4fb8d75 Mon Sep 17 00:00:00 2001 From: Monty Date: Wed, 2 Feb 2022 14:25:25 +0200 Subject: MDEV-26585 Wrong query results when `using index for group-by` The problem was that "group_min_max optimization" does not work if some aggregate functions, like COUNT(*), is used. The function get_best_group_min_max() is using the join->sum_funcs array to check which aggregate functions are used. The bug was that aggregates in HAVING where not yet added to join->sum_funcs at the time get_best_group_min_max() was called. Fixed by populate join->sum_funcs already in prepare, which means that all sum functions will be in join->sum_funcs in get_best_group_min_max(). A benefit of this approach is that we can remove several calls to make_sum_func_list() from the code and simplify the function. I removed some wrong setting of 'sort_and_group'. This variable is set when alloc_group_fields() is called, as part of allocating the cache needed by end_send_group() and does not need to be set by other functions. One problematic thing was that Spider is using *join->sum_funcs to detect at which stage the optimizer is and do internal calculations of aggregate functions. Updating join->sum_funcs early caused Spider to fail when trying to find min/max values in opt_sum_query(). Fixed by temporarily resetting sum_funcs during opt_sum_query(). Reviewer: Sergei Petrunia --- sql/sql_select.cc | 72 +++++++++++++++++++++++++++++++++---------------------- sql/sql_select.h | 14 +++++++++-- 2 files changed, 55 insertions(+), 31 deletions(-) (limited to 'sql') diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 6fdde5f82dd..fdf58349fc2 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -1589,7 +1589,8 @@ bool JOIN::prepare_stage2() #endif if (select_lex->olap == ROLLUP_TYPE && rollup_init()) goto err; - if (alloc_func_list()) + if (alloc_func_list() || + make_sum_func_list(all_fields, fields_list, false)) goto err; res= FALSE; @@ -2204,7 +2205,21 @@ JOIN::optimize_inner() If all items were resolved by opt_sum_query, there is no need to open any tables. */ - if ((res=opt_sum_query(thd, select_lex->leaf_tables, all_fields, conds))) + + /* + The following resetting and restoring of sum_funcs is needed to + go around a bug in spider where it assumes that + make_sum_func_list() has not been called yet and do logical + choices based on this if special handling of min/max functions should + be done. We disable this special handling while we are trying to find + out if we can replace MIN/MAX values with constants. + */ + Item_sum **save_func_sums= sum_funcs, *tmp_sum_funcs= 0; + sum_funcs= &tmp_sum_funcs; + res= opt_sum_query(thd, select_lex->leaf_tables, all_fields, conds); + sum_funcs= save_func_sums; + + if (res) { DBUG_ASSERT(res >= 0); if (res == HA_ERR_KEY_NOT_FOUND) @@ -2776,13 +2791,15 @@ int JOIN::optimize_stage2() calc_group_buffer(this, group_list); } - if (test_if_subpart(group_list, order) || - (!group_list && tmp_table_param.sum_func_count)) - { + /* + We can ignore ORDER BY if it's a prefix of the GROUP BY list + (as MariaDB is by default sorting on GROUP BY) or + if there is no GROUP BY and aggregate functions are used + (as the result will only contain one row). + */ + if (order && (test_if_subpart(group_list, order) || + (!group_list && tmp_table_param.sum_func_count))) order=0; - if (is_indexed_agg_distinct(this, NULL)) - sort_and_group= 0; - } // Can't use sort on head table if using join buffering if (full_join || hash_join) @@ -2814,7 +2831,6 @@ int JOIN::optimize_stage2() if (select_lex->have_window_funcs()) simple_order= FALSE; - /* If the hint FORCE INDEX FOR ORDER BY/GROUP BY is used for the table whose columns are required to be returned in a sorted order, then @@ -3540,7 +3556,7 @@ bool JOIN::make_aggr_tables_info() // for the first table if (group_list || tmp_table_param.sum_func_count) { - if (make_sum_func_list(*curr_all_fields, *curr_fields_list, true, true)) + if (make_sum_func_list(*curr_all_fields, *curr_fields_list, true)) DBUG_RETURN(true); if (prepare_sum_aggregators(sum_funcs, !join_tab->is_using_agg_loose_index_scan())) @@ -3650,7 +3666,7 @@ bool JOIN::make_aggr_tables_info() last_tab->all_fields= &tmp_all_fields3; last_tab->fields= &tmp_fields_list3; } - if (make_sum_func_list(*curr_all_fields, *curr_fields_list, true, true)) + if (make_sum_func_list(*curr_all_fields, *curr_fields_list, true)) DBUG_RETURN(true); if (prepare_sum_aggregators(sum_funcs, !join_tab || @@ -3846,8 +3862,6 @@ JOIN::create_postjoin_aggr_table(JOIN_TAB *tab, List *table_fields, } else { - if (make_sum_func_list(all_fields, fields_list, false)) - goto err; if (prepare_sum_aggregators(sum_funcs, !join_tab->is_using_agg_loose_index_scan())) goto err; @@ -7089,8 +7103,7 @@ void optimize_keyuse(JOIN *join, DYNAMIC_ARRAY *keyuse_array) Check for the presence of AGGFN(DISTINCT a) queries that may be subject to loose index scan. - - Check if the query is a subject to AGGFN(DISTINCT) using loose index scan + Check if the query is a subject to AGGFN(DISTINCT) using loose index scan (QUICK_GROUP_MIN_MAX_SELECT). Optionally (if out_args is supplied) will push the arguments of AGGFN(DISTINCT) to the list @@ -7123,14 +7136,11 @@ is_indexed_agg_distinct(JOIN *join, List *out_args) Item_sum **sum_item_ptr; bool result= false; - if (join->table_count != 1 || /* reference more than 1 table */ + if (join->table_count != 1 || /* reference more than 1 table */ join->select_distinct || /* or a DISTINCT */ join->select_lex->olap == ROLLUP_TYPE) /* Check (B3) for ROLLUP */ return false; - if (join->make_sum_func_list(join->all_fields, join->fields_list, true)) - return false; - Bitmap first_aggdistinct_fields; bool first_aggdistinct_fields_initialized= false; for (sum_item_ptr= join->sum_funcs; *sum_item_ptr; sum_item_ptr++) @@ -7232,16 +7242,23 @@ add_group_and_distinct_keys(JOIN *join, JOIN_TAB *join_tab) while ((item= select_items_it++)) item->walk(&Item::collect_item_field_processor, 0, &indexed_fields); } - else if (join->tmp_table_param.sum_func_count && - is_indexed_agg_distinct(join, &indexed_fields)) + else if (!join->tmp_table_param.sum_func_count || + !is_indexed_agg_distinct(join, &indexed_fields)) { - join->sort_and_group= 1; - } - else + /* + There where no GROUP BY fields and also either no aggregate + functions or not all aggregate functions where used with the + same DISTINCT (or MIN() / MAX() that works similarly). + Nothing to do there. + */ return; + } if (indexed_fields.elements == 0) + { + /* There where no index we could use to satisfy the GROUP BY */ return; + } /* Intersect the keys of all group fields. */ cur_item= indexed_fields_it++; @@ -25692,16 +25709,13 @@ bool JOIN::alloc_func_list() bool JOIN::make_sum_func_list(List &field_list, List &send_result_set_metadata, - bool before_group_by, bool recompute) + bool before_group_by) { List_iterator_fast it(field_list); Item_sum **func; Item *item; DBUG_ENTER("make_sum_func_list"); - if (*sum_funcs && !recompute) - DBUG_RETURN(FALSE); /* We have already initialized sum_funcs. */ - func= sum_funcs; while ((item=it++)) { @@ -25848,7 +25862,7 @@ change_to_use_tmp_fields(THD *thd, Ref_ptr_array ref_pointer_array, Change all funcs to be fields in tmp table. @param thd THD pointer - @param ref_pointer_array array of pointers to top elements of filed list + @param ref_pointer_array array of pointers to top elements of field list @param res_selected_fields new list of items of select item list @param res_all_fields new list of all items @param elements number of elements in select item list diff --git a/sql/sql_select.h b/sql/sql_select.h index 29e42ff8ef8..eea1ca9becd 100644 --- a/sql/sql_select.h +++ b/sql/sql_select.h @@ -1196,7 +1196,17 @@ public: Indicates that grouping will be performed on the result set during query execution. This field belongs to query execution. - @see make_group_fields, alloc_group_fields, JOIN::exec + If 'sort_and_group' is set, then the optimizer is going to use on of + the following algorithms to resolve GROUP BY. + + - If one table, sort the table and then calculate groups on the fly. + - If more than one table, create a temporary table to hold the join, + sort it and then resolve group by on the fly. + + The 'on the fly' calculation is done in end_send_group() + + @see make_group_fields, alloc_group_fields, JOIN::exec, + setup_end_select_func */ bool sort_and_group; bool first_record,full_join, no_field_update; @@ -1654,7 +1664,7 @@ public: bool make_range_rowid_filters(); bool init_range_rowid_filters(); bool make_sum_func_list(List &all_fields, List &send_fields, - bool before_group_by, bool recompute= FALSE); + bool before_group_by); /// Initialzes a slice, see comments for ref_ptrs above. Ref_ptr_array ref_ptr_array_slice(size_t slice_num) -- cgit v1.2.1