From 779307dd5b5de8a6eea9d5ce5db574e5d6a2f70b Mon Sep 17 00:00:00 2001 From: Yuchen Pei Date: Thu, 27 Apr 2023 09:48:26 +1000 Subject: MDEV-29676 Some changes in behaviour w.r.t. spider sts crd - assign spider->share early when !new_share - remove locking before spider_share_init_{sts,crd} --- storage/spider/spd_table.cc | 64 ++++++++++++++++++++------------------------- 1 file changed, 29 insertions(+), 35 deletions(-) diff --git a/storage/spider/spd_table.cc b/storage/spider/spd_table.cc index 0a652b7473e..66042b36a66 100644 --- a/storage/spider/spd_table.cc +++ b/storage/spider/spd_table.cc @@ -5079,11 +5079,6 @@ bool spider_init_share( else first_byte = '0'; - /* fixme: do we need to assign spider->share at different places - depending on new_share? */ - if (!new_share) - spider->share = share; - if (!(spider->wide_handler->trx = spider_get_trx(thd, TRUE, error_num))) { spider_share_init_error_free(share, new_share, true); @@ -5091,23 +5086,17 @@ bool spider_init_share( } spider->set_error_mode(); - if (!share->sts_spider_init) - { - pthread_mutex_lock(&share->mutex); - if (!share->sts_spider_init && - (*error_num= spider_share_init_sts(table_name, spider, share, new_share))) + /* There's no other place doing anything that + `spider_share_init_sts()` does or updating + `st_spider_share::sts_spider_init`, therefore there's no need to + lock/unlock. Same goes for crd */ + if (!share->sts_spider_init && + (*error_num= spider_share_init_sts(table_name, spider, share, new_share))) DBUG_RETURN(TRUE); - pthread_mutex_unlock(&share->mutex); - } - if (!share->crd_spider_init) - { - pthread_mutex_lock(&share->mutex); - if (!share->crd_spider_init && - (*error_num= spider_share_init_crd(table_name, spider, share, new_share))) + if (!share->crd_spider_init && + (*error_num= spider_share_init_crd(table_name, spider, share, new_share))) DBUG_RETURN(TRUE); - pthread_mutex_unlock(&share->mutex); - } if (continue_with_sql_command && (*error_num = spider_create_mon_threads(spider->wide_handler->trx, @@ -5162,26 +5151,29 @@ bool spider_init_share( } spider->search_link_idx= search_link_idx; - if (new_share) - { - if (continue_with_sql_command && - spider_share_get_sts_crd(thd, spider, share, table, true, false, - error_num)) - goto error_after_alloc_dbton_handler; - } else if (share->init_error) + if (continue_with_sql_command) { - pthread_mutex_lock(&share->sts_mutex); - pthread_mutex_lock(&share->crd_mutex); - if (share->init_error) + if (new_share) { - if (continue_with_sql_command && - spider_share_get_sts_crd(thd, spider, share, table, FALSE, TRUE, - error_num)) + if (spider_share_get_sts_crd(thd, spider, share, table, true, false, + error_num)) goto error_after_alloc_dbton_handler; - share->init_error= FALSE; + } else if (share->init_error) + { + /* fixme: can we move the locking and unlocking into + spider_share_get_sts_crd()? */ + pthread_mutex_lock(&share->sts_mutex); + pthread_mutex_lock(&share->crd_mutex); + if (share->init_error) + { + if (spider_share_get_sts_crd(thd, spider, share, table, FALSE, TRUE, + error_num)) + goto error_after_alloc_dbton_handler; + share->init_error= FALSE; + } + pthread_mutex_unlock(&share->crd_mutex); + pthread_mutex_unlock(&share->sts_mutex); } - pthread_mutex_unlock(&share->crd_mutex); - pthread_mutex_unlock(&share->sts_mutex); } DBUG_RETURN(FALSE); @@ -5283,6 +5275,8 @@ SPIDER_SHARE *spider_get_share( my_sleep(10000); // wait 10 ms } + spider->share = share; + if (spider_init_share(table_name, table, thd, spider, error_num, share, table_share, FALSE)) DBUG_RETURN(NULL); -- cgit v1.2.1