Timeout not passed to MySQL timeout about with_advisory_lock HOT 8 OPEN

@mceachen Hi Matthew, I was looking into this but have a few questions (First Time contributor, so bare with me :)
It looks like the pg_try_advisory method of PostgresSQL does not support timeouts, see the documentation.

One idea is to make the implementation of the yield_with_lock method specific per database, i.e. move it to the respective database files. In this case then we can set a timeout for MySQL, which seems to be supported (see https://dev.mysql.com/doc/refman/5.7/en/miscellaneous-functions.html#function_get-lock) . But it feels like we are loosing genericity and introducing a more complex implementation. Is there anything i miss here? Thanks for any help/input :)

from with_advisory_lock.

Howdy, @serenaf! Per @mceachen's recommendations, I added activerecord 5.1 and both flavors of MySQL (< 5.7.5 and >= 5.7.5) to the testing matrix. Would you be able to help craft a failing test so that we can get started on resolving?

from with_advisory_lock.

@danielventulieri sorry, I don't know. This gem is being maintained by @seuros at this point, and I'll defer to his response.

from with_advisory_lock.

Sure!

from with_advisory_lock.

Cleaning out my open issues.

from with_advisory_lock.

Reopening-- @relwell, you're not on the hook for this. Anyone can submit a PR for it.

from with_advisory_lock.

@serenaf thanks for looking into this. Your link to MySQL's fundamental change to get_lock (in a patch release, of course) is most unfortunate, and breaks this gem as it stands. So, good news that you highlighted that, bad news is there was something to highlight.

Just because Postgres doesn't support a timeout doesn't mean we can't support a timeout--we can do a polling retry with a fast-fail method (pg_try_advisory) for the duration of the timeout.

To be honest I'm not using ruby (or rails) for any projects (not for 4 years now!), and I've been supporting my ruby projects because there are too many abandoned libraries out there. If you'd like to take a stab at a PR, though, I'm happy to review it and help you get it merged, but there should be tests added to the matrix for MySQL < 5.7.5 and >= 5.7.5, and ActiveRecord 5.1 as well (given that is the currently supported version).

from with_advisory_lock.

Hi @mceachen. I know I'm 4 years late, but what do you mean by "breaks this gem as it stands"? I've tested locally with multiple threads trying to access the lock and everything looks fine. Is this gem safe to use in production?

from with_advisory_lock.