From 09889d417ba3a907daa550d9c382157a4dabc569 Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Tue, 1 Oct 2024 16:01:38 +0200 Subject: [PATCH] MDEV-35055 ASAN errors in TABLE_SHARE::lock_share upon committing transaction after FLUSH on table with vector key MHNSW_Trx cannot store a pointer to the TABLE_SHARE for the duration of a transaction, because the share can be evicted from the cache any time. Use a pointer to the MDL_ticket instead, it is stored in the THD and has a duration of MDL_TRANSACTION, so won't go away. When we need a TABLE_SHARE - on commit - get a new one from tdc. Normally, it'll be already in the cache, so it'd be fast. We don't optimize for the edge case when TABLE_SHARE was evicted. --- mysql-test/main/vector_innodb.result | 13 +++++++ mysql-test/main/vector_innodb.test | 14 ++++++++ sql/vector_mhnsw.cc | 53 ++++++++++++++++++---------- 3 files changed, 62 insertions(+), 18 deletions(-) diff --git a/mysql-test/main/vector_innodb.result b/mysql-test/main/vector_innodb.result index bf08d37b751..a2cd18e08af 100644 --- a/mysql-test/main/vector_innodb.result +++ b/mysql-test/main/vector_innodb.result @@ -97,6 +97,7 @@ delete from t1 where id=2; select release_all_locks(); release_all_locks() 1 +disconnect con2; connection default; id d 6 0.93093 @@ -150,3 +151,15 @@ t CREATE TABLE `t` ( VECTOR KEY `v` (`v`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_uca1400_ai_ci drop table t; +# +# MDEV-35055 ASAN errors in TABLE_SHARE::lock_share upon committing transaction after FLUSH on table with vector key +# +create table t (pk int primary key, v blob not null, vector index(v)) engine=innodb; +start transaction; +insert into t values (1,vec_fromtext('[1,2,3]')); +connect con1,localhost,root; +flush tables; +disconnect con1; +connection default; +commit; +drop table t; diff --git a/mysql-test/main/vector_innodb.test b/mysql-test/main/vector_innodb.test index a44c15340f7..6dd4271992e 100644 --- a/mysql-test/main/vector_innodb.test +++ b/mysql-test/main/vector_innodb.test @@ -94,6 +94,7 @@ delete from t1 where id=2; select release_all_locks(); --enable_ps2_protocol +disconnect con2; connection default; --replace_regex /(\.\d{5})\d+/\1/ reap; @@ -136,3 +137,16 @@ alter table t add v blob not null, add vector index (v); alter table t add v blob not null default x'00000000', add vector index (v); show create table t; drop table t; + +--echo # +--echo # MDEV-35055 ASAN errors in TABLE_SHARE::lock_share upon committing transaction after FLUSH on table with vector key +--echo # +create table t (pk int primary key, v blob not null, vector index(v)) engine=innodb; +start transaction; +insert into t values (1,vec_fromtext('[1,2,3]')); +connect con1,localhost,root; +flush tables; +disconnect con1; +connection default; +commit; +drop table t; diff --git a/sql/vector_mhnsw.cc b/sql/vector_mhnsw.cc index 9d53ad685b9..e880a7298b4 100644 --- a/sql/vector_mhnsw.cc +++ b/sql/vector_mhnsw.cc @@ -18,6 +18,7 @@ #include #include "key.h" // key_copy() #include "create_options.h" +#include "table_cache.h" #include "vector_mhnsw.h" #include "item_vectorfunc.h" #include @@ -490,11 +491,11 @@ public: class MHNSW_Trx : public MHNSW_Share { public: - TABLE_SHARE *table_share; + MDL_ticket *table_id; bool list_of_nodes_is_lost= false; MHNSW_Trx *next= nullptr; - MHNSW_Trx(TABLE *table) : MHNSW_Share(table), table_share(table->s) {} + MHNSW_Trx(TABLE *table) : MHNSW_Share(table), table_id(table->mdl_ticket) {} void reset(TABLE_SHARE *) override { node_cache.clear(); @@ -566,24 +567,38 @@ int MHNSW_Trx::do_commit(THD *thd, bool) trx; trx= trx_next) { trx_next= trx->next; - auto ctx= MHNSW_Share::get_from_share(trx->table_share, nullptr); - if (ctx) + if (trx->table_id) { - mysql_rwlock_wrlock(&ctx->commit_lock); - ctx->version++; - if (trx->list_of_nodes_is_lost) - ctx->reset(trx->table_share); - else + MDL_key *key= trx->table_id->get_key(); + LEX_CSTRING db= {key->db_name(), key->db_name_length()}, + tbl= {key->name(), key->name_length()}; + TABLE_LIST tl; + tl.init_one_table(&db, &tbl, nullptr, TL_IGNORE); + TABLE_SHARE *share= tdc_acquire_share(thd, &tl, GTS_TABLE, nullptr); + if (share) { - // consider copying nodes from trx to shared cache when it makes sense - // for ann_benchmarks it does not - // also, consider flushing only changed nodes (a flag in the node) - for (FVectorNode &from : trx->get_cache()) - if (FVectorNode *node= ctx->find_node(from.gref())) - node->vec= nullptr; - ctx->start= nullptr; + auto ctx= share->hlindex ? MHNSW_Share::get_from_share(share, nullptr) + : nullptr; + if (ctx) + { + mysql_rwlock_wrlock(&ctx->commit_lock); + ctx->version++; + if (trx->list_of_nodes_is_lost) + ctx->reset(share); + else + { + // consider copying nodes from trx to shared cache when it makes + // sense. for ann_benchmarks it does not. + // also, consider flushing only changed nodes (a flag in the node) + for (FVectorNode &from : trx->get_cache()) + if (FVectorNode *node= ctx->find_node(from.gref())) + node->vec= nullptr; + ctx->start= nullptr; + } + ctx->release(true, share); + } + tdc_release_share(share); } - ctx->release(true, trx->table_share); } trx->~MHNSW_Trx(); } @@ -601,7 +616,9 @@ MHNSW_Trx *MHNSW_Trx::get_from_thd(TABLE *table, bool for_update) if (!for_update && !trx) return NULL; - while (trx && trx->table_share != table->s) trx= trx->next; + DBUG_ASSERT(!table->mdl_ticket || + table->mdl_ticket->m_duration == MDL_TRANSACTION); + while (trx && trx->table_id != table->mdl_ticket) trx= trx->next; if (!trx) { trx= new (&thd->transaction->mem_root) MHNSW_Trx(table);