Commit 6651768a authored by Sergei Golubchik's avatar Sergei Golubchik

MDEV-35055 ASAN errors in TABLE_SHARE::lock_share upon committing transaction...

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.
parent 4881ab43
...@@ -97,6 +97,7 @@ delete from t1 where id=2; ...@@ -97,6 +97,7 @@ delete from t1 where id=2;
select release_all_locks(); select release_all_locks();
release_all_locks() release_all_locks()
1 1
disconnect con2;
connection default; connection default;
id d id d
6 0.93093 6 0.93093
...@@ -150,3 +151,15 @@ t CREATE TABLE `t` ( ...@@ -150,3 +151,15 @@ t CREATE TABLE `t` (
VECTOR KEY `v` (`v`) VECTOR KEY `v` (`v`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_uca1400_ai_ci ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_uca1400_ai_ci
drop table t; 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;
...@@ -90,6 +90,7 @@ delete from t1 where id=2; ...@@ -90,6 +90,7 @@ delete from t1 where id=2;
select release_all_locks(); select release_all_locks();
disconnect con2;
connection default; connection default;
--replace_regex /(\.\d{5})\d+/\1/ --replace_regex /(\.\d{5})\d+/\1/
reap; reap;
...@@ -132,3 +133,16 @@ alter table t add v blob not null, add vector index (v); ...@@ -132,3 +133,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); alter table t add v blob not null default x'00000000', add vector index (v);
show create table t; show create table t;
drop 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;
...@@ -18,6 +18,7 @@ ...@@ -18,6 +18,7 @@
#include <my_global.h> #include <my_global.h>
#include "key.h" // key_copy() #include "key.h" // key_copy()
#include "create_options.h" #include "create_options.h"
#include "table_cache.h"
#include "vector_mhnsw.h" #include "vector_mhnsw.h"
#include "item_vectorfunc.h" #include "item_vectorfunc.h"
#include <scope.h> #include <scope.h>
...@@ -490,11 +491,11 @@ class MHNSW_Share : public Sql_alloc ...@@ -490,11 +491,11 @@ class MHNSW_Share : public Sql_alloc
class MHNSW_Trx : public MHNSW_Share class MHNSW_Trx : public MHNSW_Share
{ {
public: public:
TABLE_SHARE *table_share; MDL_ticket *table_id;
bool list_of_nodes_is_lost= false; bool list_of_nodes_is_lost= false;
MHNSW_Trx *next= nullptr; 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 void reset(TABLE_SHARE *) override
{ {
node_cache.clear(); node_cache.clear();
...@@ -566,24 +567,38 @@ int MHNSW_Trx::do_commit(THD *thd, bool) ...@@ -566,24 +567,38 @@ int MHNSW_Trx::do_commit(THD *thd, bool)
trx; trx= trx_next) trx; trx= trx_next)
{ {
trx_next= trx->next; trx_next= trx->next;
auto ctx= MHNSW_Share::get_from_share(trx->table_share, nullptr); if (trx->table_id)
if (ctx)
{ {
mysql_rwlock_wrlock(&ctx->commit_lock); MDL_key *key= trx->table_id->get_key();
ctx->version++; LEX_CSTRING db= {key->db_name(), key->db_name_length()},
if (trx->list_of_nodes_is_lost) tbl= {key->name(), key->name_length()};
ctx->reset(trx->table_share); TABLE_LIST tl;
else 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 auto ctx= share->hlindex ? MHNSW_Share::get_from_share(share, nullptr)
// for ann_benchmarks it does not : nullptr;
// also, consider flushing only changed nodes (a flag in the node) if (ctx)
for (FVectorNode &from : trx->get_cache()) {
if (FVectorNode *node= ctx->find_node(from.gref())) mysql_rwlock_wrlock(&ctx->commit_lock);
node->vec= nullptr; ctx->version++;
ctx->start= nullptr; 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(); trx->~MHNSW_Trx();
} }
...@@ -601,7 +616,9 @@ MHNSW_Trx *MHNSW_Trx::get_from_thd(TABLE *table, bool for_update) ...@@ -601,7 +616,9 @@ MHNSW_Trx *MHNSW_Trx::get_from_thd(TABLE *table, bool for_update)
if (!for_update && !trx) if (!for_update && !trx)
return NULL; 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) if (!trx)
{ {
trx= new (&thd->transaction->mem_root) MHNSW_Trx(table); trx= new (&thd->transaction->mem_root) MHNSW_Trx(table);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment