Sergei, On Wed, Jun 2, 2021 at 9:06 PM Sergei Golubchik <[email protected]> wrote: > > Hi, Aleksey! > > On Jun 02, Aleksey Midenkov wrote: > > > > > > > > > > > > > > > > - if (!(sql_lock= (MYSQL_LOCK*) > > > > > > > > - my_malloc(key_memory_MYSQL_LOCK, sizeof(*sql_lock) + > > > > > > > > - > > > > > > > > sizeof(THR_LOCK_DATA*)*((a->lock_count+b->lock_count)*2) + > > > > > > > > - > > > > > > > > sizeof(TABLE*)*(a->table_count+b->table_count),MYF(MY_WME)))) > > > > > > > > - DBUG_RETURN(0); // Fatal error > > > > > > > > + const size_t lock_size= sizeof(*sql_lock) + > > > > > > > > + sizeof(THR_LOCK_DATA *) * ((a->lock_count + b->lock_count) > > > > > > > > * 2) + > > > > > > > > + sizeof(TABLE *) * (a->table_count + b->table_count); > > > > > > > > + if (thd) > > > > > > > > + { > > > > > > > > + sql_lock= (MYSQL_LOCK *) thd->alloc(lock_size); > > > > > > > > + if (!sql_lock) > > > > > > > > + DBUG_RETURN(0); > > > > > > > > + sql_lock->flags= GET_LOCK_ON_THD; > > > > > > > > > > > Sorry, I meant the moment when MDEV-23639 calls mysql_lock_merge(). > > > > Yes, that is the part of the bug fix. > > > > > > What did it fix and how? > > > > > That's not about changing the method, that's about merging locks. When > > I merged my locks with thd there were already thd-allocated locks. > > And? How did allocating locks on THD fix anything?
LTM_PRELOCKED had locks on thd, I keep merged locks on thd. Isn't that enough for you? Also freeing was impossible for locks on thd. > > > > > > > > > + /* > > > > > > > > + NOTE: The semantics of vers_set_hist_part() is double: > > > > > > > > > > > > > > twofold > > > > > > > > > > Do you really believe in that butter butterish? :) I mean do we > > > > need to discuss all sorts of butter? That level of white noise > > > > should be ignored. > > > > > > I'm just trying to avoid incorrect usage of a language. > > > > I wish that threshold of incorrectness would be more relaxed. > > Sure. If any authoritative source will show that both words are correct > in this context, I'll readily admit that I'm wrong and will remember it > for the future. I didn't comment on a whim, it wasn't a matter of taste, > it was as far as I know an actual incorrect usage of words, an error. Go ahead and play that game again. > > > > > > > > > + table_list->vers_skip_create= false; > > > > > > > > + ot_ctx->vers_create_count= 0; > > > > > > > > + action= Open_table_context::OT_REOPEN_TABLES; > > > > > > > > + table_arg= NULL; > > > > > > > > + } > > > > > > > > > > > > > > I'm afraid I don't understand. All this business with > > > > > > > vers_skip_create and vers_skip_auto_create, it wasn't in the > > > > > > > previous version of the patch. So, I believe it was a fix > > > > > > > for one of the MDEV bugs reported and fixed meanwhile. > > > > > > > > > > > > No, that was the multi-threaded case which worked good for me, > > > > > > but suddenly I discovered it fails on some buildbot. > > > > > > > > > > Could you elaborate on what the problem was? Two threads trying > > > > > to add the partition in parallel? I'd expect MDL_EXCLUSIVE to > > > > > prevent that. > > > > > > > > MDL_EXCLUSIVE prevents parallel execution of > > > > repair_from_failed_open(), but not sequential. So it can add > > > > several partitions instead of 1, one after another. > > > > > > What's the sequence of events? One thread decides to add a > > > partition, takes an MDL_EXCLUSIVE, the other thread decides to add a > > > partition, waits for MDL_EXCLUSIVE, the first one finishes adding a > > > partition, releases the lock, the second grabs it and adds a second > > > partition? > > > > Right. > > Okay. Then, why a thread didn't check the number of partitions after > acquiring MDL_EXCLUSIVE? Like with mutexes, if there's a mutex that > protects a shared variable, you first acquire the mutex, then read the > variable's value. Not the other way around. Number of partitions is not a shared variable. part_info is kept in TABLE instance. To get new value it must reacquire share, reparse part_sql string. Then comparing with what? After releasing MDL_SHARED_WRITE everything is gone: TABLE, TABLE_SHARE. You must store somewhere old value, presumably in Open_table_context. > > Regards, > Sergei > VP of MariaDB Server Engineering > and [email protected] -- All the best, Aleksey Midenkov @midenok _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
