From d8e9f3d981595034270d934da8e56c862a594d90 Mon Sep 17 00:00:00 2001 From: Yuchen Pei Date: Thu, 14 Sep 2023 15:50:31 +1000 Subject: [PATCH 1/8] MDEV-31673 MDEV-29502 Remove spider_db_handler::need_lock_before_set_sql_for_exec This function trivially returns false --- storage/spider/ha_spider.cc | 210 +++---------------- storage/spider/spd_conn.cc | 12 +- storage/spider/spd_db_conn.cc | 271 +++---------------------- storage/spider/spd_db_include.h | 3 - storage/spider/spd_db_mysql.cc | 8 - storage/spider/spd_db_mysql.h | 3 - storage/spider/spd_group_by_handler.cc | 20 +- 7 files changed, 56 insertions(+), 471 deletions(-) diff --git a/storage/spider/ha_spider.cc b/storage/spider/ha_spider.cc index dd23e4f1354..7c5e7b86200 100644 --- a/storage/spider/ha_spider.cc +++ b/storage/spider/ha_spider.cc @@ -2332,25 +2332,12 @@ int ha_spider::index_read_map_internal( #endif spider_db_handler *dbton_hdl = dbton_handler[conn->dbton_id]; pthread_mutex_assert_not_owner(&conn->mta_conn_mutex); - if (dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } if ((error_num = dbton_hdl->set_sql_for_exec(sql_type, roop_count))) { - if (dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - SPIDER_CLEAR_FILE_POS(&conn->mta_conn_mutex_file_pos); - pthread_mutex_unlock(&conn->mta_conn_mutex); - } DBUG_RETURN(error_num); } - if (!dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } + pthread_mutex_lock(&conn->mta_conn_mutex); + SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); DBUG_PRINT("info",("spider sql_type=%lu", sql_type)); #ifdef HA_CAN_BULK_ACCESS if ( @@ -2842,25 +2829,12 @@ int ha_spider::index_read_last_map_internal( #endif spider_db_handler *dbton_hdl = dbton_handler[conn->dbton_id]; pthread_mutex_assert_not_owner(&conn->mta_conn_mutex); - if (dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } if ((error_num = dbton_hdl->set_sql_for_exec(sql_type, roop_count))) { - if (dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - SPIDER_CLEAR_FILE_POS(&conn->mta_conn_mutex_file_pos); - pthread_mutex_unlock(&conn->mta_conn_mutex); - } DBUG_RETURN(error_num); } - if (!dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } + pthread_mutex_lock(&conn->mta_conn_mutex); + SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); DBUG_PRINT("info",("spider sql_type=%lu", sql_type)); #ifdef HA_CAN_BULK_ACCESS if (is_bulk_access_clone) @@ -3326,26 +3300,13 @@ int ha_spider::index_first_internal( #endif spider_db_handler *dbton_hdl = dbton_handler[conn->dbton_id]; pthread_mutex_assert_not_owner(&conn->mta_conn_mutex); - if (dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } if ((error_num = dbton_hdl->set_sql_for_exec(sql_type, roop_count))) { - if (dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - SPIDER_CLEAR_FILE_POS(&conn->mta_conn_mutex_file_pos); - pthread_mutex_unlock(&conn->mta_conn_mutex); - } DBUG_RETURN(error_num); } - if (!dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } + pthread_mutex_lock(&conn->mta_conn_mutex); + SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); DBUG_PRINT("info",("spider sql_type=%lu", sql_type)); #ifdef HA_CAN_BULK_ACCESS if (is_bulk_access_clone) @@ -3728,26 +3689,13 @@ int ha_spider::index_last_internal( #endif spider_db_handler *dbton_hdl = dbton_handler[conn->dbton_id]; pthread_mutex_assert_not_owner(&conn->mta_conn_mutex); - if (dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } if ((error_num = dbton_hdl->set_sql_for_exec(sql_type, roop_count))) { - if (dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - SPIDER_CLEAR_FILE_POS(&conn->mta_conn_mutex_file_pos); - pthread_mutex_unlock(&conn->mta_conn_mutex); - } DBUG_RETURN(error_num); } - if (!dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } + pthread_mutex_lock(&conn->mta_conn_mutex); + SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); DBUG_PRINT("info",("spider sql_type=%lu", sql_type)); #ifdef HA_CAN_BULK_ACCESS if (is_bulk_access_clone) @@ -4190,25 +4138,12 @@ int ha_spider::read_range_first_internal( #endif spider_db_handler *dbton_hdl = dbton_handler[conn->dbton_id]; pthread_mutex_assert_not_owner(&conn->mta_conn_mutex); - if (dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } if ((error_num = dbton_hdl->set_sql_for_exec(sql_type, roop_count))) { - if (dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - SPIDER_CLEAR_FILE_POS(&conn->mta_conn_mutex_file_pos); - pthread_mutex_unlock(&conn->mta_conn_mutex); - } DBUG_RETURN(error_num); } - if (!dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } + pthread_mutex_lock(&conn->mta_conn_mutex); + SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); DBUG_PRINT("info",("spider sql_type=%lu", sql_type)); #ifdef HA_CAN_BULK_ACCESS if (is_bulk_access_clone) @@ -4893,26 +4828,13 @@ int ha_spider::read_multi_range_first_internal( #endif spider_db_handler *dbton_hdl = dbton_handler[conn->dbton_id]; pthread_mutex_assert_not_owner(&conn->mta_conn_mutex); - if (dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } if ((error_num = dbton_hdl->set_sql_for_exec(sql_type, roop_count))) { - if (dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - SPIDER_CLEAR_FILE_POS(&conn->mta_conn_mutex_file_pos); - pthread_mutex_unlock(&conn->mta_conn_mutex); - } DBUG_RETURN(error_num); } - if (!dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } + pthread_mutex_lock(&conn->mta_conn_mutex); + SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); DBUG_PRINT("info",("spider sql_type=%lu", sql_type)); #ifdef HA_CAN_BULK_ACCESS if (is_bulk_access_clone) @@ -5717,26 +5639,13 @@ int ha_spider::read_multi_range_first_internal( #endif spider_db_handler *dbton_hdl = dbton_handler[conn->dbton_id]; pthread_mutex_assert_not_owner(&conn->mta_conn_mutex); - if (dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } if ((error_num = dbton_hdl->set_sql_for_exec(sql_type, roop_count))) { - if (dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - SPIDER_CLEAR_FILE_POS(&conn->mta_conn_mutex_file_pos); - pthread_mutex_unlock(&conn->mta_conn_mutex); - } DBUG_RETURN(error_num); } - if (!dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } + pthread_mutex_lock(&conn->mta_conn_mutex); + SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); sql_type &= ~SPIDER_SQL_TYPE_TMP_SQL; DBUG_PRINT("info",("spider sql_type=%lu", sql_type)); #ifdef HA_CAN_BULK_ACCESS @@ -6393,26 +6302,13 @@ int ha_spider::read_multi_range_next( #endif spider_db_handler *dbton_hdl = dbton_handler[conn->dbton_id]; pthread_mutex_assert_not_owner(&conn->mta_conn_mutex); - if (dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } if ((error_num = dbton_hdl->set_sql_for_exec(sql_type, roop_count))) { - if (dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - SPIDER_CLEAR_FILE_POS(&conn->mta_conn_mutex_file_pos); - pthread_mutex_unlock(&conn->mta_conn_mutex); - } DBUG_RETURN(error_num); } - if (!dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } + pthread_mutex_lock(&conn->mta_conn_mutex); + SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); DBUG_PRINT("info",("spider sql_type=%lu", sql_type)); #ifdef HA_CAN_BULK_ACCESS if (is_bulk_access_clone) @@ -7212,26 +7108,13 @@ int ha_spider::read_multi_range_next( #endif spider_db_handler *dbton_hdl = dbton_handler[conn->dbton_id]; pthread_mutex_assert_not_owner(&conn->mta_conn_mutex); - if (dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } if ((error_num = dbton_hdl->set_sql_for_exec(sql_type, roop_count))) { - if (dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - SPIDER_CLEAR_FILE_POS(&conn->mta_conn_mutex_file_pos); - pthread_mutex_unlock(&conn->mta_conn_mutex); - } DBUG_RETURN(error_num); } - if (!dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } + pthread_mutex_lock(&conn->mta_conn_mutex); + SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); sql_type &= ~SPIDER_SQL_TYPE_TMP_SQL; DBUG_PRINT("info",("spider sql_type=%lu", sql_type)); #ifdef HA_CAN_BULK_ACCESS @@ -7894,26 +7777,13 @@ int ha_spider::rnd_next_internal( } spider_db_handler *dbton_hdl = dbton_handler[conn->dbton_id]; pthread_mutex_assert_not_owner(&conn->mta_conn_mutex); - if (dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } if ((error_num = dbton_hdl->set_sql_for_exec(sql_type, roop_count))) { - if (dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - SPIDER_CLEAR_FILE_POS(&conn->mta_conn_mutex_file_pos); - pthread_mutex_unlock(&conn->mta_conn_mutex); - } DBUG_RETURN(error_num); } - if (!dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } + pthread_mutex_lock(&conn->mta_conn_mutex); + SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); DBUG_PRINT("info",("spider sql_type=%lu", sql_type)); conn->need_mon = &need_mons[roop_count]; DBUG_ASSERT(!conn->mta_conn_mutex_lock_already); @@ -8524,29 +8394,13 @@ int ha_spider::ft_read_internal( spider_db_handler *dbton_hdl = dbton_handler[dbton_id]; SPIDER_CONN *conn = conns[roop_count]; pthread_mutex_assert_not_owner(&conn->mta_conn_mutex); - if (dbton_hdl->need_lock_before_set_sql_for_exec( - SPIDER_SQL_TYPE_SELECT_SQL)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } if ((error_num = dbton_hdl->set_sql_for_exec( SPIDER_SQL_TYPE_SELECT_SQL, roop_count))) { - if (dbton_hdl->need_lock_before_set_sql_for_exec( - SPIDER_SQL_TYPE_SELECT_SQL)) - { - SPIDER_CLEAR_FILE_POS(&conn->mta_conn_mutex_file_pos); - pthread_mutex_unlock(&conn->mta_conn_mutex); - } DBUG_RETURN(error_num); } - if (!dbton_hdl->need_lock_before_set_sql_for_exec( - SPIDER_SQL_TYPE_SELECT_SQL)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } + pthread_mutex_lock(&conn->mta_conn_mutex); + SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); conn->need_mon = &need_mons[roop_count]; DBUG_ASSERT(!conn->mta_conn_mutex_lock_already); DBUG_ASSERT(!conn->mta_conn_mutex_unlock_later); @@ -13313,29 +13167,13 @@ int ha_spider::drop_tmp_tables() spider_db_handler *dbton_hdl = dbton_handler[dbton_id]; SPIDER_CONN *conn = conns[roop_count]; pthread_mutex_assert_not_owner(&conn->mta_conn_mutex); - if (dbton_hdl->need_lock_before_set_sql_for_exec( - SPIDER_SQL_TYPE_TMP_SQL)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } if ((error_num = dbton_hdl->set_sql_for_exec( SPIDER_SQL_TYPE_TMP_SQL, roop_count))) { - if (dbton_hdl->need_lock_before_set_sql_for_exec( - SPIDER_SQL_TYPE_TMP_SQL)) - { - SPIDER_CLEAR_FILE_POS(&conn->mta_conn_mutex_file_pos); - pthread_mutex_unlock(&conn->mta_conn_mutex); - } DBUG_RETURN(error_num); } - if (!dbton_hdl->need_lock_before_set_sql_for_exec( - SPIDER_SQL_TYPE_TMP_SQL)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } + pthread_mutex_lock(&conn->mta_conn_mutex); + SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); conn->need_mon = &need_mon; DBUG_ASSERT(!conn->mta_conn_mutex_lock_already); DBUG_ASSERT(!conn->mta_conn_mutex_unlock_later); diff --git a/storage/spider/spd_conn.cc b/storage/spider/spd_conn.cc index 1faa65d0db5..85a7ccf88e2 100644 --- a/storage/spider/spd_conn.cc +++ b/storage/spider/spd_conn.cc @@ -2636,11 +2636,6 @@ void *spider_bg_conn_action( } #endif pthread_mutex_assert_not_owner(&conn->mta_conn_mutex); - if (dbton_handler->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } if (spider->use_fields) { if ((error_num = dbton_handler->set_sql_for_exec(sql_type, @@ -2659,11 +2654,8 @@ void *spider_bg_conn_action( strmov(result_list->bgs_error_msg, spider_stmt_da_message(thd)); } } - if (!dbton_handler->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } + pthread_mutex_lock(&conn->mta_conn_mutex); + SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); sql_type &= ~SPIDER_SQL_TYPE_TMP_SQL; DBUG_PRINT("info",("spider sql_type=%lu", sql_type)); #ifdef HA_CAN_BULK_ACCESS diff --git a/storage/spider/spd_db_conn.cc b/storage/spider/spd_db_conn.cc index de85e0b99b2..c01dc21c1ce 100644 --- a/storage/spider/spd_db_conn.cc +++ b/storage/spider/spd_db_conn.cc @@ -4770,27 +4770,14 @@ int spider_db_seek_next( spider_db_handler *dbton_handler = spider->dbton_handler[conn->dbton_id]; pthread_mutex_assert_not_owner(&conn->mta_conn_mutex); - if (dbton_handler->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } if ((error_num = dbton_handler->set_sql_for_exec(sql_type, link_idx))) { - if (dbton_handler->need_lock_before_set_sql_for_exec(sql_type)) - { - SPIDER_CLEAR_FILE_POS(&conn->mta_conn_mutex_file_pos); - pthread_mutex_unlock(&conn->mta_conn_mutex); - } DBUG_PRINT("info",("spider error_num 6=%d", error_num)); DBUG_RETURN(error_num); } - if (!dbton_handler->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); + pthread_mutex_lock(&conn->mta_conn_mutex); SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } conn->need_mon = &spider->need_mons[link_idx]; DBUG_ASSERT(!conn->mta_conn_mutex_lock_already); DBUG_ASSERT(!conn->mta_conn_mutex_unlock_later); @@ -4878,27 +4865,14 @@ int spider_db_seek_next( spider_db_handler *dbton_handler = spider->dbton_handler[conn->dbton_id]; pthread_mutex_assert_not_owner(&conn->mta_conn_mutex); - if (dbton_handler->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } if ((error_num = dbton_handler->set_sql_for_exec(sql_type, roop_count))) { - if (dbton_handler->need_lock_before_set_sql_for_exec(sql_type)) - { - SPIDER_CLEAR_FILE_POS(&conn->mta_conn_mutex_file_pos); - pthread_mutex_unlock(&conn->mta_conn_mutex); - } DBUG_PRINT("info",("spider error_num 6=%d", error_num)); DBUG_RETURN(error_num); } - if (!dbton_handler->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); + pthread_mutex_lock(&conn->mta_conn_mutex); SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } conn->need_mon = &spider->need_mons[roop_count]; DBUG_ASSERT(!conn->mta_conn_mutex_lock_already); DBUG_ASSERT(!conn->mta_conn_mutex_unlock_later); @@ -5149,25 +5123,12 @@ int spider_db_seek_last( conn = spider->conns[roop_count]; spider_db_handler *dbton_handler = spider->dbton_handler[conn->dbton_id]; pthread_mutex_assert_not_owner(&conn->mta_conn_mutex); - if (dbton_handler->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } if ((error_num = dbton_handler->set_sql_for_exec(sql_type, roop_count))) { - if (dbton_handler->need_lock_before_set_sql_for_exec(sql_type)) - { - SPIDER_CLEAR_FILE_POS(&conn->mta_conn_mutex_file_pos); - pthread_mutex_unlock(&conn->mta_conn_mutex); - } DBUG_RETURN(error_num); } - if (!dbton_handler->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } + pthread_mutex_lock(&conn->mta_conn_mutex); + SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); DBUG_PRINT("info",("spider sql_type=%lu", sql_type)); conn->need_mon = &spider->need_mons[roop_count]; DBUG_ASSERT(!conn->mta_conn_mutex_lock_already); @@ -5366,25 +5327,12 @@ int spider_db_seek_last( conn = spider->conns[roop_count]; spider_db_handler *dbton_handler = spider->dbton_handler[conn->dbton_id]; pthread_mutex_assert_not_owner(&conn->mta_conn_mutex); - if (dbton_handler->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } if ((error_num = dbton_handler->set_sql_for_exec(sql_type, roop_count))) { - if (dbton_handler->need_lock_before_set_sql_for_exec(sql_type)) - { - SPIDER_CLEAR_FILE_POS(&conn->mta_conn_mutex_file_pos); - pthread_mutex_unlock(&conn->mta_conn_mutex); - } DBUG_RETURN(error_num); } - if (!dbton_handler->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } + pthread_mutex_lock(&conn->mta_conn_mutex); + SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); DBUG_PRINT("info",("spider sql_type=%lu", sql_type)); conn->need_mon = &spider->need_mons[roop_count]; DBUG_ASSERT(!conn->mta_conn_mutex_lock_already); @@ -6227,26 +6175,11 @@ int spider_db_bulk_insert( conn = spider->conns[roop_count2]; dbton_handler = spider->dbton_handler[conn->dbton_id]; pthread_mutex_assert_not_owner(&conn->mta_conn_mutex); - if (dbton_handler->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } if ((error_num = dbton_handler->set_sql_for_exec(sql_type, roop_count2))) - { - if (dbton_handler->need_lock_before_set_sql_for_exec(sql_type)) - { - SPIDER_CLEAR_FILE_POS(&conn->mta_conn_mutex_file_pos); - pthread_mutex_unlock(&conn->mta_conn_mutex); - } DBUG_RETURN(error_num); - } - if (!dbton_handler->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } + pthread_mutex_lock(&conn->mta_conn_mutex); + SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); #if defined(HS_HAS_SQLCOM) && defined(HAVE_HANDLERSOCKET) } else { sql_type = SPIDER_SQL_TYPE_INSERT_HS; @@ -6718,29 +6651,13 @@ int spider_db_bulk_update_size_limit( conn = spider->conns[roop_count]; spider_db_handler *dbton_hdl = spider->dbton_handler[conn->dbton_id]; pthread_mutex_assert_not_owner(&conn->mta_conn_mutex); - if (dbton_hdl->need_lock_before_set_sql_for_exec( - SPIDER_SQL_TYPE_BULK_UPDATE_SQL)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } if ((error_num = dbton_hdl->set_sql_for_exec( SPIDER_SQL_TYPE_BULK_UPDATE_SQL, roop_count))) { - if (dbton_hdl->need_lock_before_set_sql_for_exec( - SPIDER_SQL_TYPE_BULK_UPDATE_SQL)) - { - SPIDER_CLEAR_FILE_POS(&conn->mta_conn_mutex_file_pos); - pthread_mutex_unlock(&conn->mta_conn_mutex); - } DBUG_RETURN(error_num); } - if (!dbton_hdl->need_lock_before_set_sql_for_exec( - SPIDER_SQL_TYPE_BULK_UPDATE_SQL)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } + pthread_mutex_lock(&conn->mta_conn_mutex); + SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); if ((error_num = spider_db_query_for_bulk_update( spider, conn, roop_count, &dup_key_found))) { @@ -6814,33 +6731,17 @@ int spider_db_bulk_update_end( conn = spider->conns[roop_count]; spider_db_handler *dbton_hdl = spider->dbton_handler[conn->dbton_id]; pthread_mutex_assert_not_owner(&conn->mta_conn_mutex); - if (dbton_hdl->need_lock_before_set_sql_for_exec( - SPIDER_SQL_TYPE_BULK_UPDATE_SQL)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } if ((error_num = dbton_hdl->set_sql_for_exec( SPIDER_SQL_TYPE_BULK_UPDATE_SQL, roop_count))) { - if (dbton_hdl->need_lock_before_set_sql_for_exec( - SPIDER_SQL_TYPE_BULK_UPDATE_SQL)) - { - SPIDER_CLEAR_FILE_POS(&conn->mta_conn_mutex_file_pos); - pthread_mutex_unlock(&conn->mta_conn_mutex); - } if (error_num == ER_SPIDER_COND_SKIP_NUM) { continue; } DBUG_RETURN(error_num); } - if (!dbton_hdl->need_lock_before_set_sql_for_exec( - SPIDER_SQL_TYPE_BULK_UPDATE_SQL)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } + pthread_mutex_lock(&conn->mta_conn_mutex); + SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); if ((error_num = spider_db_query_for_bulk_update( spider, conn, roop_count, dup_key_found))) { @@ -6873,29 +6774,13 @@ int spider_db_bulk_update_end( conn = spider->conns[roop_count]; spider_db_handler *dbton_hdl = spider->dbton_handler[conn->dbton_id]; pthread_mutex_assert_not_owner(&conn->mta_conn_mutex); - if (dbton_hdl->need_lock_before_set_sql_for_exec( - SPIDER_SQL_TYPE_BULK_UPDATE_SQL)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } if ((error_num = dbton_hdl->set_sql_for_exec( SPIDER_SQL_TYPE_BULK_UPDATE_SQL, roop_count))) { - if (dbton_hdl->need_lock_before_set_sql_for_exec( - SPIDER_SQL_TYPE_BULK_UPDATE_SQL)) - { - SPIDER_CLEAR_FILE_POS(&conn->mta_conn_mutex_file_pos); - pthread_mutex_unlock(&conn->mta_conn_mutex); - } DBUG_RETURN(error_num); } - if (!dbton_hdl->need_lock_before_set_sql_for_exec( - SPIDER_SQL_TYPE_BULK_UPDATE_SQL)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } + pthread_mutex_lock(&conn->mta_conn_mutex); + SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); if ((error_num = spider_db_query_for_bulk_update( spider, conn, roop_count, dup_key_found))) { @@ -6971,29 +6856,13 @@ int spider_db_update( conn->ignore_dup_key = spider->ignore_dup_key; #endif pthread_mutex_assert_not_owner(&conn->mta_conn_mutex); - if (dbton_hdl->need_lock_before_set_sql_for_exec( - SPIDER_SQL_TYPE_UPDATE_SQL)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } if ((error_num = dbton_hdl->set_sql_for_exec( SPIDER_SQL_TYPE_UPDATE_SQL, roop_count))) { - if (dbton_hdl->need_lock_before_set_sql_for_exec( - SPIDER_SQL_TYPE_UPDATE_SQL)) - { - SPIDER_CLEAR_FILE_POS(&conn->mta_conn_mutex_file_pos); - pthread_mutex_unlock(&conn->mta_conn_mutex); - } DBUG_RETURN(error_num); } - if (!dbton_hdl->need_lock_before_set_sql_for_exec( - SPIDER_SQL_TYPE_UPDATE_SQL)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } + pthread_mutex_lock(&conn->mta_conn_mutex); + SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); conn->need_mon = &spider->need_mons[roop_count]; DBUG_ASSERT(!conn->mta_conn_mutex_lock_already); DBUG_ASSERT(!conn->mta_conn_mutex_unlock_later); @@ -7320,25 +7189,12 @@ int spider_db_direct_update( #endif spider_db_handler *dbton_hdl = spider->dbton_handler[conn->dbton_id]; pthread_mutex_assert_not_owner(&conn->mta_conn_mutex); - if (dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } if ((error_num = dbton_hdl->set_sql_for_exec(sql_type, roop_count))) { - if (dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - SPIDER_CLEAR_FILE_POS(&conn->mta_conn_mutex_file_pos); - pthread_mutex_unlock(&conn->mta_conn_mutex); - } DBUG_RETURN(error_num); } - if (!dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } + pthread_mutex_lock(&conn->mta_conn_mutex); + SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); #ifdef HA_CAN_BULK_ACCESS if (spider->is_bulk_access_clone) { @@ -7596,25 +7452,12 @@ int spider_db_direct_update( sql_type = SPIDER_SQL_TYPE_UPDATE_SQL; spider_db_handler *dbton_hdl = spider->dbton_handler[conn->dbton_id]; pthread_mutex_assert_not_owner(&conn->mta_conn_mutex); - if (dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } if ((error_num = dbton_hdl->set_sql_for_exec(sql_type, roop_count))) { - if (dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - SPIDER_CLEAR_FILE_POS(&conn->mta_conn_mutex_file_pos); - pthread_mutex_unlock(&conn->mta_conn_mutex); - } DBUG_RETURN(error_num); } - if (!dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } + pthread_mutex_lock(&conn->mta_conn_mutex); + SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); #ifdef HA_CAN_BULK_ACCESS if (spider->is_bulk_access_clone) { @@ -7879,29 +7722,13 @@ int spider_db_delete( conn = spider->conns[roop_count]; spider_db_handler *dbton_hdl = spider->dbton_handler[conn->dbton_id]; pthread_mutex_assert_not_owner(&conn->mta_conn_mutex); - if (dbton_hdl->need_lock_before_set_sql_for_exec( - SPIDER_SQL_TYPE_DELETE_SQL)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } if ((error_num = dbton_hdl->set_sql_for_exec( SPIDER_SQL_TYPE_DELETE_SQL, roop_count))) { - if (dbton_hdl->need_lock_before_set_sql_for_exec( - SPIDER_SQL_TYPE_DELETE_SQL)) - { - SPIDER_CLEAR_FILE_POS(&conn->mta_conn_mutex_file_pos); - pthread_mutex_unlock(&conn->mta_conn_mutex); - } DBUG_RETURN(error_num); } - if (!dbton_hdl->need_lock_before_set_sql_for_exec( - SPIDER_SQL_TYPE_DELETE_SQL)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } + pthread_mutex_lock(&conn->mta_conn_mutex); + SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); DBUG_ASSERT(!conn->mta_conn_mutex_lock_already); DBUG_ASSERT(!conn->mta_conn_mutex_unlock_later); conn->mta_conn_mutex_lock_already = TRUE; @@ -8036,25 +7863,12 @@ int spider_db_direct_delete( #endif spider_db_handler *dbton_hdl = spider->dbton_handler[conn->dbton_id]; pthread_mutex_assert_not_owner(&conn->mta_conn_mutex); - if (dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } if ((error_num = dbton_hdl->set_sql_for_exec(sql_type, roop_count))) { - if (dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - SPIDER_CLEAR_FILE_POS(&conn->mta_conn_mutex_file_pos); - pthread_mutex_unlock(&conn->mta_conn_mutex); - } DBUG_RETURN(error_num); } - if (!dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } + pthread_mutex_lock(&conn->mta_conn_mutex); + SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); #ifdef HA_CAN_BULK_ACCESS if (spider->is_bulk_access_clone) { @@ -8272,25 +8086,12 @@ int spider_db_direct_delete( sql_type = SPIDER_SQL_TYPE_DELETE_SQL; spider_db_handler *dbton_hdl = spider->dbton_handler[conn->dbton_id]; pthread_mutex_assert_not_owner(&conn->mta_conn_mutex); - if (dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } if ((error_num = dbton_hdl->set_sql_for_exec(sql_type, roop_count))) { - if (dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - SPIDER_CLEAR_FILE_POS(&conn->mta_conn_mutex_file_pos); - pthread_mutex_unlock(&conn->mta_conn_mutex); - } DBUG_RETURN(error_num); } - if (!dbton_hdl->need_lock_before_set_sql_for_exec(sql_type)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } + pthread_mutex_lock(&conn->mta_conn_mutex); + SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); #ifdef HA_CAN_BULK_ACCESS if (spider->is_bulk_access_clone) { @@ -8421,29 +8222,13 @@ int spider_db_delete_all_rows( spider_db_handler *dbton_hdl = spider->dbton_handler[dbton_id]; conn = spider->conns[roop_count]; pthread_mutex_assert_not_owner(&conn->mta_conn_mutex); - if (dbton_hdl->need_lock_before_set_sql_for_exec( - SPIDER_SQL_TYPE_DELETE_SQL)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } if ((error_num = dbton_hdl->set_sql_for_exec( SPIDER_SQL_TYPE_DELETE_SQL, roop_count))) { - if (dbton_hdl->need_lock_before_set_sql_for_exec( - SPIDER_SQL_TYPE_DELETE_SQL)) - { - SPIDER_CLEAR_FILE_POS(&conn->mta_conn_mutex_file_pos); - pthread_mutex_unlock(&conn->mta_conn_mutex); - } DBUG_RETURN(error_num); } - if (!dbton_hdl->need_lock_before_set_sql_for_exec( - SPIDER_SQL_TYPE_DELETE_SQL)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } + pthread_mutex_lock(&conn->mta_conn_mutex); + SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); conn->need_mon = &spider->need_mons[roop_count]; DBUG_ASSERT(!conn->mta_conn_mutex_lock_already); DBUG_ASSERT(!conn->mta_conn_mutex_unlock_later); diff --git a/storage/spider/spd_db_include.h b/storage/spider/spd_db_include.h index eaddddc6d72..307c6638c3a 100644 --- a/storage/spider/spd_db_include.h +++ b/storage/spider/spd_db_include.h @@ -1633,9 +1633,6 @@ public: SPIDER_HS_STRING_REF &info ) = 0; #endif - virtual bool need_lock_before_set_sql_for_exec( - ulong sql_type - ) = 0; #ifdef SPIDER_HAS_GROUP_BY_HANDLER virtual int set_sql_for_exec( ulong sql_type, diff --git a/storage/spider/spd_db_mysql.cc b/storage/spider/spd_db_mysql.cc index e2f31b537e9..eb85c80e525 100644 --- a/storage/spider/spd_db_mysql.cc +++ b/storage/spider/spd_db_mysql.cc @@ -13538,14 +13538,6 @@ int spider_mbase_handler::push_back_upds( } #endif -bool spider_mbase_handler::need_lock_before_set_sql_for_exec( - ulong sql_type -) { - DBUG_ENTER("spider_mbase_handler::need_lock_before_set_sql_for_exec"); - DBUG_PRINT("info",("spider this=%p", this)); - DBUG_RETURN(FALSE); -} - #ifdef SPIDER_HAS_GROUP_BY_HANDLER int spider_mbase_handler::set_sql_for_exec( ulong sql_type, diff --git a/storage/spider/spd_db_mysql.h b/storage/spider/spd_db_mysql.h index c866376095a..7305aec5d82 100644 --- a/storage/spider/spd_db_mysql.h +++ b/storage/spider/spd_db_mysql.h @@ -1476,9 +1476,6 @@ public: SPIDER_HS_STRING_REF &info ); #endif - bool need_lock_before_set_sql_for_exec( - ulong sql_type - ); #ifdef SPIDER_HAS_GROUP_BY_HANDLER int set_sql_for_exec( ulong sql_type, diff --git a/storage/spider/spd_group_by_handler.cc b/storage/spider/spd_group_by_handler.cc index 635bff3d415..5a0c30bd790 100644 --- a/storage/spider/spd_group_by_handler.cc +++ b/storage/spider/spd_group_by_handler.cc @@ -1411,30 +1411,14 @@ int spider_group_by_handler::init_scan() } else { #endif pthread_mutex_assert_not_owner(&conn->mta_conn_mutex); - if (dbton_hdl->need_lock_before_set_sql_for_exec( - SPIDER_SQL_TYPE_SELECT_SQL)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } if ((error_num = dbton_hdl->set_sql_for_exec(SPIDER_SQL_TYPE_SELECT_SQL, link_idx, link_idx_chain))) { - if (dbton_hdl->need_lock_before_set_sql_for_exec( - SPIDER_SQL_TYPE_SELECT_SQL)) - { - SPIDER_CLEAR_FILE_POS(&conn->mta_conn_mutex_file_pos); - pthread_mutex_unlock(&conn->mta_conn_mutex); - } DBUG_RETURN(error_num); } - if (!dbton_hdl->need_lock_before_set_sql_for_exec( - SPIDER_SQL_TYPE_SELECT_SQL)) - { - pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); - } + pthread_mutex_lock(&conn->mta_conn_mutex); + SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); conn->need_mon = &spider->need_mons[link_idx]; DBUG_ASSERT(!conn->mta_conn_mutex_lock_already); DBUG_ASSERT(!conn->mta_conn_mutex_unlock_later); From b1ab4ec4e25b231da77a2683d69366ab5dea4d66 Mon Sep 17 00:00:00 2001 From: Anel Husakovic Date: Thu, 24 Aug 2023 15:37:46 +0200 Subject: [PATCH 2/8] Remove duplicated default client include from replication my.cnf - `default_client` is included already in rpl_1slave_base.cnf`, so remove it from `my.cnf` - Remove option group for `mysqld` server as and add comment how to override specific settings for specific server - Reviewer: --- mysql-test/suite/rpl/my.cnf | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/mysql-test/suite/rpl/my.cnf b/mysql-test/suite/rpl/my.cnf index 74233b056c7..d61e32ed51a 100644 --- a/mysql-test/suite/rpl/my.cnf +++ b/mysql-test/suite/rpl/my.cnf @@ -1,8 +1,8 @@ # Use settings from rpl_1slave_base.cnf # add setting to connect the slave to the master by default !include rpl_1slave_base.cnf -!include include/default_client.cnf - - -[mysqld.2] - +# Override specific server settings using [mariadb-x.y] option group +# from `test.cnf` file right after including this file. +# E.g. after !include ../my.cnf, in your `test.cnf`, specify your configuration +# in option group e.g [mysqld.x], so that number `x` corresponds to the number +# in the rpl server topology. From 2534e5bc0bc70e1121532287acc0fb389f51e5ec Mon Sep 17 00:00:00 2001 From: Anel Husakovic Date: Thu, 24 Aug 2023 13:33:49 +0200 Subject: [PATCH 3/8] MDEV-32004: Parse error in mtr tests when using rpl_check_server_ids parameter - Fix the calling of the assertion condition when `rpl_check_server_ids` parameter is used. - Fix comments regarding the default usage and configuration files extension in this case. - Reviewer: --- mysql-test/include/rpl_init.inc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/mysql-test/include/rpl_init.inc b/mysql-test/include/rpl_init.inc index 4ee4cccdc20..dfd5e668e1d 100644 --- a/mysql-test/include/rpl_init.inc +++ b/mysql-test/include/rpl_init.inc @@ -7,14 +7,15 @@ # well as extra connections server_1_1, server_2_1, ..., # server_N_1. server_I and server_I_1 are connections to the same # server. -# - Verifies that @@server_id of all servers are different. # - Calls RESET MASTER, RESET SLAVE, USE test, CHANGE MASTER, START SLAVE. # - Sets the connection to server_1 before exiting. +# With $rpl_check_server_ids parameter, the script does the following: +# - Verifies that @@server_id of all servers are different. # # ==== Usage ==== # # 1. If you are going to use more than two servers, create -# rpl_test.cfg with the following contents: +# rpl_test.cnf with the following contents: # # !include ../my.cnf # [mysqld.1] @@ -189,7 +190,7 @@ if ($rpl_check_server_ids) while ($_rpl_server2) { --let $assert_text= Servers $_rpl_server and $_rpl_server2 should have different @@server_id - --let $assert_condition= [$_rpl_server:SELECT @@server_id AS i, i, 1] != [$_rpl_server2:SELECT @@server_id AS i, i, 1] + --let $assert_cond= [SELECT @@server_id AS i, i, 1] != $_rpl_server --source include/assert.inc --dec $_rpl_server2 From 8d6ae0f2f9f69ee14ed9526a0ab5de79fdb22336 Mon Sep 17 00:00:00 2001 From: Anel Husakovic Date: Fri, 25 Aug 2023 09:37:16 +0200 Subject: [PATCH 4/8] MDEV-32004: Remove extra `server__1` connections during initialization - Remove extra connections in the form of `server_number_1` for the same server during initialization of servers in the `rpl_init.inc` file. - Remove disconnecting and reconnecting to the same connections, since they are not used by the test. - Update comments about the above. - Reviewer: --- mysql-test/include/rpl_end.inc | 2 -- mysql-test/include/rpl_init.inc | 10 ++-------- mysql-test/include/rpl_reconnect.inc | 11 ----------- 3 files changed, 2 insertions(+), 21 deletions(-) diff --git a/mysql-test/include/rpl_end.inc b/mysql-test/include/rpl_end.inc index 3fdd91ba319..4719ef16ccf 100644 --- a/mysql-test/include/rpl_end.inc +++ b/mysql-test/include/rpl_end.inc @@ -103,11 +103,9 @@ while ($_rpl_server) --connection default --let $_rpl_server= $rpl_server_count ---let $_rpl_one= _1 while ($_rpl_server) { --disconnect server_$_rpl_server - --disconnect server_$_rpl_server$_rpl_one --dec $_rpl_server } diff --git a/mysql-test/include/rpl_init.inc b/mysql-test/include/rpl_init.inc index dfd5e668e1d..bfb1fc3f60b 100644 --- a/mysql-test/include/rpl_init.inc +++ b/mysql-test/include/rpl_init.inc @@ -3,10 +3,7 @@ # Set up replication on several servers in a specified topology. # # By default, this script does the following: -# - Creates the connections server_1, server_2, ..., server_N, as -# well as extra connections server_1_1, server_2_1, ..., -# server_N_1. server_I and server_I_1 are connections to the same -# server. +# - Creates the connections server_1, server_2, ..., server_N. # - Calls RESET MASTER, RESET SLAVE, USE test, CHANGE MASTER, START SLAVE. # - Sets the connection to server_1 before exiting. # With $rpl_check_server_ids parameter, the script does the following: @@ -143,18 +140,15 @@ if (!$rpl_debug) } -# Create two connections to each server; reset master/slave, select +# Create connection to the server; reset master/slave, select # database, set autoinc variables. --let $_rpl_server= $rpl_server_count ---let $_rpl_one= _1 while ($_rpl_server) { # Connect. --let $rpl_server_number= $_rpl_server --let $rpl_connection_name= server_$_rpl_server --source include/rpl_connect.inc - --let $rpl_connection_name= server_$_rpl_server$_rpl_one - --source include/rpl_connect.inc # Configure server. --let $rpl_connection_name= server_$_rpl_server diff --git a/mysql-test/include/rpl_reconnect.inc b/mysql-test/include/rpl_reconnect.inc index cdbbd0a1bf1..9438c774461 100644 --- a/mysql-test/include/rpl_reconnect.inc +++ b/mysql-test/include/rpl_reconnect.inc @@ -72,11 +72,6 @@ if (!$_rpl_server_number) --source include/rpl_connection.inc --enable_reconnect ---let $_rpl_one= _1 ---let $rpl_connection_name= server_$rpl_server_number$_rpl_one ---source include/rpl_connection.inc ---enable_reconnect - if ($rpl_debug) { --echo ---- Wait for reconnect and disable reconnect on all connections ---- @@ -122,11 +117,5 @@ if (!$_rpl_server_number) --source include/wait_until_connected_again.inc --disable_reconnect ---let $rpl_connection_name= server_$rpl_server_number$_rpl_one ---source include/rpl_connection.inc ---source include/wait_until_connected_again.inc ---disable_reconnect - - --let $include_filename= rpl_reconnect.inc --source include/end_include_file.inc From 15cd8542cfe23eed18894c9875dd90ba802f591e Mon Sep 17 00:00:00 2001 From: Anel Husakovic Date: Fri, 25 Aug 2023 16:58:08 +0200 Subject: [PATCH 5/8] MDEV-32004: Cosmetic fixes - Reviewer: --- mysql-test/include/rpl_change_topology.inc | 34 ++++++++++++------- mysql-test/include/rpl_connect.inc | 2 +- mysql-test/include/rpl_for_each_slave.inc | 16 ++++++++- mysql-test/include/rpl_init.inc | 23 ++++++++++--- mysql-test/include/rpl_start_slaves.inc | 2 +- .../include/wait_for_slave_io_to_start.inc | 9 ++--- .../include/wait_for_slave_sql_to_start.inc | 3 +- .../include/wait_for_slave_to_start.inc | 1 + 8 files changed, 64 insertions(+), 26 deletions(-) diff --git a/mysql-test/include/rpl_change_topology.inc b/mysql-test/include/rpl_change_topology.inc index 799262986e6..1ee6e711372 100644 --- a/mysql-test/include/rpl_change_topology.inc +++ b/mysql-test/include/rpl_change_topology.inc @@ -5,15 +5,15 @@ # need to change topology after they have sourced include/rpl_init.inc # # This file sets up variables needed by include/rpl_sync.inc and many -# other replication scripts in the include/ directory. It also issues +# other replication scripts in the include/ directory. It also issues # CHANGE MASTER on all servers where the configuration changes from -# what it was before. It does not issue START SLAVE (use +# what it was before. It does not issue START SLAVE (use # include/rpl_start_slaves.inc for that). # # Note: it is not currently possible to change the number of servers # after the rpl_init.inc, without first calling rpl_end.inc. So the # test has to set $rpl_server_count to the total number of servers -# that the test uses, before it sources include/rpl_init.inc. After +# that the test uses, before it sources include/rpl_init.inc. After # that, $rpl_server_count must not change until after next time the # test sources include/rpl_end.inc. # @@ -37,7 +37,7 @@ # By default, CHANGE MASTER is executed with MASTER_LOG_FILE set # to the name of the last binlog file on the master (retrieved by # executing SHOW MASTER STATUS). This variable can be set to -# specify another filename. This variable should be a +# specify another filename. This variable should be a # comma-separated list of the following form: # # SERVER_NUMBER_1:FILE_NAME_1,SERVER_NUMBER_2:FILE_NAME_2,... @@ -45,7 +45,7 @@ # Before CHANGE MASTER is executed on server N, this script checks # if $rpl_master_log_file contains the text N:FILE_NAME. If it # does, then MASTER_LOG_FILE is set to FILE_NAME. Otherwise, -# MASTER_LOG_FILE is set to the last binlog on the master. For +# MASTER_LOG_FILE is set to the last binlog on the master. For # example, to specify that server_1 should start replicate from # master-bin.000007 and server_5 should start replicate from # master-bin.012345, do: @@ -53,9 +53,9 @@ # # $rpl_master_log_pos # By default, CHANGE MASTER is executed without specifying the -# MASTER_LOG_POS parameter. This variable can be set to set a -# specific position. It has the same form as $rpl_master_log_file -# (see above). For example, to specify that server_3 should start +# MASTER_LOG_POS parameter. This variable can be set to set a +# specific position. It has the same form as $rpl_master_log_file +# (see above). For example, to specify that server_3 should start # replicate from position 4711 of its master, do: # --let $rpl_master_log_pos= 3:4711 # @@ -72,7 +72,7 @@ # include/rpl_stop_slaves.inc # include/rpl_end.inc # -# $rpl_server_count_length: +# $rpl_server_count_length # Set to LENGTH($rpl_server_count). So if $rpl_server_count < 10, # then $rpl_server_count_length = 1; if 10 <= $rpl_server_count < # 100, then $rpl_server_count_length = 2, etc. @@ -83,12 +83,12 @@ # server N is a slave, then the N'th number is the master of server # N. If server N is not a slave, then the N'th number is just spaces # (so in fact it is not a number). For example, if $rpl_topology is -# '1->2,2->3,3->1,2->4,5->6', then $rpl_master_list is '3122 6'. +# '1->2,2->3,3->1,2->4,5->6', then $rpl_master_list is '3122 5'. # # $rpl_sync_chain_dirty -# This variable is set to 1. This tells include/rpl_sync.inc to +# This variable is set to 1. This tells include/rpl_sync.inc to # compute a new value for $rpl_sync_chain next time that -# include/rpl_sync.inc is sourced. See +# include/rpl_sync.inc is sourced. See # include/rpl_generate_sync_chain.inc and include/rpl_sync.inc for # details. @@ -124,7 +124,7 @@ if ($rpl_master_list == '') if ($rpl_debug) { --echo \$rpl_server_count='$rpl_server_count' - --echo \$rpl_server_count_length='$rpl_server_count_length' + --echo old \$rpl_server_count_length='$rpl_server_count_length' --echo new \$rpl_topology='$_rpl_topology' --echo old \$rpl_master_list='$rpl_master_list' --echo old \$rpl_sync_chain='$rpl_sync_chain' @@ -210,6 +210,10 @@ if (!$rpl_skip_change_master) --let $rpl_connection_name= server_$_rpl_master --source include/rpl_connection.inc --let $_rpl_master_log_file= query_get_value(SHOW MASTER STATUS, File, 1) + if ($rpl_debug) + { + --echo "\$rpl_master_log_file parameter not set for the master: $_rpl_master, use the latest binlog file by executing SHOW MASTER STATUS." + } } # Change connection. --let $rpl_connection_name= server_$_rpl_server @@ -224,6 +228,10 @@ if (!$rpl_skip_change_master) if (!$_rpl_master_log_pos_index) { --let $_rpl_master_log_pos= + if ($rpl_debug) + { + --echo "\$rpl_master_log_pos parameter not set for the master: $_rpl_master. Set log position to empty." + } } eval CHANGE MASTER TO MASTER_HOST = '127.0.0.1', MASTER_PORT = $_rpl_port, MASTER_USER = 'root', MASTER_LOG_FILE = '$_rpl_master_log_file'$_rpl_master_log_pos, MASTER_CONNECT_RETRY = 1; } diff --git a/mysql-test/include/rpl_connect.inc b/mysql-test/include/rpl_connect.inc index e30769eb335..a962ba6f601 100644 --- a/mysql-test/include/rpl_connect.inc +++ b/mysql-test/include/rpl_connect.inc @@ -5,7 +5,7 @@ # This script is normally used internally by rpl_init.inc and # master-slave.inc, but it can also be used in test cases that need to # create more connections or re-create connections after disconnect. -# +# Default ports SERVER_MYPORT_[1,2] are set by rpl_init.inc. # # ==== Usage ==== # diff --git a/mysql-test/include/rpl_for_each_slave.inc b/mysql-test/include/rpl_for_each_slave.inc index 65d242cf894..c1d4581dfa5 100644 --- a/mysql-test/include/rpl_for_each_slave.inc +++ b/mysql-test/include/rpl_for_each_slave.inc @@ -1,7 +1,7 @@ # ==== Purpose ==== # # Execute a .inc file once for each server that was configured as a -# slave by rpl_init.inc +# slave by rpl_init.inc, for example start_slave.inc or stop_slave.inc file. # # # ==== Usage ==== @@ -14,6 +14,20 @@ # $rpl_source_file # The file that will be sourced. # +# $rpl_server_count +# The number of servers to configure. If this is not set, the largest +# number in $rpl_topology will be used. +# This parameter is obtained from rpl_init.inc. +# +# $rpl_master_list +# This parameter is calculated from within rpl_init.inc. +# +# $rpl_server_count_length +# Set to LENGTH($rpl_server_count). So if $rpl_server_count < 10, +# then $rpl_server_count_length = 1; if 10 <= $rpl_server_count < +# 100, then $rpl_server_count_length = 2, etc. +# This parameter is calculated from within rpl_change_topology.inc. +# # $rpl_debug # See include/rpl_init.inc diff --git a/mysql-test/include/rpl_init.inc b/mysql-test/include/rpl_init.inc index bfb1fc3f60b..fd13d13976b 100644 --- a/mysql-test/include/rpl_init.inc +++ b/mysql-test/include/rpl_init.inc @@ -32,8 +32,9 @@ # # (It is allowed, but not required, to configure SERVER_MYPORT_1 # and SERVER_MYPORT_2 too. If these variables are not set, the -# variables MASTER_MYPORT and SLAVE_MYPORT, configured in the -# default my.cnf used by the rpl suite, are used instead.) +# variables MASTER_MYPORT and SLAVE_MYPORT are used instead. +# These variables are configured in the rpl_1slave_base.cnf, +# that is used in the default my.cnf, which is used by the rpl suite.) # # 2. Execute the following near the top of the test: # @@ -193,18 +194,30 @@ if ($rpl_check_server_ids) } } -# $rpl_master_list must be set so that include/rpl_change_topology.inc -# knows which servers are initialized and not. +if ($rpl_debug) +{ + --echo ---- Check the topology and call CHANGE MASTER ---- +} + +# $rpl_master_list must be set so that include/rpl_change_topology.inc and later +# include/rpl_for_each_slave.inc knows which servers are initialized and not. --let $rpl_master_list= `SELECT REPEAT('x', $rpl_server_count * LENGTH($rpl_server_count))` --source include/rpl_change_topology.inc if (!$rpl_skip_start_slave) { + if ($rpl_debug) + { + --echo ---- Start slaves ---- + } --source include/rpl_start_slaves.inc } - +if ($rpl_debug) +{ + --echo ---- Set connection to the server_1 ---- +} --let $rpl_connection_name= server_1 --source include/rpl_connection.inc diff --git a/mysql-test/include/rpl_start_slaves.inc b/mysql-test/include/rpl_start_slaves.inc index fdd90eb12c5..9c1f9f7b293 100644 --- a/mysql-test/include/rpl_start_slaves.inc +++ b/mysql-test/include/rpl_start_slaves.inc @@ -19,7 +19,7 @@ # # $slave_timeout # Set the timeout when waiting for slave threads to stop and -# start, respectively. See include/wait_for_slave_param.inc +# start, respectively. See include/wait_for_slave_param.inc --let $include_filename= rpl_start_slaves.inc diff --git a/mysql-test/include/wait_for_slave_io_to_start.inc b/mysql-test/include/wait_for_slave_io_to_start.inc index cd8e5d374a4..0cb4091755c 100644 --- a/mysql-test/include/wait_for_slave_io_to_start.inc +++ b/mysql-test/include/wait_for_slave_io_to_start.inc @@ -14,13 +14,14 @@ # # Parameters: # $slave_timeout -# See include/wait_for_slave_param.inc +# Timeout used when waiting for the slave IO thread to start. +# See include/wait_for_slave_param.inc. # # $rpl_allow_error # By default, this file fails if there is an error in the IO -# thread. However, the IO thread can recover and reconnect after -# certain errors. If such an error is expected, can set -# $rpl_allow_error=1. This will prevent this file from failing if +# thread. However, the IO thread can recover and reconnect after +# certain errors. If such an error is expected, can set +# $rpl_allow_error=1. This will prevent this file from failing if # there is an error in the IO thread. # # $rpl_debug diff --git a/mysql-test/include/wait_for_slave_sql_to_start.inc b/mysql-test/include/wait_for_slave_sql_to_start.inc index 4aea9fba569..9286f1a08a2 100644 --- a/mysql-test/include/wait_for_slave_sql_to_start.inc +++ b/mysql-test/include/wait_for_slave_sql_to_start.inc @@ -11,6 +11,7 @@ # # Parameters: # $slave_timeout +# Timeout used when waiting for the slave SQL thread to start. # See include/wait_for_slave_param.inc # # $rpl_debug @@ -25,7 +26,7 @@ let $slave_param= Slave_SQL_Running; let $slave_param_value= Yes; # Unfortunately, the slave sql thread sets Slave_SQL_Running=Yes -# *before* it clears Last_SQL_Errno. So we have to allow errors in +# *before* it clears Last_SQL_Errno. So we have to allow errors in # the SQL thread here. #--let $slave_error_param= Last_SQL_Errno diff --git a/mysql-test/include/wait_for_slave_to_start.inc b/mysql-test/include/wait_for_slave_to_start.inc index a916e2ea615..742e7f7e0b4 100644 --- a/mysql-test/include/wait_for_slave_to_start.inc +++ b/mysql-test/include/wait_for_slave_to_start.inc @@ -12,6 +12,7 @@ # # Parameters: # $slave_timeout +# Timeout used when waiting for the slave threads to start. # See include/wait_for_slave_param.inc # # $rpl_debug From d59334da94ec97f2b44db8406038fefe540de6ef Mon Sep 17 00:00:00 2001 From: Yuchen Pei Date: Fri, 15 Sep 2023 11:01:16 +1000 Subject: [PATCH 6/8] MDEV-31673 [fixup] Fixing indentation from previous mdev-31673 patch --- storage/spider/spd_db_conn.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/storage/spider/spd_db_conn.cc b/storage/spider/spd_db_conn.cc index c01dc21c1ce..7250b53c192 100644 --- a/storage/spider/spd_db_conn.cc +++ b/storage/spider/spd_db_conn.cc @@ -4777,7 +4777,7 @@ int spider_db_seek_next( DBUG_RETURN(error_num); } pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); + SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); conn->need_mon = &spider->need_mons[link_idx]; DBUG_ASSERT(!conn->mta_conn_mutex_lock_already); DBUG_ASSERT(!conn->mta_conn_mutex_unlock_later); @@ -4872,7 +4872,7 @@ int spider_db_seek_next( DBUG_RETURN(error_num); } pthread_mutex_lock(&conn->mta_conn_mutex); - SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); + SPIDER_SET_FILE_POS(&conn->mta_conn_mutex_file_pos); conn->need_mon = &spider->need_mons[roop_count]; DBUG_ASSERT(!conn->mta_conn_mutex_lock_already); DBUG_ASSERT(!conn->mta_conn_mutex_unlock_later); From 96760d3accced5bc71543468ec1e1bc72b1c0898 Mon Sep 17 00:00:00 2001 From: Yuchen Pei Date: Thu, 14 Sep 2023 16:58:22 +1000 Subject: [PATCH 7/8] MDEV-31787 MDEV-26151 Add a test exercising non-0 spider_casual_read Also: - clean up spider_check_and_get_casual_read_conn() and spider_check_and_set_autocommit() - remove a couple of commented out code blocks --- .../spider/bugfix/r/mdev_26151.result | 34 +++++++ .../spider/bugfix/t/mdev_26151.test | 54 +++++++++++ storage/spider/spd_conn.cc | 90 ++++++++++++------- storage/spider/spd_trx.cc | 33 +------ 4 files changed, 147 insertions(+), 64 deletions(-) create mode 100644 storage/spider/mysql-test/spider/bugfix/r/mdev_26151.result create mode 100644 storage/spider/mysql-test/spider/bugfix/t/mdev_26151.test diff --git a/storage/spider/mysql-test/spider/bugfix/r/mdev_26151.result b/storage/spider/mysql-test/spider/bugfix/r/mdev_26151.result new file mode 100644 index 00000000000..b0a430e0e21 --- /dev/null +++ b/storage/spider/mysql-test/spider/bugfix/r/mdev_26151.result @@ -0,0 +1,34 @@ + +MDEV-26151 MDEV-31787 + +for master_1 +for child2 +for child3 +set @old_spider_bgs_mode= @@spider_bgs_mode; +set session spider_bgs_mode=1; +CREATE SERVER $srv FOREIGN DATA WRAPPER MYSQL OPTIONS (SOCKET "$MASTER_1_MYSOCK", DATABASE 'test',user 'root'); +create table td (a int, PRIMARY KEY (a)); +create table ts (a int, PRIMARY KEY (a)) ENGINE=Spider COMMENT='WRAPPER "mysql", srv "srv_mdev_26151",TABLE "td", casual_read "3"'; +insert into td values (11), (42); +select max(a) from ts; +max(a) +42 +drop table td, ts; +create table td (a int, PRIMARY KEY (a)); +create table ts (a int, PRIMARY KEY (a)) ENGINE=Spider COMMENT='WRAPPER "mysql", srv "srv_mdev_26151",TABLE "td", casual_read "1"'; +insert into td values (11), (42); +select max(a) from ts; +max(a) +42 +select min(a) from ts; +min(a) +11 +drop table td, ts; +drop server srv_mdev_26151; +set session spider_bgs_mode=@old_spider_bgs_mode; +for master_1 +for child2 +for child3 + +end of test + diff --git a/storage/spider/mysql-test/spider/bugfix/t/mdev_26151.test b/storage/spider/mysql-test/spider/bugfix/t/mdev_26151.test new file mode 100644 index 00000000000..f9e157d33e6 --- /dev/null +++ b/storage/spider/mysql-test/spider/bugfix/t/mdev_26151.test @@ -0,0 +1,54 @@ +--echo +--echo MDEV-26151 MDEV-31787 +--echo + +# This test exercises the code path where a nonzero casual_read takes +# effect. + +--disable_query_log +--disable_result_log +--source ../../t/test_init.inc +--enable_result_log +--enable_query_log + +--let $srv=srv_mdev_26151 +set @old_spider_bgs_mode= @@spider_bgs_mode; +set session spider_bgs_mode=1; +evalp CREATE SERVER $srv FOREIGN DATA WRAPPER MYSQL OPTIONS (SOCKET "$MASTER_1_MYSOCK", DATABASE 'test',user 'root'); + +# casual_read != 0 && casual_read != 1 +create table td (a int, PRIMARY KEY (a)); + +eval create table ts (a int, PRIMARY KEY (a)) ENGINE=Spider COMMENT='WRAPPER "mysql", srv "$srv",TABLE "td", casual_read "3"'; + +insert into td values (11), (42); + +select max(a) from ts; + +drop table td, ts; + +create table td (a int, PRIMARY KEY (a)); + +# casual_read = 1 +eval create table ts (a int, PRIMARY KEY (a)) ENGINE=Spider COMMENT='WRAPPER "mysql", srv "$srv",TABLE "td", casual_read "1"'; + +insert into td values (11), (42); + +select max(a) from ts; + +select min(a) from ts; + +drop table td, ts; + +eval drop server $srv; +set session spider_bgs_mode=@old_spider_bgs_mode; + +--disable_query_log +--disable_result_log +--source ../../t/test_deinit.inc +--enable_result_log +--enable_query_log + +--echo +--echo end of test +--echo diff --git a/storage/spider/spd_conn.cc b/storage/spider/spd_conn.cc index 85a7ccf88e2..4998c07370f 100644 --- a/storage/spider/spd_conn.cc +++ b/storage/spider/spd_conn.cc @@ -1260,6 +1260,30 @@ int spider_free_conn( DBUG_RETURN(0); } +/** + May get or create a connection spawning a background thread + + For each link (data node, formally representable as the tuple + (spider, link_idx)), there is an associated casual read value + (`spider->result_list.casual_read[link_idx]'). + + If the CRV is 0, do nothing. Otherwise, An casual read id + (`conn->casual_read_current_id`) is associated with the link and + query id. The CRI starts from 2, and is used only when CRV is 1, to + update the CRV (see below). The updated CRV is then used to + construct the connection key used for get or create a connection + that spawns a background thread to execute queries. + + If the CRV is 1, it is assigned CRI. The latter is then incremented + by 1. The CRI will only go up to 63, before "wrapping" back to 2. + + If 2 <= CRV <= 63, it is left alone. + + Note that this function relies on the assumption that the CRV is + reset (e.g. using `spider_param_casual_read()') between consecutive + calls of this function for the CRV == 1 case to auto-increment as + expected. +*/ int spider_check_and_get_casual_read_conn( THD *thd, ha_spider *spider, @@ -1267,41 +1291,35 @@ int spider_check_and_get_casual_read_conn( ) { int error_num; DBUG_ENTER("spider_check_and_get_casual_read_conn"); - if (spider->result_list.casual_read[link_idx]) + if (!spider->result_list.casual_read[link_idx]) + DBUG_RETURN(0); + SPIDER_CONN *conn = spider->conns[link_idx]; + if (conn->casual_read_query_id != thd->query_id) { - SPIDER_CONN *conn = spider->conns[link_idx]; - if (conn->casual_read_query_id != thd->query_id) - { - conn->casual_read_query_id = thd->query_id; - conn->casual_read_current_id = 2; - } - if (spider->result_list.casual_read[link_idx] == 1) - { - spider->result_list.casual_read[link_idx] = conn->casual_read_current_id; - ++conn->casual_read_current_id; - if (conn->casual_read_current_id > 63) - { - conn->casual_read_current_id = 2; - } - } - char first_byte_bak = *spider->conn_keys[link_idx]; - *spider->conn_keys[link_idx] = - '0' + spider->result_list.casual_read[link_idx]; - if ( - !(spider->conns[link_idx] = - spider_get_conn(spider->share, link_idx, - spider->conn_keys[link_idx], spider->trx, - spider, FALSE, TRUE, SPIDER_CONN_KIND_MYSQL, - &error_num)) - ) { - *spider->conn_keys[link_idx] = first_byte_bak; - DBUG_RETURN(error_num); - } - *spider->conn_keys[link_idx] = first_byte_bak; - spider->conns[link_idx]->casual_read_base_conn = conn; - conn = spider->conns[link_idx]; - spider_check_and_set_autocommit(thd, conn, NULL); + conn->casual_read_query_id = thd->query_id; + conn->casual_read_current_id = 2; } + if (spider->result_list.casual_read[link_idx] == 1) + { + spider->result_list.casual_read[link_idx] = conn->casual_read_current_id; + ++conn->casual_read_current_id; + if (conn->casual_read_current_id > 63) + conn->casual_read_current_id = 2; + } + char first_byte_bak = *spider->conn_keys[link_idx]; + *spider->conn_keys[link_idx] = + '0' + spider->result_list.casual_read[link_idx]; + if (!(spider->conns[link_idx]= spider_get_conn( + spider->share, link_idx, spider->conn_keys[link_idx], + spider->trx, spider, FALSE, TRUE, SPIDER_CONN_KIND_MYSQL, + &error_num))) + { + *spider->conn_keys[link_idx] = first_byte_bak; + DBUG_RETURN(error_num); + } + *spider->conn_keys[link_idx] = first_byte_bak; + spider->conns[link_idx]->casual_read_base_conn = conn; + spider_check_and_set_autocommit(thd, spider->conns[link_idx], NULL); DBUG_RETURN(0); } @@ -1837,6 +1855,12 @@ int spider_set_conn_bg_param( DBUG_RETURN(0); } +/** + Creates a background thread on `conn' to run `spider_bg_conn_action()' + + Does not create when `conn' is NULL or a bg thread has already been + created for `conn'. +*/ int spider_create_conn_thread( SPIDER_CONN *conn ) { diff --git a/storage/spider/spd_trx.cc b/storage/spider/spd_trx.cc index d64393fb445..311c78ccb5c 100644 --- a/storage/spider/spd_trx.cc +++ b/storage/spider/spd_trx.cc @@ -1589,27 +1589,9 @@ int spider_check_and_set_autocommit( SPIDER_CONN *conn, int *need_mon ) { - bool autocommit; DBUG_ENTER("spider_check_and_set_autocommit"); - - autocommit = !thd_test_options(thd, OPTION_NOT_AUTOCOMMIT); - if (autocommit) - { - spider_conn_queue_autocommit(conn, TRUE); - } else { - spider_conn_queue_autocommit(conn, FALSE); - } -/* - if (autocommit && conn->autocommit != 1) - { - spider_conn_queue_autocommit(conn, TRUE); - conn->autocommit = 1; - } else if (!autocommit && conn->autocommit != 0) - { - spider_conn_queue_autocommit(conn, FALSE); - conn->autocommit = 0; - } -*/ + spider_conn_queue_autocommit( + conn, !thd_test_options(thd, OPTION_NOT_AUTOCOMMIT)); DBUG_RETURN(0); } @@ -1631,17 +1613,6 @@ int spider_check_and_set_sql_log_off( spider_conn_queue_sql_log_off(conn, FALSE); } } -/* - if (internal_sql_log_off && conn->sql_log_off != 1) - { - spider_conn_queue_sql_log_off(conn, TRUE); - conn->sql_log_off = 1; - } else if (!internal_sql_log_off && conn->sql_log_off != 0) - { - spider_conn_queue_sql_log_off(conn, FALSE); - conn->sql_log_off = 0; - } -*/ DBUG_RETURN(0); } From 68a002071b7e5d5cbd7609459e04a2d46a08e9f7 Mon Sep 17 00:00:00 2001 From: Yuchen Pei Date: Thu, 14 Sep 2023 16:12:10 +1000 Subject: [PATCH 8/8] MDEV-29502 Fix some issues with spider direct aggregate The direct aggregate mechanism sems to be only intended to work when otherwise a full table scan query will be executed from the spider node and the aggregation done at the spider node too. Typically this happens in sub_select(). In the test spider.direct_aggregate_part direct aggregate allows to send COUNT statements directly to the data nodes and adds up the results at the spider node, instead of iterating over the rows one by one at the spider node. By contrast, the group by handler (GBH) typically sends aggregated queries directly to data nodes, in which case DA does not improve the situation here. That is why we should fix it by disabling DA when GBH is used. There are other reasons supporting this change. First, the creation of GBH results in a call to change_to_use_tmp_fields() (as opposed to setup_copy_fields()) which causes the spider DA function spider_db_fetch_for_item_sum_funcs() to work on wrong items. Second, the spider DA function only calls direct_add() on the items, and the follow-up add() needs to be called by the sql layer code. In do_select(), after executing the query with the GBH, it seems that the required add() would not necessarily be called. Disabling DA when GBH is used does fix the bug. There are a few other things included in this commit to improve the situation with spider DA: 1. Add a session variable that allows user to disable DA completely, this will help as a temporary measure if/when further bugs with DA emerge. 2. Move the increment of direct_aggregate_count to the spider DA function. Currently this is done in rather bizarre and random locations. 3. Fix the spider_db_mbase_row creation so that the last of its row field (sentinel) is NULL. The code is already doing a null check, but somehow the sentinel field is on an invalid address, causing the segfaults. With a correct implementation of the row creation, we can avoid such segfaults. --- storage/spider/ha_spider.cc | 1 - .../spider/bg/r/direct_aggregate.result | 8 +-- .../spider/bg/r/direct_aggregate_part.result | 39 +++++++++-- .../spider/bg/t/direct_aggregate_part.test | 16 +++++ .../bugfix/r/group_by_order_by_limit.result | 9 +++ .../spider/bugfix/r/mdev_25116.result | 9 +++ .../spider/bugfix/r/mdev_29502.result | 68 +++++++++++++++++++ .../spider/bugfix/r/strict_group_by.result | 9 +++ .../bugfix/t/group_by_order_by_limit.test | 5 ++ .../spider/bugfix/t/mdev_25116.test | 7 ++ .../spider/bugfix/t/mdev_29502.test | 67 ++++++++++++++++++ .../spider/bugfix/t/strict_group_by.test | 5 ++ .../spider/r/direct_aggregate.result | 28 ++++++-- .../spider/r/direct_aggregate_part.result | 39 +++++++++-- .../r/group_by_order_by_limit_ok.result | 9 +++ .../e112122/t/group_by_order_by_limit_ok.test | 5 ++ .../mysql-test/spider/t/direct_aggregate.test | 10 +++ .../spider/t/direct_aggregate_part.test | 17 +++++ storage/spider/spd_db_conn.cc | 23 +++++++ storage/spider/spd_db_mysql.cc | 14 ++-- storage/spider/spd_group_by_handler.cc | 2 + storage/spider/spd_param.cc | 17 +++++ storage/spider/spd_param.h | 1 + storage/spider/spd_table.cc | 2 +- 24 files changed, 386 insertions(+), 24 deletions(-) create mode 100644 storage/spider/mysql-test/spider/bugfix/r/mdev_29502.result create mode 100644 storage/spider/mysql-test/spider/bugfix/t/mdev_29502.test diff --git a/storage/spider/ha_spider.cc b/storage/spider/ha_spider.cc index 787556bc070..40e56846092 100644 --- a/storage/spider/ha_spider.cc +++ b/storage/spider/ha_spider.cc @@ -14839,7 +14839,6 @@ int ha_spider::append_sum_select_sql_part( DBUG_RETURN(error_num); } } - wide_handler->trx->direct_aggregate_count++; DBUG_RETURN(0); } #endif diff --git a/storage/spider/mysql-test/spider/bg/r/direct_aggregate.result b/storage/spider/mysql-test/spider/bg/r/direct_aggregate.result index ede48906a84..9a8660ba79e 100644 --- a/storage/spider/mysql-test/spider/bg/r/direct_aggregate.result +++ b/storage/spider/mysql-test/spider/bg/r/direct_aggregate.result @@ -60,25 +60,25 @@ MAX(a) 5 SHOW STATUS LIKE 'Spider_direct_aggregate'; Variable_name Value -Spider_direct_aggregate 1 +Spider_direct_aggregate 0 SELECT MIN(a) FROM ta_l; MIN(a) 1 SHOW STATUS LIKE 'Spider_direct_aggregate'; Variable_name Value -Spider_direct_aggregate 2 +Spider_direct_aggregate 0 SELECT MAX(a) FROM ta_l WHERE a < 5; MAX(a) 4 SHOW STATUS LIKE 'Spider_direct_aggregate'; Variable_name Value -Spider_direct_aggregate 3 +Spider_direct_aggregate 0 SELECT MIN(a) FROM ta_l WHERE a > 1; MIN(a) 2 SHOW STATUS LIKE 'Spider_direct_aggregate'; Variable_name Value -Spider_direct_aggregate 4 +Spider_direct_aggregate 0 deinit connection master_1; diff --git a/storage/spider/mysql-test/spider/bg/r/direct_aggregate_part.result b/storage/spider/mysql-test/spider/bg/r/direct_aggregate_part.result index 02cdc033a88..9d65fcc2570 100644 --- a/storage/spider/mysql-test/spider/bg/r/direct_aggregate_part.result +++ b/storage/spider/mysql-test/spider/bg/r/direct_aggregate_part.result @@ -36,6 +36,8 @@ b CHAR(1), c DATETIME, PRIMARY KEY(a) ) MASTER_1_ENGINE MASTER_1_COMMENT2_P_2_1 +set @old_spider_direct_aggregate=@@session.spider_direct_aggregate; +set spider_direct_aggregate=1; SHOW STATUS LIKE 'Spider_direct_aggregate'; Variable_name Value Spider_direct_aggregate 0 @@ -44,19 +46,25 @@ COUNT(*) 5 SHOW STATUS LIKE 'Spider_direct_aggregate'; Variable_name Value -Spider_direct_aggregate 2 +Spider_direct_aggregate 4 +SELECT SUM(a) FROM ta_l2; +SUM(a) +15 +SHOW STATUS LIKE 'Spider_direct_aggregate'; +Variable_name Value +Spider_direct_aggregate 8 SELECT MAX(a) FROM ta_l2; MAX(a) 5 SHOW STATUS LIKE 'Spider_direct_aggregate'; Variable_name Value -Spider_direct_aggregate 4 +Spider_direct_aggregate 8 SELECT MIN(a) FROM ta_l2; MIN(a) 1 SHOW STATUS LIKE 'Spider_direct_aggregate'; Variable_name Value -Spider_direct_aggregate 6 +Spider_direct_aggregate 8 SELECT MAX(a) FROM ta_l2 WHERE a < 5; MAX(a) 4 @@ -68,7 +76,30 @@ MIN(a) 2 SHOW STATUS LIKE 'Spider_direct_aggregate'; Variable_name Value -Spider_direct_aggregate 10 +Spider_direct_aggregate 8 +SET spider_direct_aggregate=0; +SELECT COUNT(*) FROM ta_l2; +COUNT(*) +5 +SELECT SUM(a) FROM ta_l2; +SUM(a) +15 +SELECT MAX(a) FROM ta_l2; +MAX(a) +5 +SELECT MIN(a) FROM ta_l2; +MIN(a) +1 +SELECT MAX(a) FROM ta_l2 WHERE a < 5; +MAX(a) +4 +SELECT MIN(a) FROM ta_l2 WHERE a > 1; +MIN(a) +2 +SHOW STATUS LIKE 'Spider_direct_aggregate'; +Variable_name Value +Spider_direct_aggregate 8 +set spider_direct_aggregate=@old_spider_direct_aggregate; deinit connection master_1; diff --git a/storage/spider/mysql-test/spider/bg/t/direct_aggregate_part.test b/storage/spider/mysql-test/spider/bg/t/direct_aggregate_part.test index d6d4623afc5..05a81c766ee 100644 --- a/storage/spider/mysql-test/spider/bg/t/direct_aggregate_part.test +++ b/storage/spider/mysql-test/spider/bg/t/direct_aggregate_part.test @@ -133,9 +133,14 @@ if ($HAVE_PARTITION) (5, 'c', '2001-12-31 23:59:59'); --enable_query_log --disable_ps2_protocol + set @old_spider_direct_aggregate=@@session.spider_direct_aggregate; + set spider_direct_aggregate=1; + eval $MASTER_1_CHECK_DIRECT_AGGREGATE_STATUS; SELECT COUNT(*) FROM ta_l2; eval $MASTER_1_CHECK_DIRECT_AGGREGATE_STATUS; + SELECT SUM(a) FROM ta_l2; + eval $MASTER_1_CHECK_DIRECT_AGGREGATE_STATUS; SELECT MAX(a) FROM ta_l2; eval $MASTER_1_CHECK_DIRECT_AGGREGATE_STATUS; SELECT MIN(a) FROM ta_l2; @@ -145,6 +150,17 @@ if ($HAVE_PARTITION) SELECT MIN(a) FROM ta_l2 WHERE a > 1; eval $MASTER_1_CHECK_DIRECT_AGGREGATE_STATUS; --enable_ps2_protocol + + SET spider_direct_aggregate=0; + SELECT COUNT(*) FROM ta_l2; + SELECT SUM(a) FROM ta_l2; + SELECT MAX(a) FROM ta_l2; + SELECT MIN(a) FROM ta_l2; + SELECT MAX(a) FROM ta_l2 WHERE a < 5; + SELECT MIN(a) FROM ta_l2 WHERE a > 1; + eval $MASTER_1_CHECK_DIRECT_AGGREGATE_STATUS; + + set spider_direct_aggregate=@old_spider_direct_aggregate; if ($USE_CHILD_GROUP2) { if (!$OUTPUT_CHILD_GROUP2) diff --git a/storage/spider/mysql-test/spider/bugfix/r/group_by_order_by_limit.result b/storage/spider/mysql-test/spider/bugfix/r/group_by_order_by_limit.result index 8a2bcb73537..3bce815436d 100644 --- a/storage/spider/mysql-test/spider/bugfix/r/group_by_order_by_limit.result +++ b/storage/spider/mysql-test/spider/bugfix/r/group_by_order_by_limit.result @@ -46,6 +46,11 @@ TRUNCATE TABLE mysql.general_log; connection child2_2; TRUNCATE TABLE mysql.general_log; connection master_1; +set @old_spider_direct_aggregate=@@session.spider_direct_aggregate; +set spider_direct_aggregate=1; +SHOW STATUS LIKE 'Spider_direct_aggregate'; +Variable_name Value +Spider_direct_aggregate 0 SELECT skey, count(*) cnt FROM tbl_a GROUP BY skey ORDER BY cnt DESC, skey DESC LIMIT 5; skey cnt 14 2 @@ -53,6 +58,10 @@ skey cnt 12 2 11 2 10 2 +SHOW STATUS LIKE 'Spider_direct_aggregate'; +Variable_name Value +Spider_direct_aggregate 60 +set spider_direct_aggregate=@old_spider_direct_aggregate; connection child2_1; SELECT argument FROM mysql.general_log WHERE argument LIKE '%select %'; argument diff --git a/storage/spider/mysql-test/spider/bugfix/r/mdev_25116.result b/storage/spider/mysql-test/spider/bugfix/r/mdev_25116.result index cd2ca4b1635..09efba25227 100644 --- a/storage/spider/mysql-test/spider/bugfix/r/mdev_25116.result +++ b/storage/spider/mysql-test/spider/bugfix/r/mdev_25116.result @@ -18,9 +18,18 @@ CREATE TABLE tbl_a ( id INT ) ENGINE=Spider DEFAULT CHARSET=utf8 COMMENT='table "tbl_a", srv "s_2_1"'; connection master_1; +set @old_spider_direct_aggregate=@@session.spider_direct_aggregate; +set spider_direct_aggregate=1; +SHOW STATUS LIKE 'Spider_direct_aggregate'; +Variable_name Value +Spider_direct_aggregate 0 SELECT IF(COUNT(id > 0),'Y','N') FROM tbl_a; IF(COUNT(id > 0),'Y','N') N +SHOW STATUS LIKE 'Spider_direct_aggregate'; +Variable_name Value +Spider_direct_aggregate 1 +set spider_direct_aggregate=@old_spider_direct_aggregate; connection master_1; DROP DATABASE IF EXISTS auto_test_local; connection child2_1; diff --git a/storage/spider/mysql-test/spider/bugfix/r/mdev_29502.result b/storage/spider/mysql-test/spider/bugfix/r/mdev_29502.result new file mode 100644 index 00000000000..54c5e9a6473 --- /dev/null +++ b/storage/spider/mysql-test/spider/bugfix/r/mdev_29502.result @@ -0,0 +1,68 @@ +# +# MDEV-29502 ASAN: heap-buffer-overflow & stack-buffer-overflow in spider_db_mbase_row::append_to_str | SIGSEGV's in __memmove_avx_unaligned_erms from memcpy in Binary_string::q_append, in Static_binary_string::q_append and my_strntoull10rnd_8bit | Unknown error 12801 +# +for master_1 +for child2 +for child3 +CREATE SERVER $srv FOREIGN DATA WRAPPER MYSQL OPTIONS (SOCKET "$MASTER_1_MYSOCK", DATABASE 'test',user 'root'); +CREATE TABLE t (a INT); +INSERT INTO t VALUES (23),(48); +set @old_spider_direct_aggregate=@@session.spider_direct_aggregate; +set spider_direct_aggregate=1; +CREATE TABLE t1 (a INT KEY) ENGINE=Spider COMMENT='WRAPPER "mysql",srv "srv_mdev_29502",TABLE "t"'; +SELECT MAX(a) FROM t1; +MAX(a) +48 +SELECT SUM(a) FROM t1; +SUM(a) +71 +SELECT COUNT(a) FROM t1; +COUNT(a) +2 +SELECT MAX(a), SUM(a) FROM t1; +MAX(a) SUM(a) +48 71 +SELECT COUNT(a), MAX(a), SUM(a) FROM t1; +COUNT(a) MAX(a) SUM(a) +2 48 71 +SELECT MAX(a), COUNT(a), SUM(a) FROM t1; +MAX(a) COUNT(a) SUM(a) +48 2 71 +SELECT MAX(a), MAX(COALESCE(a)) FROM t1; +MAX(a) MAX(COALESCE(a)) +48 48 +SHOW STATUS LIKE 'Spider_direct_aggregate'; +Variable_name Value +Spider_direct_aggregate 0 +DROP TABLE t, t1; +CREATE TABLE t (a INT, b INT); +INSERT INTO t VALUES (23, -1),(48, 97); +CREATE TABLE t1 (a INT, b INT) ENGINE=Spider COMMENT='WRAPPER "mysql",srv "srv_mdev_29502",TABLE "t"'; +SELECT MAX(a + b), SUM(a - b) FROM t1; +MAX(a + b) SUM(a - b) +145 -25 +SHOW STATUS LIKE 'Spider_direct_aggregate'; +Variable_name Value +Spider_direct_aggregate 0 +DROP TABLE t, t1; +CREATE TABLE t (a INT); +INSERT INTO t VALUES (23),(97),(42); +CREATE TABLE t1 (a INT) ENGINE=Spider COMMENT='WRAPPER "mysql",srv "srv_mdev_29502",TABLE "t"'; +SELECT IF(COUNT(a > 0),'Y','N'), MAX(a) FROM t1; +IF(COUNT(a > 0),'Y','N') MAX(a) +Y 97 +SHOW STATUS LIKE 'Spider_direct_aggregate'; +Variable_name Value +Spider_direct_aggregate 2 +SELECT MAX(a), IF(COUNT(a > 0),'Y','N') FROM t1; +MAX(a) IF(COUNT(a > 0),'Y','N') +97 Y +SHOW STATUS LIKE 'Spider_direct_aggregate'; +Variable_name Value +Spider_direct_aggregate 4 +DROP TABLE t, t1; +set spider_direct_aggregate=@old_spider_direct_aggregate; +DROP SERVER srv_mdev_29502; +for master_1 +for child2 +for child3 diff --git a/storage/spider/mysql-test/spider/bugfix/r/strict_group_by.result b/storage/spider/mysql-test/spider/bugfix/r/strict_group_by.result index f2287dea65a..4b97b3799bf 100644 --- a/storage/spider/mysql-test/spider/bugfix/r/strict_group_by.result +++ b/storage/spider/mysql-test/spider/bugfix/r/strict_group_by.result @@ -53,9 +53,18 @@ connection child2_2; TRUNCATE TABLE mysql.general_log; connection master_1; FLUSH TABLES; +set @old_spider_direct_aggregate=@@session.spider_direct_aggregate; +set spider_direct_aggregate=1; +SHOW STATUS LIKE 'Spider_direct_aggregate'; +Variable_name Value +Spider_direct_aggregate 0 SELECT count(pkey) cnt, skey FROM tbl_a; cnt skey 30 1 +SHOW STATUS LIKE 'Spider_direct_aggregate'; +Variable_name Value +Spider_direct_aggregate 2 +set spider_direct_aggregate=@old_spider_direct_aggregate; connection child2_1; SELECT argument FROM mysql.general_log WHERE argument LIKE '%select %'; argument diff --git a/storage/spider/mysql-test/spider/bugfix/t/group_by_order_by_limit.test b/storage/spider/mysql-test/spider/bugfix/t/group_by_order_by_limit.test index 96c2ea6c849..d694230bf2a 100644 --- a/storage/spider/mysql-test/spider/bugfix/t/group_by_order_by_limit.test +++ b/storage/spider/mysql-test/spider/bugfix/t/group_by_order_by_limit.test @@ -68,7 +68,12 @@ TRUNCATE TABLE mysql.general_log; --disable_ps2_protocol --connection master_1 +set @old_spider_direct_aggregate=@@session.spider_direct_aggregate; +set spider_direct_aggregate=1; +SHOW STATUS LIKE 'Spider_direct_aggregate'; SELECT skey, count(*) cnt FROM tbl_a GROUP BY skey ORDER BY cnt DESC, skey DESC LIMIT 5; +SHOW STATUS LIKE 'Spider_direct_aggregate'; +set spider_direct_aggregate=@old_spider_direct_aggregate; --connection child2_1 eval $CHILD2_1_SELECT_ARGUMENT1; diff --git a/storage/spider/mysql-test/spider/bugfix/t/mdev_25116.test b/storage/spider/mysql-test/spider/bugfix/t/mdev_25116.test index 70c2bcc51b7..7ef23538259 100644 --- a/storage/spider/mysql-test/spider/bugfix/t/mdev_25116.test +++ b/storage/spider/mysql-test/spider/bugfix/t/mdev_25116.test @@ -23,7 +23,14 @@ eval CREATE TABLE tbl_a ( ) $MASTER_1_ENGINE $MASTER_1_CHARSET COMMENT='table "tbl_a", srv "s_2_1"'; --connection master_1 +--disable_ps2_protocol +set @old_spider_direct_aggregate=@@session.spider_direct_aggregate; +set spider_direct_aggregate=1; +SHOW STATUS LIKE 'Spider_direct_aggregate'; SELECT IF(COUNT(id > 0),'Y','N') FROM tbl_a; +SHOW STATUS LIKE 'Spider_direct_aggregate'; +set spider_direct_aggregate=@old_spider_direct_aggregate; +--enable_ps2_protocol --connection master_1 DROP DATABASE IF EXISTS auto_test_local; diff --git a/storage/spider/mysql-test/spider/bugfix/t/mdev_29502.test b/storage/spider/mysql-test/spider/bugfix/t/mdev_29502.test new file mode 100644 index 00000000000..88591f803f5 --- /dev/null +++ b/storage/spider/mysql-test/spider/bugfix/t/mdev_29502.test @@ -0,0 +1,67 @@ +--echo # +--echo # MDEV-29502 ASAN: heap-buffer-overflow & stack-buffer-overflow in spider_db_mbase_row::append_to_str | SIGSEGV's in __memmove_avx_unaligned_erms from memcpy in Binary_string::q_append, in Static_binary_string::q_append and my_strntoull10rnd_8bit | Unknown error 12801 +--echo # + +--disable_query_log +--disable_result_log +--source ../../t/test_init.inc +--enable_result_log +--enable_query_log + +--let $srv=srv_mdev_29502 +evalp CREATE SERVER $srv FOREIGN DATA WRAPPER MYSQL OPTIONS (SOCKET "$MASTER_1_MYSOCK", DATABASE 'test',user 'root'); + +CREATE TABLE t (a INT); +INSERT INTO t VALUES (23),(48); + +set @old_spider_direct_aggregate=@@session.spider_direct_aggregate; +set spider_direct_aggregate=1; + +eval CREATE TABLE t1 (a INT KEY) ENGINE=Spider COMMENT='WRAPPER "mysql",srv "$srv",TABLE "t"'; + +SELECT MAX(a) FROM t1; +SELECT SUM(a) FROM t1; +SELECT COUNT(a) FROM t1; +SELECT MAX(a), SUM(a) FROM t1; +SELECT COUNT(a), MAX(a), SUM(a) FROM t1; +SELECT MAX(a), COUNT(a), SUM(a) FROM t1; +SELECT MAX(a), MAX(COALESCE(a)) FROM t1; +SHOW STATUS LIKE 'Spider_direct_aggregate'; + +DROP TABLE t, t1; + +CREATE TABLE t (a INT, b INT); +INSERT INTO t VALUES (23, -1),(48, 97); + +eval CREATE TABLE t1 (a INT, b INT) ENGINE=Spider COMMENT='WRAPPER "mysql",srv "$srv",TABLE "t"'; + +SELECT MAX(a + b), SUM(a - b) FROM t1; +SHOW STATUS LIKE 'Spider_direct_aggregate'; + +DROP TABLE t, t1; + +# An example where GBH is not created (MDEV-25116), and direct +# aggregate is used to speed up the query. +CREATE TABLE t (a INT); +INSERT INTO t VALUES (23),(97),(42); + +eval CREATE TABLE t1 (a INT) ENGINE=Spider COMMENT='WRAPPER "mysql",srv "$srv",TABLE "t"'; + +--disable_ps2_protocol +SELECT IF(COUNT(a > 0),'Y','N'), MAX(a) FROM t1; +SHOW STATUS LIKE 'Spider_direct_aggregate'; +SELECT MAX(a), IF(COUNT(a > 0),'Y','N') FROM t1; +SHOW STATUS LIKE 'Spider_direct_aggregate'; +--enable_ps2_protocol + +DROP TABLE t, t1; + +set spider_direct_aggregate=@old_spider_direct_aggregate; + +eval DROP SERVER $srv; + +--disable_query_log +--disable_result_log +--source ../../t/test_deinit.inc +--enable_result_log +--enable_query_log diff --git a/storage/spider/mysql-test/spider/bugfix/t/strict_group_by.test b/storage/spider/mysql-test/spider/bugfix/t/strict_group_by.test index a084be372fa..eaf149fa5da 100644 --- a/storage/spider/mysql-test/spider/bugfix/t/strict_group_by.test +++ b/storage/spider/mysql-test/spider/bugfix/t/strict_group_by.test @@ -68,9 +68,14 @@ TRUNCATE TABLE mysql.general_log; --connection master_1 FLUSH TABLES; +set @old_spider_direct_aggregate=@@session.spider_direct_aggregate; +set spider_direct_aggregate=1; +SHOW STATUS LIKE 'Spider_direct_aggregate'; --disable_ps2_protocol SELECT count(pkey) cnt, skey FROM tbl_a; --enable_ps2_protocol +SHOW STATUS LIKE 'Spider_direct_aggregate'; +set spider_direct_aggregate=@old_spider_direct_aggregate; --connection child2_1 --disable_ps2_protocol diff --git a/storage/spider/mysql-test/spider/r/direct_aggregate.result b/storage/spider/mysql-test/spider/r/direct_aggregate.result index ede48906a84..2455fe1823a 100644 --- a/storage/spider/mysql-test/spider/r/direct_aggregate.result +++ b/storage/spider/mysql-test/spider/r/direct_aggregate.result @@ -60,25 +60,45 @@ MAX(a) 5 SHOW STATUS LIKE 'Spider_direct_aggregate'; Variable_name Value -Spider_direct_aggregate 1 +Spider_direct_aggregate 0 SELECT MIN(a) FROM ta_l; MIN(a) 1 SHOW STATUS LIKE 'Spider_direct_aggregate'; Variable_name Value -Spider_direct_aggregate 2 +Spider_direct_aggregate 0 SELECT MAX(a) FROM ta_l WHERE a < 5; MAX(a) 4 SHOW STATUS LIKE 'Spider_direct_aggregate'; Variable_name Value -Spider_direct_aggregate 3 +Spider_direct_aggregate 0 SELECT MIN(a) FROM ta_l WHERE a > 1; MIN(a) 2 SHOW STATUS LIKE 'Spider_direct_aggregate'; Variable_name Value -Spider_direct_aggregate 4 +Spider_direct_aggregate 0 +SET spider_direct_aggregate=0; +SELECT COUNT(*) FROM ta_l; +COUNT(*) +5 +SELECT MAX(a) FROM ta_l; +MAX(a) +5 +SELECT MIN(a) FROM ta_l; +MIN(a) +1 +SELECT MAX(a) FROM ta_l WHERE a < 5; +MAX(a) +4 +SELECT MIN(a) FROM ta_l WHERE a > 1; +MIN(a) +2 +SHOW STATUS LIKE 'Spider_direct_aggregate'; +Variable_name Value +Spider_direct_aggregate 0 +SET spider_direct_aggregate=1; deinit connection master_1; diff --git a/storage/spider/mysql-test/spider/r/direct_aggregate_part.result b/storage/spider/mysql-test/spider/r/direct_aggregate_part.result index 02cdc033a88..9d65fcc2570 100644 --- a/storage/spider/mysql-test/spider/r/direct_aggregate_part.result +++ b/storage/spider/mysql-test/spider/r/direct_aggregate_part.result @@ -36,6 +36,8 @@ b CHAR(1), c DATETIME, PRIMARY KEY(a) ) MASTER_1_ENGINE MASTER_1_COMMENT2_P_2_1 +set @old_spider_direct_aggregate=@@session.spider_direct_aggregate; +set spider_direct_aggregate=1; SHOW STATUS LIKE 'Spider_direct_aggregate'; Variable_name Value Spider_direct_aggregate 0 @@ -44,19 +46,25 @@ COUNT(*) 5 SHOW STATUS LIKE 'Spider_direct_aggregate'; Variable_name Value -Spider_direct_aggregate 2 +Spider_direct_aggregate 4 +SELECT SUM(a) FROM ta_l2; +SUM(a) +15 +SHOW STATUS LIKE 'Spider_direct_aggregate'; +Variable_name Value +Spider_direct_aggregate 8 SELECT MAX(a) FROM ta_l2; MAX(a) 5 SHOW STATUS LIKE 'Spider_direct_aggregate'; Variable_name Value -Spider_direct_aggregate 4 +Spider_direct_aggregate 8 SELECT MIN(a) FROM ta_l2; MIN(a) 1 SHOW STATUS LIKE 'Spider_direct_aggregate'; Variable_name Value -Spider_direct_aggregate 6 +Spider_direct_aggregate 8 SELECT MAX(a) FROM ta_l2 WHERE a < 5; MAX(a) 4 @@ -68,7 +76,30 @@ MIN(a) 2 SHOW STATUS LIKE 'Spider_direct_aggregate'; Variable_name Value -Spider_direct_aggregate 10 +Spider_direct_aggregate 8 +SET spider_direct_aggregate=0; +SELECT COUNT(*) FROM ta_l2; +COUNT(*) +5 +SELECT SUM(a) FROM ta_l2; +SUM(a) +15 +SELECT MAX(a) FROM ta_l2; +MAX(a) +5 +SELECT MIN(a) FROM ta_l2; +MIN(a) +1 +SELECT MAX(a) FROM ta_l2 WHERE a < 5; +MAX(a) +4 +SELECT MIN(a) FROM ta_l2 WHERE a > 1; +MIN(a) +2 +SHOW STATUS LIKE 'Spider_direct_aggregate'; +Variable_name Value +Spider_direct_aggregate 8 +set spider_direct_aggregate=@old_spider_direct_aggregate; deinit connection master_1; diff --git a/storage/spider/mysql-test/spider/regression/e112122/r/group_by_order_by_limit_ok.result b/storage/spider/mysql-test/spider/regression/e112122/r/group_by_order_by_limit_ok.result index 96746e09b8d..e9ded930046 100644 --- a/storage/spider/mysql-test/spider/regression/e112122/r/group_by_order_by_limit_ok.result +++ b/storage/spider/mysql-test/spider/regression/e112122/r/group_by_order_by_limit_ok.result @@ -46,6 +46,11 @@ TRUNCATE TABLE mysql.general_log; connection child2_2; TRUNCATE TABLE mysql.general_log; connection master_1; +set @old_spider_direct_aggregate=@@session.spider_direct_aggregate; +set spider_direct_aggregate=1; +SHOW STATUS LIKE 'Spider_direct_aggregate'; +Variable_name Value +Spider_direct_aggregate 0 SELECT skey, count(*) cnt FROM tbl_a GROUP BY skey ORDER BY cnt DESC, skey DESC LIMIT 5; skey cnt 14 2 @@ -53,6 +58,10 @@ skey cnt 12 2 11 2 10 2 +SHOW STATUS LIKE 'Spider_direct_aggregate'; +Variable_name Value +Spider_direct_aggregate 20 +set spider_direct_aggregate=@old_spider_direct_aggregate; connection child2_1; SELECT argument FROM mysql.general_log WHERE argument LIKE '%select %'; argument diff --git a/storage/spider/mysql-test/spider/regression/e112122/t/group_by_order_by_limit_ok.test b/storage/spider/mysql-test/spider/regression/e112122/t/group_by_order_by_limit_ok.test index 868886a284e..e4f3f2155bb 100644 --- a/storage/spider/mysql-test/spider/regression/e112122/t/group_by_order_by_limit_ok.test +++ b/storage/spider/mysql-test/spider/regression/e112122/t/group_by_order_by_limit_ok.test @@ -68,7 +68,12 @@ TRUNCATE TABLE mysql.general_log; --disable_ps2_protocol --connection master_1 +set @old_spider_direct_aggregate=@@session.spider_direct_aggregate; +set spider_direct_aggregate=1; +SHOW STATUS LIKE 'Spider_direct_aggregate'; SELECT skey, count(*) cnt FROM tbl_a GROUP BY skey ORDER BY cnt DESC, skey DESC LIMIT 5; +SHOW STATUS LIKE 'Spider_direct_aggregate'; +set spider_direct_aggregate=@old_spider_direct_aggregate; --connection child2_1 eval $CHILD2_1_SELECT_ARGUMENT1; diff --git a/storage/spider/mysql-test/spider/t/direct_aggregate.test b/storage/spider/mysql-test/spider/t/direct_aggregate.test index ca96d4bd41f..dd568f34579 100644 --- a/storage/spider/mysql-test/spider/t/direct_aggregate.test +++ b/storage/spider/mysql-test/spider/t/direct_aggregate.test @@ -139,6 +139,16 @@ eval $MASTER_1_CHECK_DIRECT_AGGREGATE_STATUS; SELECT MIN(a) FROM ta_l WHERE a > 1; eval $MASTER_1_CHECK_DIRECT_AGGREGATE_STATUS; --enable_ps2_protocol + +SET spider_direct_aggregate=0; +SELECT COUNT(*) FROM ta_l; +SELECT MAX(a) FROM ta_l; +SELECT MIN(a) FROM ta_l; +SELECT MAX(a) FROM ta_l WHERE a < 5; +SELECT MIN(a) FROM ta_l WHERE a > 1; +eval $MASTER_1_CHECK_DIRECT_AGGREGATE_STATUS; +SET spider_direct_aggregate=1; + if ($USE_CHILD_GROUP2) { if (!$OUTPUT_CHILD_GROUP2) diff --git a/storage/spider/mysql-test/spider/t/direct_aggregate_part.test b/storage/spider/mysql-test/spider/t/direct_aggregate_part.test index d6d4623afc5..dccb72f5c91 100644 --- a/storage/spider/mysql-test/spider/t/direct_aggregate_part.test +++ b/storage/spider/mysql-test/spider/t/direct_aggregate_part.test @@ -133,9 +133,14 @@ if ($HAVE_PARTITION) (5, 'c', '2001-12-31 23:59:59'); --enable_query_log --disable_ps2_protocol + set @old_spider_direct_aggregate=@@session.spider_direct_aggregate; + set spider_direct_aggregate=1; + eval $MASTER_1_CHECK_DIRECT_AGGREGATE_STATUS; SELECT COUNT(*) FROM ta_l2; eval $MASTER_1_CHECK_DIRECT_AGGREGATE_STATUS; + SELECT SUM(a) FROM ta_l2; + eval $MASTER_1_CHECK_DIRECT_AGGREGATE_STATUS; SELECT MAX(a) FROM ta_l2; eval $MASTER_1_CHECK_DIRECT_AGGREGATE_STATUS; SELECT MIN(a) FROM ta_l2; @@ -145,6 +150,18 @@ if ($HAVE_PARTITION) SELECT MIN(a) FROM ta_l2 WHERE a > 1; eval $MASTER_1_CHECK_DIRECT_AGGREGATE_STATUS; --enable_ps2_protocol + + SET spider_direct_aggregate=0; + SELECT COUNT(*) FROM ta_l2; + SELECT SUM(a) FROM ta_l2; + SELECT MAX(a) FROM ta_l2; + SELECT MIN(a) FROM ta_l2; + SELECT MAX(a) FROM ta_l2 WHERE a < 5; + SELECT MIN(a) FROM ta_l2 WHERE a > 1; + eval $MASTER_1_CHECK_DIRECT_AGGREGATE_STATUS; + + set spider_direct_aggregate=@old_spider_direct_aggregate; + if ($USE_CHILD_GROUP2) { if (!$OUTPUT_CHILD_GROUP2) diff --git a/storage/spider/spd_db_conn.cc b/storage/spider/spd_db_conn.cc index 5bb103f3efc..c46c207352d 100644 --- a/storage/spider/spd_db_conn.cc +++ b/storage/spider/spd_db_conn.cc @@ -2778,6 +2778,17 @@ int spider_db_refetch_for_item_sum_funcs( DBUG_RETURN(0); } +/** + Directly add field values in a row to aggregate items. + + Iterate over the result row fields and aggregate items, and call + direct_add on the latter with the values from the former. + + @param row The result row + @param spider The spider handler + @retval 0 Success + @retval nonzero Failure +*/ int spider_db_fetch_for_item_sum_funcs( SPIDER_DB_ROW *row, ha_spider *spider @@ -2798,6 +2809,17 @@ int spider_db_fetch_for_item_sum_funcs( DBUG_RETURN(0); } +/** + Directly add one field of a row to an item for direct aggregate. + + Call direct_add on an Item_sum with the value stored in a field. + + @param row A row whose `row` points to the field to be added + @param item_sum The Item_sum to be added the field + @param ha_spider The spider handler + @retval 0 Success + @retval nonzero Failure +*/ int spider_db_fetch_for_item_sum_func( SPIDER_DB_ROW *row, Item_sum *item_sum, @@ -2948,6 +2970,7 @@ int spider_db_fetch_for_item_sum_func( default: DBUG_RETURN(ER_SPIDER_COND_SKIP_NUM); } + spider->wide_handler->trx->direct_aggregate_count++; DBUG_RETURN(0); } #endif diff --git a/storage/spider/spd_db_mysql.cc b/storage/spider/spd_db_mysql.cc index 115cd1b16f9..51c20026cb1 100644 --- a/storage/spider/spd_db_mysql.cc +++ b/storage/spider/spd_db_mysql.cc @@ -554,11 +554,12 @@ SPIDER_DB_ROW *spider_db_mbase_row::clone() } else { row_size = record_size + field_count; } - if (!spider_bulk_malloc(spider_current_trx, 29, MYF(MY_WME), - &clone_row->row, (uint) (sizeof(char*) * field_count), - &tmp_char, (uint) (row_size), - &clone_row->lengths, (uint) (sizeof(ulong) * field_count), - NullS) + if (!spider_bulk_malloc( + spider_current_trx, 29, MYF(MY_WME), + &clone_row->row, (uint) (sizeof(char*) * (field_count + 1)), + &tmp_char, (uint) (row_size), + &clone_row->lengths, (uint) (sizeof(ulong) * field_count), + NullS) ) { delete clone_row; DBUG_RETURN(NULL); @@ -583,6 +584,7 @@ SPIDER_DB_ROW *spider_db_mbase_row::clone() tmp_lengths++; tmp_row++; } + clone_row->row[field_count] = NULL; clone_row->field_count = field_count; clone_row->record_size = record_size; clone_row->row_first = clone_row->row; @@ -741,6 +743,7 @@ SPIDER_DB_ROW *spider_db_mbase_result::fetch_row() } row.lengths = mysql_fetch_lengths(db_result); row.field_count = mysql_num_fields(db_result); + row.row[row.field_count] = NULL; row.row_first = row.row; row.lengths_first = row.lengths; row.record_size = 0; @@ -15145,7 +15148,6 @@ int spider_mbase_handler::show_records( DBUG_PRINT("info", ("spider error_num=%d", error_num)); DBUG_RETURN(error_num); } - spider->wide_handler->trx->direct_aggregate_count++; DBUG_RETURN(0); } diff --git a/storage/spider/spd_group_by_handler.cc b/storage/spider/spd_group_by_handler.cc index 69b3d21a051..e98c899ed31 100644 --- a/storage/spider/spd_group_by_handler.cc +++ b/storage/spider/spd_group_by_handler.cc @@ -1237,6 +1237,8 @@ int spider_group_by_handler::init_scan() spider->init_index_handler = FALSE; spider->use_spatial_index = FALSE; result_list->check_direct_order_limit = FALSE; + /* Disable direct aggregate when GBH is on (MDEV-29502). */ + result_list->direct_aggregate = FALSE; spider->select_column_mode = 0; spider->search_link_idx = fields->get_ok_link_idx(); spider->result_link_idx = spider->search_link_idx; diff --git a/storage/spider/spd_param.cc b/storage/spider/spd_param.cc index f9b232372ab..272ed53ddfc 100644 --- a/storage/spider/spd_param.cc +++ b/storage/spider/spd_param.cc @@ -2823,6 +2823,22 @@ static MYSQL_THDVAR_INT( SPIDER_THDVAR_OVERRIDE_VALUE_FUNC(int, strict_group_by) +/* + -1 : use table parameter + 0 : do not strict + 1 : do strict + */ +static MYSQL_THDVAR_BOOL( + direct_aggregate, /* name */ + PLUGIN_VAR_RQCMDARG, /* opt */ + "Whether to enable direct aggregate", + NULL, /* check */ + NULL, /* update */ + TRUE /* def */ +); + +SPIDER_THDVAR_VALUE_FUNC(bool, direct_aggregate) + static struct st_mysql_storage_engine spider_storage_engine = { MYSQL_HANDLERTON_INTERFACE_VERSION }; @@ -2978,6 +2994,7 @@ static struct st_mysql_sys_var* spider_system_variables[] = { MYSQL_SYSVAR(wait_timeout), MYSQL_SYSVAR(sync_sql_mode), MYSQL_SYSVAR(strict_group_by), + MYSQL_SYSVAR(direct_aggregate), NULL }; diff --git a/storage/spider/spd_param.h b/storage/spider/spd_param.h index c3a79cec065..6b786685ced 100644 --- a/storage/spider/spd_param.h +++ b/storage/spider/spd_param.h @@ -440,3 +440,4 @@ int spider_param_strict_group_by( THD *thd, int strict_group_by ); +bool spider_param_direct_aggregate(THD *thd); diff --git a/storage/spider/spd_table.cc b/storage/spider/spd_table.cc index 36948baabf0..3743a458ba7 100644 --- a/storage/spider/spd_table.cc +++ b/storage/spider/spd_table.cc @@ -8867,7 +8867,7 @@ bool spider_check_direct_order_limit( spider->result_list.direct_distinct = TRUE; } #ifdef HANDLER_HAS_DIRECT_AGGREGATE - spider->result_list.direct_aggregate = TRUE; + spider->result_list.direct_aggregate = spider_param_direct_aggregate(thd); #endif DBUG_PRINT("info",("spider select_limit=%lld", select_limit)); DBUG_PRINT("info",("spider offset_limit=%lld", offset_limit));