From 8e5d9991d19d26b6be9a7d9ef13516e14a69e8c4 Mon Sep 17 00:00:00 2001 From: Tor Didriksen Date: Tue, 14 Feb 2012 08:11:28 +0100 Subject: Bug#13633383 63183: SMALL SORT_BUFFER_SIZE CRASH IN MERGE_BUFFERS This patch is a backport of some of the cleanups/refactorings that were done as part of WL#1393 Optimizing filesort with small limit. --- sql/filesort.cc | 179 +++++++++++++++++++++++++------------------------------- 1 file changed, 81 insertions(+), 98 deletions(-) (limited to 'sql/filesort.cc') diff --git a/sql/filesort.cc b/sql/filesort.cc index e7740cfdd8a..23f48c0033f 100644 --- a/sql/filesort.cc +++ b/sql/filesort.cc @@ -48,7 +48,7 @@ static uchar *read_buffpek_from_file(IO_CACHE *buffer_file, uint count, uchar *buf); static ha_rows find_all_keys(SORTPARAM *param,SQL_SELECT *select, uchar * *sort_keys, IO_CACHE *buffer_file, - IO_CACHE *tempfile,IO_CACHE *indexfile); + IO_CACHE *tempfile); static int write_keys(SORTPARAM *param,uchar * *sort_keys, uint count, IO_CACHE *buffer_file, IO_CACHE *tempfile); static void make_sortkey(SORTPARAM *param,uchar *to, uchar *ref_pos); @@ -88,8 +88,6 @@ static void unpack_addon_fields(struct st_sort_addon_field *addon_field, (Needed by UPDATE/INSERT or ALTER TABLE) @param examined_rows Store number of examined rows here - @todo - check why we do this (param.keys--) @note If we sort by position (like if sort_positions is 1) filesort() will call table->prepare_for_position(). @@ -107,12 +105,12 @@ ha_rows filesort(THD *thd, TABLE *table, SORT_FIELD *sortorder, uint s_length, bool sort_positions, ha_rows *examined_rows) { int error; - ulong memavl, min_sort_memory; + ulong memory_available= thd->variables.sortbuff_size; uint maxbuffer; BUFFPEK *buffpek; - ha_rows records= HA_POS_ERROR; + ha_rows num_rows= HA_POS_ERROR; uchar **sort_keys= 0; - IO_CACHE tempfile, buffpek_pointers, *selected_records_file, *outfile; + IO_CACHE tempfile, buffpek_pointers, *outfile; SORTPARAM param; bool multi_byte_charset; DBUG_ENTER("filesort"); @@ -187,86 +185,71 @@ ha_rows filesort(THD *thd, TABLE *table, SORT_FIELD *sortorder, uint s_length, param.max_rows= max_rows; if (select && select->quick) - { status_var_increment(thd->status_var.filesort_range_count); - } else - { status_var_increment(thd->status_var.filesort_scan_count); - } -#ifdef CAN_TRUST_RANGE - if (select && select->quick && select->quick->records > 0L) - { - records=min((ha_rows) (select->quick->records*2+EXTRA_RECORDS*2), - table->file->stats.records)+EXTRA_RECORDS; - selected_records_file=0; - } - else -#endif - { - records= table->file->estimate_rows_upper_bound(); - /* - If number of records is not known, use as much of sort buffer - as possible. - */ - if (records == HA_POS_ERROR) - records--; // we use 'records+1' below. - selected_records_file= 0; - } + + // If number of rows is not known, use as much of sort buffer as possible. + num_rows= table->file->estimate_rows_upper_bound(); if (multi_byte_charset && !(param.tmp_buffer= (char*) my_malloc(param.sort_length,MYF(MY_WME)))) goto err; - memavl= thd->variables.sortbuff_size; - min_sort_memory= max(MIN_SORT_MEMORY, param.sort_length*MERGEBUFF2); - while (memavl >= min_sort_memory) { - ulong old_memavl; - ulong keys= memavl/(param.rec_length+sizeof(char*)); - param.keys=(uint) min(records+1, keys); - - if (table_sort.sort_keys && - table_sort.sort_keys_size != char_array_size(param.keys, - param.rec_length)) + const ulong min_sort_memory= + max(MIN_SORT_MEMORY, param.sort_length * MERGEBUFF2); + while (memory_available >= min_sort_memory) { - my_free(table_sort.sort_keys); - table_sort.sort_keys= NULL; - table_sort.sort_keys_size= 0; + ulong keys= memory_available / (param.rec_length + sizeof(char*)); + param.keys=(uint) min(num_rows, keys); + if (table_sort.sort_keys && + table_sort.sort_keys_size != char_array_size(param.keys, + param.rec_length)) + { + my_free(table_sort.sort_keys); + table_sort.sort_keys= NULL; + table_sort.sort_keys_size= 0; + } + if ((table_sort.sort_keys= + make_char_array(table_sort.sort_keys, + param.keys, param.rec_length, MYF(0)))) + { + table_sort.sort_keys_size= + char_array_size(param.keys, param.rec_length); + break; + } + ulong old_memory_available= memory_available; + memory_available= memory_available/4*3; + if (memory_available < min_sort_memory && + old_memory_available > min_sort_memory) + memory_available= min_sort_memory; } - if ((table_sort.sort_keys= - make_char_array(table_sort.sort_keys, - param.keys, param.rec_length, MYF(0)))) + sort_keys= table_sort.sort_keys; + if (memory_available < min_sort_memory) { - table_sort.sort_keys_size= char_array_size(param.keys, param.rec_length); - break; + my_error(ER_OUT_OF_SORTMEMORY,MYF(ME_ERROR+ME_WAITTANG)); + goto err; } - old_memavl=memavl; - if ((memavl=memavl/4*3) < min_sort_memory && old_memavl > min_sort_memory) - memavl= min_sort_memory; - } - sort_keys= table_sort.sort_keys; - if (memavl < min_sort_memory) - { - my_error(ER_OUT_OF_SORTMEMORY,MYF(ME_ERROR+ME_WAITTANG)); - goto err; } if (open_cached_file(&buffpek_pointers,mysql_tmpdir,TEMP_PREFIX, DISK_BUFFER_SIZE, MYF(MY_WME))) goto err; - param.keys--; /* TODO: check why we do this */ param.sort_form= table; param.end=(param.local_sortorder=sortorder)+s_length; - if ((records=find_all_keys(¶m,select,sort_keys, &buffpek_pointers, - &tempfile, selected_records_file)) == - HA_POS_ERROR) + num_rows= find_all_keys(¶m, + select, + sort_keys, + &buffpek_pointers, + &tempfile); + if (num_rows == HA_POS_ERROR) goto err; maxbuffer= (uint) (my_b_tell(&buffpek_pointers)/sizeof(*buffpek)); if (maxbuffer == 0) // The whole set is in memory { - if (save_index(¶m,sort_keys,(uint) records, &table_sort)) + if (save_index(¶m,sort_keys,(uint) num_rows, &table_sort)) goto err; } else @@ -298,8 +281,9 @@ ha_rows filesort(THD *thd, TABLE *table, SORT_FIELD *sortorder, uint s_length, Use also the space previously used by string pointers in sort_buffer for temporary key storage. */ - param.keys=((param.keys*(param.rec_length+sizeof(char*))) / - param.rec_length-1); + param.keys=((param.keys * + (param.rec_length+sizeof(char*))) / + param.rec_length - 1); maxbuffer--; // Offset from 0 if (merge_many_buff(¶m,(uchar*) sort_keys,buffpek,&maxbuffer, &tempfile)) @@ -307,13 +291,21 @@ ha_rows filesort(THD *thd, TABLE *table, SORT_FIELD *sortorder, uint s_length, if (flush_io_cache(&tempfile) || reinit_io_cache(&tempfile,READ_CACHE,0L,0,0)) goto err; - if (merge_index(¶m,(uchar*) sort_keys,buffpek,maxbuffer,&tempfile, + if (merge_index(¶m, + (uchar*) sort_keys, + buffpek, + maxbuffer, + &tempfile, outfile)) goto err; } - if (records > param.max_rows) - records=param.max_rows; - error =0; + + if (num_rows > param.max_rows) + { + // If find_all_keys() produced more results than the query LIMIT. + num_rows= param.max_rows; + } + error= 0; err: my_free(param.tmp_buffer); @@ -362,15 +354,15 @@ ha_rows filesort(THD *thd, TABLE *table, SORT_FIELD *sortorder, uint s_length, } else statistic_add(thd->status_var.filesort_rows, - (ulong) records, &LOCK_status); + (ulong) num_rows, &LOCK_status); *examined_rows= param.examined_rows; #ifdef SKIP_DBUG_IN_FILESORT DBUG_POP(); /* Ok to DBUG */ #endif memcpy(&table->sort, &table_sort, sizeof(FILESORT_INFO)); - DBUG_PRINT("exit",("records: %ld", (long) records)); - MYSQL_FILESORT_DONE(error, records); - DBUG_RETURN(error ? HA_POS_ERROR : records); + DBUG_PRINT("exit",("num_rows: %ld", (long) num_rows)); + MYSQL_FILESORT_DONE(error, num_rows); + DBUG_RETURN(error ? HA_POS_ERROR : num_rows); } /* filesort */ @@ -513,7 +505,6 @@ static void dbug_print_record(TABLE *table, bool print_rowid) @param buffpek_pointers File to write BUFFPEKs describing sorted segments in tempfile. @param tempfile File to write sorted sequences of sortkeys to. - @param indexfile If !NULL, use it for source data (contains rowids) @note Basic idea: @@ -543,7 +534,7 @@ static void dbug_print_record(TABLE *table, bool print_rowid) static ha_rows find_all_keys(SORTPARAM *param, SQL_SELECT *select, uchar **sort_keys, IO_CACHE *buffpek_pointers, - IO_CACHE *tempfile, IO_CACHE *indexfile) + IO_CACHE *tempfile) { int error,flag,quick_select; uint idx,indexpos,ref_length; @@ -568,12 +559,11 @@ static ha_rows find_all_keys(SORTPARAM *param, SQL_SELECT *select, ref_pos= ref_buff; quick_select=select && select->quick; record=0; - flag= ((!indexfile && file->ha_table_flags() & HA_REC_NOT_IN_SEQ) - || quick_select); - if (indexfile || flag) + flag= ((file->ha_table_flags() & HA_REC_NOT_IN_SEQ) || quick_select); + if (flag) ref_pos= &file->ref[0]; next_pos=ref_pos; - if (! indexfile && ! quick_select) + if (!quick_select) { next_pos=(uchar*) 0; /* Find records in sequence */ file->ha_rnd_init(1); @@ -611,18 +601,8 @@ static ha_rows find_all_keys(SORTPARAM *param, SQL_SELECT *select, } else /* Not quick-select */ { - if (indexfile) - { - if (my_b_read(indexfile,(uchar*) ref_pos,ref_length)) /* purecov: deadcode */ - { - error= my_errno ? my_errno : -1; /* Abort */ - break; - } - error=file->rnd_pos(sort_form->record[0],next_pos); - } - else { - error=file->rnd_next(sort_form->record[0]); + error= file->rnd_next(sort_form->record[0]); if (!flag) { my_store_ptr(ref_pos,ref_length,record); // Position to row @@ -638,7 +618,7 @@ static ha_rows find_all_keys(SORTPARAM *param, SQL_SELECT *select, if (*killed) { DBUG_PRINT("info",("Sort killed by user")); - if (!indexfile && !quick_select) + if (!quick_select) { (void) file->extra(HA_EXTRA_NO_CACHE); file->ha_rnd_end(); @@ -652,9 +632,10 @@ static ha_rows find_all_keys(SORTPARAM *param, SQL_SELECT *select, { if (idx == param->keys) { - if (write_keys(param,sort_keys,idx,buffpek_pointers,tempfile)) + if (write_keys(param, sort_keys, + idx, buffpek_pointers, tempfile)) DBUG_RETURN(HA_POS_ERROR); - idx=0; + idx= 0; indexpos++; } make_sortkey(param,sort_keys[idx++],ref_pos); @@ -681,15 +662,17 @@ static ha_rows find_all_keys(SORTPARAM *param, SQL_SELECT *select, DBUG_PRINT("test",("error: %d indexpos: %d",error,indexpos)); if (error != HA_ERR_END_OF_FILE) { - file->print_error(error,MYF(ME_ERROR | ME_WAITTANG)); /* purecov: inspected */ + file->print_error(error,MYF(ME_ERROR | ME_WAITTANG)); // purecov: inspected DBUG_RETURN(HA_POS_ERROR); /* purecov: inspected */ } if (indexpos && idx && - write_keys(param,sort_keys,idx,buffpek_pointers,tempfile)) + write_keys(param, sort_keys, + idx, buffpek_pointers, tempfile)) DBUG_RETURN(HA_POS_ERROR); /* purecov: inspected */ - DBUG_RETURN(my_b_inited(tempfile) ? - (ha_rows) (my_b_tell(tempfile)/param->rec_length) : - idx); + const ha_rows retval= + my_b_inited(tempfile) ? + (ha_rows) (my_b_tell(tempfile)/param->rec_length) : idx; + DBUG_RETURN(retval); } /* find_all_keys */ @@ -1269,8 +1252,8 @@ int merge_buffers(SORTPARAM *param, IO_CACHE *from_file, { buffpek->base= strpos; buffpek->max_keys= maxcount; - strpos+= (uint) (error= (int) read_to_buffer(from_file, buffpek, - rec_length)); + strpos+= + (uint) (error= (int) read_to_buffer(from_file, buffpek, rec_length)); if (error == -1) goto err; /* purecov: inspected */ buffpek->max_keys= buffpek->mem_count; // If less data in buffers than expected -- cgit v1.2.1