maria-developers team mailing list archive

[kentoku <kentokushiba@xxxxxxxxx>]

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