Skip to content

Commit b140456

Browse files
author
Konstantin Osipov
committed
WL#5419 "LOCK_open scalability: make tdc_refresh_version
an atomic counter" Split the large LOCK_open section in open_table(). Do not call open_table_from_share() under LOCK_open. Remove thd->version. This fixes Bug#50589 "Server hang on a query evaluated using a temporary table" Bug#51557 "LOCK_open and kernel_mutex are not happy together" Bug#49463 "LOCK_table and innodb are not nice when handler instances are created". This patch has effect on storage engines that rely on ha_open() PSEA method being called under LOCK_open. In particular: 1) NDB is broken and left unfixed. NDB relies on LOCK_open being kept as part of ha_open(), since it uses auto-discovery. While previously the NDB open code was race-prone, now it simply fails on asserts. 2) HEAP engine had a race in ha_heap::open() when a share for the same table could be added twice to the list of shares, or a dangling reference to a share stored in HEAP handler. This patch aims to address this problem by 'pinning' the newly created share in the internal HEAP engine share list until at least one handler instance is created using that share.
1 parent 218bf86 commit b140456

File tree

17 files changed

+207
-146
lines changed

17 files changed

+207
-146
lines changed

‎include/heap.h‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -184,19 +184,30 @@ typedef struct st_heap_info
184184

185185
typedef struct st_heap_create_info
186186
{
187+
HP_KEYDEF *keydef;
188+
ulong max_records;
189+
ulong min_records;
187190
uint auto_key; /* keynr [1 - maxkey] for auto key */
188191
uint auto_key_type;
192+
uint keys;
193+
uint reclength;
189194
ulonglong max_table_size;
190195
ulonglong auto_increment;
191196
my_bool with_auto_increment;
192197
my_bool internal_table;
198+
/*
199+
TRUE if heap_create should 'pin' the created share by setting
200+
open_count to 1. Is only looked at if not internal_table.
201+
*/
202+
my_bool pin_share;
193203
} HP_CREATE_INFO;
194204

195205
/* Prototypes for heap-functions */
196206

197207
extern HP_INFO *heap_open(const char *name, int mode);
198208
extern HP_INFO *heap_open_from_share(HP_SHARE *share, int mode);
199209
extern HP_INFO *heap_open_from_share_and_register(HP_SHARE *share, int mode);
210+
extern void heap_release_share(HP_SHARE *share, my_bool internal_table);
200211
extern int heap_close(HP_INFO *info);
201212
extern int heap_write(HP_INFO *info,const uchar *buff);
202213
extern int heap_update(HP_INFO *info,const uchar *old,const uchar *newdata);
@@ -205,9 +216,9 @@ extern int heap_scan_init(HP_INFO *info);
205216
extern int heap_scan(register HP_INFO *info, uchar *record);
206217
extern int heap_delete(HP_INFO *info,const uchar *buff);
207218
extern int heap_info(HP_INFO *info,HEAPINFO *x,int flag);
208-
extern int heap_create(const char *name, uint keys, HP_KEYDEF *keydef,
209-
uint reclength, ulong max_records, ulong min_records,
210-
HP_CREATE_INFO *create_info, HP_SHARE **share);
219+
extern int heap_create(const char *name,
220+
HP_CREATE_INFO *create_info, HP_SHARE **share,
221+
my_bool *created_new_share);
211222
extern int heap_delete_table(const char *name);
212223
extern void heap_drop_table(HP_INFO *info);
213224
extern int heap_extra(HP_INFO *info,enum ha_extra_function function);

‎sql/lock.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1298,7 +1298,8 @@ wait_if_global_read_lock(THD *thd, bool abort_on_refresh,
12981298
old_message=thd->enter_cond(&COND_global_read_lock, &LOCK_global_read_lock,
12991299
"Waiting for release of readlock");
13001300
while (must_wait && ! thd->killed &&
1301-
(!abort_on_refresh || thd->version == refresh_version))
1301+
(!abort_on_refresh || !thd->open_tables ||
1302+
thd->open_tables->s->version == refresh_version))
13021303
{
13031304
DBUG_PRINT("signal", ("Waiting for COND_global_read_lock"));
13041305
mysql_cond_wait(&COND_global_read_lock, &LOCK_global_read_lock);

‎sql/repl_failsafe.cc‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,6 @@ static int init_failsafe_rpl_thread(THD* thd)
102102

103103
thd->mem_root->free= thd->mem_root->used= 0;
104104
thd_proc_info(thd, "Thread initialized");
105-
thd->version=refresh_version;
106105
thd->set_time();
107106
DBUG_RETURN(0);
108107
}

‎sql/sql_base.cc‎

Lines changed: 46 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1589,25 +1589,26 @@ bool close_thread_table(THD *thd, TABLE **table_ptr)
15891589
*table_ptr=table->next;
15901590
mysql_mutex_unlock(&thd->LOCK_thd_data);
15911591

1592+
if (! table->needs_reopen())
1593+
{
1594+
/* Avoid having MERGE tables with attached children in unused_tables. */
1595+
table->file->extra(HA_EXTRA_DETACH_CHILDREN);
1596+
/* Free memory and reset for next loop. */
1597+
free_field_buffers_larger_than(table, MAX_TDC_BLOB_SIZE);
1598+
table->file->ha_reset();
1599+
}
1600+
15921601
mysql_mutex_lock(&LOCK_open);
15931602

1594-
if (table->s->needs_reopen() ||
1595-
thd->version != refresh_version || table->needs_reopen() ||
1603+
if (table->s->needs_reopen() || table->needs_reopen() ||
15961604
table_def_shutdown_in_progress)
15971605
{
15981606
free_cache_entry(table);
15991607
found_old_table= 1;
16001608
}
16011609
else
16021610
{
1603-
/* Avoid to have MERGE tables with attached children in unused_tables. */
16041611
DBUG_ASSERT(table->file);
1605-
table->file->extra(HA_EXTRA_DETACH_CHILDREN);
1606-
1607-
/* Free memory and reset for next loop */
1608-
free_field_buffers_larger_than(table,MAX_TDC_BLOB_SIZE);
1609-
1610-
table->file->ha_reset();
16111612
table_def_unuse_table(table);
16121613
/*
16131614
We free the least used table, not the subject table,
@@ -2305,36 +2306,38 @@ void wait_for_condition(THD *thd, mysql_mutex_t *mutex, mysql_cond_t *cond)
23052306
@param[out] exists Out parameter which is set to TRUE if table
23062307
exists and to FALSE otherwise.
23072308
2308-
@note This function assumes that caller owns LOCK_open mutex.
2309+
@note This function acquires LOCK_open internally.
23092310
It also assumes that the fact that there are no exclusive
23102311
metadata locks on the table was checked beforehand.
23112312
23122313
@note If there is no .FRM file for the table but it exists in one
23132314
of engines (e.g. it was created on another node of NDB cluster)
23142315
this function will fetch and create proper .FRM file for it.
23152316
2316-
@retval TRUE Some error occured
2317+
@retval TRUE Some error occurred
23172318
@retval FALSE No error. 'exists' out parameter set accordingly.
23182319
*/
23192320

23202321
bool check_if_table_exists(THD *thd, TABLE_LIST *table, bool *exists)
23212322
{
23222323
char path[FN_REFLEN + 1];
2323-
int rc;
2324+
int rc= 0;
23242325
DBUG_ENTER("check_if_table_exists");
23252326

2326-
mysql_mutex_assert_owner(&LOCK_open);
2327+
mysql_mutex_assert_not_owner(&LOCK_open);
23272328

23282329
*exists= TRUE;
23292330

2331+
mysql_mutex_lock(&LOCK_open);
2332+
23302333
if (get_cached_table_share(table->db, table->table_name))
2331-
DBUG_RETURN(FALSE);
2334+
goto end;
23322335

23332336
build_table_filename(path, sizeof(path) - 1, table->db, table->table_name,
23342337
reg_ext, 0);
23352338

23362339
if (!access(path, F_OK))
2337-
DBUG_RETURN(FALSE);
2340+
goto end;
23382341

23392342
/* .FRM file doesn't exist. Check if some engine can provide it. */
23402343

@@ -2344,19 +2347,17 @@ bool check_if_table_exists(THD *thd, TABLE_LIST *table, bool *exists)
23442347
{
23452348
/* Table does not exists in engines as well. */
23462349
*exists= FALSE;
2347-
DBUG_RETURN(FALSE);
2348-
}
2349-
else if (!rc)
2350-
{
2351-
/* Table exists in some engine and .FRM for it was created. */
2352-
DBUG_RETURN(FALSE);
2350+
rc= 0;
23532351
}
2354-
else /* (rc > 0) */
2352+
else if (rc)
23552353
{
23562354
my_printf_error(ER_UNKNOWN_ERROR, "Failed to open '%-.64s', error while "
23572355
"unpacking from engine", MYF(0), table->table_name);
2358-
DBUG_RETURN(TRUE);
23592356
}
2357+
2358+
end:
2359+
mysql_mutex_unlock(&LOCK_open);
2360+
DBUG_RETURN(test(rc));
23602361
}
23612362

23622363

@@ -2651,7 +2652,7 @@ bool open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
26512652
if (thd->global_read_lock.wait_if_global_read_lock(thd, 1, 1))
26522653
DBUG_RETURN(TRUE);
26532654

2654-
if (thd->version != refresh_version)
2655+
if (thd->open_tables && thd->open_tables->s->version != refresh_version)
26552656
{
26562657
(void) ot_ctx->request_backoff_action(Open_table_context::OT_WAIT_TDC,
26572658
NULL);
@@ -2848,48 +2849,24 @@ bool open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
28482849
}
28492850

28502851
hash_value= my_calc_hash(&table_def_cache, (uchar*) key, key_length);
2851-
mysql_mutex_lock(&LOCK_open);
28522852

2853-
/*
2854-
If it's the first table from a list of tables used in a query,
2855-
remember refresh_version (the version of open_cache state).
2856-
If the version changes while we're opening the remaining tables,
2857-
we will have to back off, close all the tables opened-so-far,
2858-
and try to reopen them.
2859-
Note: refresh_version is currently changed only during FLUSH TABLES.
2860-
*/
2861-
if (!thd->open_tables)
2862-
thd->version=refresh_version;
2863-
else if ((thd->version != refresh_version) &&
2864-
! (flags & MYSQL_OPEN_IGNORE_FLUSH))
2865-
{
2866-
/* Someone did a refresh while thread was opening tables */
2867-
mysql_mutex_unlock(&LOCK_open);
2868-
(void) ot_ctx->request_backoff_action(Open_table_context::OT_WAIT_TDC,
2869-
NULL);
2870-
DBUG_RETURN(TRUE);
2871-
}
28722853

28732854
if (table_list->open_strategy == TABLE_LIST::OPEN_IF_EXISTS)
28742855
{
28752856
bool exists;
28762857

28772858
if (check_if_table_exists(thd, table_list, &exists))
2878-
goto err_unlock2;
2859+
DBUG_RETURN(TRUE);
28792860

28802861
if (!exists)
2881-
{
2882-
mysql_mutex_unlock(&LOCK_open);
28832862
DBUG_RETURN(FALSE);
2884-
}
2863+
28852864
/* Table exists. Let us try to open it. */
28862865
}
28872866
else if (table_list->open_strategy == TABLE_LIST::OPEN_STUB)
2888-
{
2889-
mysql_mutex_unlock(&LOCK_open);
28902867
DBUG_RETURN(FALSE);
2891-
}
28922868

2869+
mysql_mutex_lock(&LOCK_open);
28932870
#ifdef DISABLED_UNTIL_GRL_IS_MADE_PART_OF_MDL
28942871
if (!(share= (TABLE_SHARE *) mdl_ticket->get_cached_object()))
28952872
#endif
@@ -2911,7 +2888,7 @@ bool open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
29112888
my_error(ER_WRONG_MRG_TABLE, MYF(0));
29122889
goto err_unlock;
29132890
}
2914-
2891+
29152892
/*
29162893
This table is a view. Validate its metadata version: in particular,
29172894
that it was a view when the statement was prepared.
@@ -2980,7 +2957,15 @@ bool open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
29802957
}
29812958
#endif
29822959

2983-
if (share->needs_reopen())
2960+
2961+
/*
2962+
If the version changes while we're opening the tables,
2963+
we have to back off, close all the tables opened-so-far,
2964+
and try to reopen them. Note: refresh_version is currently
2965+
changed only during FLUSH TABLES.
2966+
*/
2967+
if (share->needs_reopen() ||
2968+
(thd->open_tables && thd->open_tables->s->version != share->version))
29842969
{
29852970
if (!(flags & MYSQL_OPEN_IGNORE_FLUSH))
29862971
{
@@ -3000,8 +2985,6 @@ bool open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
30002985
NULL);
30012986
DBUG_RETURN(TRUE);
30022987
}
3003-
/* Force close at once after usage */
3004-
thd->version= share->version;
30052988
}
30062989

30072990
if (!share->free_tables.is_empty())
@@ -3017,9 +3000,11 @@ bool open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
30173000
while (table_cache_count > table_cache_size && unused_tables)
30183001
free_cache_entry(unused_tables);
30193002

3003+
mysql_mutex_unlock(&LOCK_open);
3004+
30203005
/* make a new table */
30213006
if (!(table=(TABLE*) my_malloc(sizeof(*table),MYF(MY_WME))))
3022-
goto err_unlock;
3007+
goto err_lock;
30233008

30243009
error= open_table_from_share(thd, share, alias,
30253010
(uint) (HA_OPEN_KEYFILE |
@@ -3041,16 +3026,17 @@ bool open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
30413026
(void) ot_ctx->request_backoff_action(Open_table_context::OT_REPAIR,
30423027
table_list);
30433028

3044-
goto err_unlock;
3029+
goto err_lock;
30453030
}
30463031

30473032
if (open_table_entry_fini(thd, share, table))
30483033
{
30493034
closefrm(table, 0);
30503035
my_free((uchar*)table, MYF(0));
3051-
goto err_unlock;
3036+
goto err_lock;
30523037
}
30533038

3039+
mysql_mutex_lock(&LOCK_open);
30543040
/* Add table to the share's used tables list. */
30553041
table_def_add_used_table(thd, table);
30563042
}
@@ -3112,6 +3098,8 @@ bool open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
31123098
table->file->extra(HA_EXTRA_DETACH_CHILDREN);
31133099
DBUG_RETURN(FALSE);
31143100

3101+
err_lock:
3102+
mysql_mutex_lock(&LOCK_open);
31153103
err_unlock:
31163104
release_table_share(share);
31173105
err_unlock2:

‎sql/sql_base.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,7 @@ TABLE *find_table_for_mdl_upgrade(TABLE *list, const char *db,
267267
const char *table_name,
268268
bool no_error);
269269
void mark_tmp_table_for_reuse(TABLE *table);
270+
bool check_if_table_exists(THD *thd, TABLE_LIST *table, bool *exists);
270271

271272
extern uint table_cache_count;
272273
extern TABLE *unused_tables;

‎sql/sql_class.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -592,7 +592,7 @@ THD::THD()
592592
*scramble= '\0';
593593

594594
/* Call to init() below requires fully initialized Open_tables_state. */
595-
init_open_tables_state(this, refresh_version);
595+
reset_open_tables_state(this);
596596

597597
init();
598598
#if defined(ENABLED_PROFILING)

‎sql/sql_class.h‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1006,7 +1006,6 @@ class Open_tables_state
10061006
of the main statement is called.
10071007
*/
10081008
enum enum_locked_tables_mode locked_tables_mode;
1009-
ulong version;
10101009
uint current_tablenr;
10111010

10121011
enum enum_flags {
@@ -1025,15 +1024,6 @@ class Open_tables_state
10251024
*/
10261025
Open_tables_state() : state_flags(0U) { }
10271026

1028-
/**
1029-
Prepare Open_tables_state instance for operations dealing with tables.
1030-
*/
1031-
void init_open_tables_state(THD *thd, ulong version_arg)
1032-
{
1033-
reset_open_tables_state(thd);
1034-
version= version_arg;
1035-
}
1036-
10371027
void set_open_tables_state(Open_tables_state *state)
10381028
{
10391029
*this= *state;

‎sql/sql_plugin.cc‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -261,11 +261,6 @@ static plugin_ref intern_plugin_lock(LEX *lex, plugin_ref plugin
261261
static void intern_plugin_unlock(LEX *lex, plugin_ref plugin);
262262
static void reap_plugins(void);
263263

264-
#ifdef EMBEDDED_LIBRARY
265-
/* declared in sql_base.cc */
266-
extern bool check_if_table_exists(THD *thd, TABLE_LIST *table, bool *exists);
267-
#endif /* EMBEDDED_LIBRARY */
268-
269264
static void report_error(int where_to, uint error, ...)
270265
{
271266
va_list args;
@@ -1475,10 +1470,8 @@ static void plugin_load(MEM_ROOT *tmp_root, int *argc, char **argv)
14751470
When building an embedded library, if the mysql.plugin table
14761471
does not exist, we silently ignore the missing table
14771472
*/
1478-
mysql_mutex_lock(&LOCK_open);
14791473
if (check_if_table_exists(new_thd, &tables, &table_exists))
14801474
table_exists= FALSE;
1481-
mysql_mutex_unlock(&LOCK_open);
14821475
if (!table_exists)
14831476
goto end;
14841477
#endif /* EMBEDDED_LIBRARY */
@@ -1519,7 +1512,7 @@ static void plugin_load(MEM_ROOT *tmp_root, int *argc, char **argv)
15191512
if (error > 0)
15201513
sql_print_error(ER(ER_GET_ERRNO), my_errno);
15211514
end_read_record(&read_record_info);
1522-
new_thd->version--; // Force close to free memory
1515+
table->m_needs_reopen= TRUE; // Force close to free memory
15231516
end:
15241517
close_thread_tables(new_thd);
15251518
/* Remember that we don't have a THD */

‎sql/sql_udf.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ void udf_init()
248248
if (error > 0)
249249
sql_print_error("Got unknown error: %d", my_errno);
250250
end_read_record(&read_record_info);
251-
new_thd->version--; // Force close to free memory
251+
table->m_needs_reopen= TRUE; // Force close to free memory
252252

253253
end:
254254
close_thread_tables(new_thd);

‎sql/table.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -590,7 +590,6 @@ struct TABLE_SHARE
590590
enum enum_ha_unused unused2;
591591

592592
uint ref_count; /* How many TABLE objects uses this */
593-
uint open_count; /* Number of tables in open list */
594593
uint blob_ptr_size; /* 4 or 8 */
595594
uint key_block_size; /* create key_block_size, if used */
596595
uint null_bytes, last_null_bit_pos;

0 commit comments

Comments
 (0)