Re: [Maria-developers] 23b70146c16: MDEV-6268 SPIDER table with no COMMENT clause causes queries to wait forever

2019-07-04 Thread kentoku
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 :
>
> Hi, Kentoku!
>
> On Jul 04, Kentoku wrote:
> > revision-id: 23b70146c16 (mariadb-10.4.5-21-g23b70146c16)
> > parent(s): 24773bf3802
> > author: Kentoku 
> > committer: Kentoku 
> > 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 secur...@mariadb.org

___
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help   : https://help.launchpad.net/ListHelp


Re: [Maria-developers] 23b70146c16: MDEV-6268 SPIDER table with no COMMENT clause causes queries to wait forever

2019-07-04 Thread Sergei Golubchik
Hi, Kentoku!

On Jul 04, Kentoku wrote:
> revision-id: 23b70146c16 (mariadb-10.4.5-21-g23b70146c16)
> parent(s): 24773bf3802
> author: Kentoku 
> committer: Kentoku 
> 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 secur...@mariadb.org

___
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help   : https://help.launchpad.net/ListHelp