Re: [Maria-developers] Rev 3966: MDEV-5492 - Reduce usage of LOCK_open: TABLE::in_use
Hi, Sergey! On Jan 29, Sergey Vojtovich wrote:
At lp:maria/10.0
------------------------------------------------------------ revno: 3966 revision-id: svoj@mariadb.org-20140129083115-jwye7zfyncrq9crs parent: svoj@mariadb.org-20140124075326-d7twer07e0bu42b5 committer: Sergey Vojtovich <svoj@mariadb.org> branch nick: 10.0 timestamp: Wed 2014-01-29 12:31:15 +0400 message: MDEV-5492 - Reduce usage of LOCK_open: TABLE::in_use
Move TABLE::in_use out of LOCK_open.
This is done with assumtion that foreign threads accessing TABLE::in_use will only need consistent value _after_ marking table for flush and purging unused table instances. In this case TABLE::in_use will always point to a valid thread object.
I believe I was able to understand what you were doing here :) Seems ok. The assumption above looks a bit risky without any way to enforce it. Perhaps you could make in_use private (renaming to, say, m_in_use) and add an inline getter method TABLE::in_use() that in debug builds will assert that your assumption holds? Regards, Sergei
Hi Sergei, On Wed, Jan 29, 2014 at 10:33:55PM +0100, Sergei Golubchik wrote:
Hi, Sergey!
On Jan 29, Sergey Vojtovich wrote:
At lp:maria/10.0
------------------------------------------------------------ revno: 3966 revision-id: svoj@mariadb.org-20140129083115-jwye7zfyncrq9crs parent: svoj@mariadb.org-20140124075326-d7twer07e0bu42b5 committer: Sergey Vojtovich <svoj@mariadb.org> branch nick: 10.0 timestamp: Wed 2014-01-29 12:31:15 +0400 message: MDEV-5492 - Reduce usage of LOCK_open: TABLE::in_use
Move TABLE::in_use out of LOCK_open.
This is done with assumtion that foreign threads accessing TABLE::in_use will only need consistent value _after_ marking table for flush and purging unused table instances. In this case TABLE::in_use will always point to a valid thread object.
I believe I was able to understand what you were doing here :) Seems ok.
The assumption above looks a bit risky without any way to enforce it.
Perhaps you could make in_use private (renaming to, say, m_in_use) and add an inline getter method TABLE::in_use() that in debug builds will assert that your assumption holds? Like DBUG_ASSERT(table->in_use == current_thd || (table->s->flushed && table->in_use))? I would gladly do that, but that's quite big change which will also affect many storage engines, at least federated, maria, myisam, sphinx, oqgraph, connect, merge, spider, cassandra, tokudb.
If it isn't a problem I'd prefer to create a separate patch. Thanks, Sergey
Hi, Sergey! On Jan 30, Sergey Vojtovich wrote:
The assumption above looks a bit risky without any way to enforce it.
Perhaps you could make in_use private (renaming to, say, m_in_use) and add an inline getter method TABLE::in_use() that in debug builds will assert that your assumption holds? Like DBUG_ASSERT(table->in_use == current_thd || (table->s->flushed && table->in_use))?
Exactly.
I would gladly do that, but that's quite big change which will also affect many storage engines, at least federated, maria, myisam, sphinx, oqgraph, connect, merge, spider, cassandra, tokudb.
Hmm, indeed. connect, tokudb, merge, federated, cassandra, sphinx, and, partially, other engines, should use ha_thd() handler's method instead of table->in_use. Still, spider, maria, myisam, oqgraph cannot always do that, and have to use table->in_use.
If it isn't a problem I'd prefer to create a separate patch.
Of course. Regards, Sergei
participants (2)
-
Sergei Golubchik -
Sergey Vojtovich