Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature suggestion - region.is_key_locked(key) #101

Closed
sqlalchemy-bot opened this issue Jun 20, 2016 · 8 comments
Closed

feature suggestion - region.is_key_locked(key) #101

sqlalchemy-bot opened this issue Jun 20, 2016 · 8 comments
Labels

Comments

@sqlalchemy-bot
Copy link

Migrated issue, originally created by jvanasco (jvanasco)

I wavered on proposing this for a few days, and am leaning towards this idea.

It would be useful for those who are dealing with expensive value generators to query a region and see if an active dogpile lock exists for a given key. (which would then need to be implemented on the backends as well, returning a boolean or perhaps the seconds remaining)

the rationale is that expensive/long-running generators still result in a pileup of processes that are just waiting for a bottleneck to passes. in some known instances, you may want to handle this.

There are other ways around this, I'm using a few myself. This just seems like it would be useful.

@sqlalchemy-bot
Copy link
Author

Michael Bayer (zzzeek) wrote:

i think it should be possible now that dogpile core is merged into cache it would be just one patch.

@sqlalchemy-bot
Copy link
Author

jvanasco (jvanasco) wrote:

I don't think this would even hit the dogpile.core files. I think everything would be on cache's region.py and the backends themselves.

I wanted to bring this up before considering a PR.

@sqlalchemy-bot sqlalchemy-bot added the bug Something isn't working label Nov 24, 2018
@zzzeek zzzeek added feature and removed bug Something isn't working labels Nov 25, 2018
@bagerard
Copy link
Contributor

We are also very interested in such feature, @zzzeek ticket is quite old, will you accept a PR if we open one?

@zzzeek
Copy link
Member

zzzeek commented Jan 22, 2021

IIUC this would be :

   def key_is_locked(self, key):
        mutex = self._mutex(key)
        if not mutex.acquire(wait=False):
            return True
        else:
            mutex.release()
            return False

is that it?

@bagerard
Copy link
Contributor

bagerard commented Jan 22, 2021

That is one way to get that information but I was wondering if we could check if a lock exist without actually acquiring it (as it may clash with actual other process trying to acquire it to do the work, unless it is handling that gracefully already) but maybe that is not possible or complicated to be supported/implemented on all backends.

To give a concrete example, we've noticed that for redis, when we acquire a lock for a given key, it creates a record with "lock_{our_key}" in redis so in that case we can look if that key exist as a way to know if a lock exist. That is a bit of a hack but perhaps there is a better way to achieve this

If this can't be done, I agree that what you provided above would already be a step forward and something we could exploit for our use case

@zzzeek
Copy link
Member

zzzeek commented Jan 22, 2021

I would invite you to look at the implementation in Region. If given a normal Python threading.Lock(), there's an object called _LockWrapper that's placed around it. If the mutex is provided by the backend, the interface is now the dogpile.cache.api.CacheMutex object. So what you would want is that CacheMutex has a non-invasive way of checking if the lock is currently acquired.

The good news that I just saw is that threading.Lock() appears to have a locked() method on it:

https://docs.python.org/3/library/threading.html#threading.Lock.locked

Seems to be in Python 2 even, I seem to recall that this method was not available but I guess the last time I would have looked was in a much older version of Python.

based on this we can alter the method to look like this:

   def key_is_locked(self, key):
        mutex = self._mutex(key)
        return mutex.locked()

which would include the requisite methods being added to CacheMutex, and if some third party backend didn't support it it would just raise NotImplementedError.

seems simple how does that work for you?

@sqla-tester
Copy link
Collaborator

Bastien Gerard has proposed a fix for this issue in the master branch:

Add feature region.key_is_locked() https://gerrit.sqlalchemy.org/c/sqlalchemy/dogpile.cache/+/2508

1 similar comment
@sqla-tester
Copy link
Collaborator

Bastien Gerard has proposed a fix for this issue in the master branch:

Add feature region.key_is_locked() https://gerrit.sqlalchemy.org/c/sqlalchemy/dogpile.cache/+/2508

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants