Re: [Maria-developers] 23b70146c16: MDEV-6268 SPIDER table with no COMMENT clause causes queries to wait forever
Hi, Kentoku! On Jul 04, Kentoku wrote:
revision-id: 23b70146c16 (mariadb-10.4.5-21-g23b70146c16) parent(s): 24773bf3802 author: Kentoku <kentokushiba@gmail.com> committer: Kentoku <kentokushiba@gmail.com> timestamp: 2019-06-10 13:59:18 +0900 message:
MDEV-6268 SPIDER table with no COMMENT clause causes queries to wait forever Add mysql_mutex_timedlock() and add the following parameter to Spider - spider_internal_lock_wait_timeout The timeout when Spider tries to get internal locks. 0 or more : the tomeout. (second) The default value is -1
please, don't forget to leave an empty line after the first line in a commit comment. That's how various git tools (github included) separate "commit subject" from the "commit comment body". Git itself doesn't care, but it's a convention that some git-related tools rely on. Now, about the fix. Svoj made a general obervation on Slack that it's much better to eliminate or detect deadlocks, instead of wait them out with timeouts. And here we were able to find a rather cheap solution that would avoid such deadlocks in Spider. Here, how it can work: * On startup Spider calculates the unique spider instance ID by combining my_gethwaddr() and getpid(). With delimiters it might look something like "-d46d6d7345c9:23050-". This is done only once on startup. * When Spider needs to open a table, it checks whether this THD has a user variable named `spider_parents` and it it does, whether this variable contains the above mentioned unique spider instance ID. This is just one strstr(), so fast, no complex parsing. If the string is found, Spider returns an error, doesn't open a table. * When Spider connects to another host now it sends show table status from `test` like 'user' in the new approach it'll send set @spider_parents="${its current spider_parents}-d46d6d7345c9:23050-"; show table status from `test` like 'user' this is all in one packet, so no additional round-trips. Here `${its current spider_parents}` is the value of @spider_parents on the connecting host. This is it, it'll prevent self-connects, both direct and indirect via a chain of spider tables. Because @spider_parents variable is user-visible, better to use a different name for it, something that has a smaller chance of causing a conflict. May be @insternal_spider_deadlock_prevention_parent_list :) Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
Hi Sergei,
please, don't forget to leave an empty line after the first line in a commit comment. That's how various git tools (github included) separate "commit subject" from the "commit comment body".
Git itself doesn't care, but it's a convention that some git-related tools rely on.
I'm sorry about that. This commit was made before you mention and I understood this.
Now, about the fix.
Svoj made a general obervation on Slack that it's much better to eliminate or detect deadlocks, instead of wait them out with timeouts.
And here we were able to find a rather cheap solution that would avoid such deadlocks in Spider. Here, how it can work:
* On startup Spider calculates the unique spider instance ID by combining my_gethwaddr() and getpid(). With delimiters it might look something like "-d46d6d7345c9:23050-". This is done only once on startup.
* When Spider needs to open a table, it checks whether this THD has a user variable named `spider_parents` and it it does, whether this variable contains the above mentioned unique spider instance ID. This is just one strstr(), so fast, no complex parsing. If the string is found, Spider returns an error, doesn't open a table.
* When Spider connects to another host now it sends
show table status from `test` like 'user'
in the new approach it'll send
set @spider_parents="${its current spider_parents}-d46d6d7345c9:23050-"; show table status from `test` like 'user'
this is all in one packet, so no additional round-trips. Here `${its current spider_parents}` is the value of @spider_parents on the connecting host.
This is it, it'll prevent self-connects, both direct and indirect via a chain of spider tables.
Because @spider_parents variable is user-visible, better to use a different name for it, something that has a smaller chance of causing a conflict. May be @insternal_spider_deadlock_prevention_parent_list :)
Thank you for thinking about it. I try to do it with some arranging. Best Regards, Kentoku 2019年7月5日(金) 2:11 Sergei Golubchik <serg@mariadb.org>:
Hi, Kentoku!
On Jul 04, Kentoku wrote:
revision-id: 23b70146c16 (mariadb-10.4.5-21-g23b70146c16) parent(s): 24773bf3802 author: Kentoku <kentokushiba@gmail.com> committer: Kentoku <kentokushiba@gmail.com> timestamp: 2019-06-10 13:59:18 +0900 message:
MDEV-6268 SPIDER table with no COMMENT clause causes queries to wait forever Add mysql_mutex_timedlock() and add the following parameter to Spider - spider_internal_lock_wait_timeout The timeout when Spider tries to get internal locks. 0 or more : the tomeout. (second) The default value is -1
please, don't forget to leave an empty line after the first line in a commit comment. That's how various git tools (github included) separate "commit subject" from the "commit comment body".
Git itself doesn't care, but it's a convention that some git-related tools rely on.
Now, about the fix.
Svoj made a general obervation on Slack that it's much better to eliminate or detect deadlocks, instead of wait them out with timeouts.
And here we were able to find a rather cheap solution that would avoid such deadlocks in Spider. Here, how it can work:
* On startup Spider calculates the unique spider instance ID by combining my_gethwaddr() and getpid(). With delimiters it might look something like "-d46d6d7345c9:23050-". This is done only once on startup.
* When Spider needs to open a table, it checks whether this THD has a user variable named `spider_parents` and it it does, whether this variable contains the above mentioned unique spider instance ID. This is just one strstr(), so fast, no complex parsing. If the string is found, Spider returns an error, doesn't open a table.
* When Spider connects to another host now it sends
show table status from `test` like 'user'
in the new approach it'll send
set @spider_parents="${its current spider_parents}-d46d6d7345c9:23050-"; show table status from `test` like 'user'
this is all in one packet, so no additional round-trips. Here `${its current spider_parents}` is the value of @spider_parents on the connecting host.
This is it, it'll prevent self-connects, both direct and indirect via a chain of spider tables.
Because @spider_parents variable is user-visible, better to use a different name for it, something that has a smaller chance of causing a conflict. May be @insternal_spider_deadlock_prevention_parent_list :)
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
participants (2)
-
kentoku -
Sergei Golubchik