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

lock on NFS mount not working #92

Closed
alcoat opened this issue Dec 5, 2023 · 10 comments
Closed

lock on NFS mount not working #92

alcoat opened this issue Dec 5, 2023 · 10 comments

Comments

@alcoat
Copy link

alcoat commented Dec 5, 2023

Hello,
I create this issue because it seems to me that portalocker does not work well with NFS mount. The problem was discovered while using cachier package : python-cachier/cachier#128

@shaypal5
Copy link

shaypal5 commented Dec 5, 2023

Hello there,

I'm the writer of the cachier package, and I'm just chiming in to add that this is the second time file locking on NFS mounts seems to be an issue for us as a user package. This the the first:
python-cachier/cachier#86

Thank you for the great package, and we hope to see this get fixed!

Shay

@wolph
Copy link
Owner

wolph commented Dec 6, 2023

There have been other reports regarding NFS in the past: #66

Locking with NFS is rather problematic, while it's not entirely impossible to do it, it absolutely kills your performance in my experience.
If at all possible I would highly recommend using Redis Locks instead: https://github.com/wolph/portalocker#redis-locks

Having that said, this specific issue seems to be an uncaught exception that I need to fix :)

@carnil
Copy link

carnil commented Jan 13, 2024

Can this be reopened? (should issues realy be closed when they are stale? I'm questioning this as the report is perfectly valid IMHO and just missing catched exception can still be implemented at least)

@wolph
Copy link
Owner

wolph commented Jan 13, 2024

Sorry about that, it seems I have misconfigured the stalebot.

@wolph wolph reopened this Jan 13, 2024
@wolph wolph removed the Stale label Jan 13, 2024
@carnil
Copy link

carnil commented Jan 13, 2024

Sorry about that, it seems I have misconfigured the stalebot.

Thank you for re-opening!

@rpmcginty
Copy link

@wolph Is there a solution being worked on? Or is the label just meant to avoid the stalebot actions?

@wolph
Copy link
Owner

wolph commented Jun 19, 2024

@rpmcginty I believe it's already fixed on dev but I haven't created a new release yet. I'm having issues with recreating so if you could test it, that would be fantastic!

@rpmcginty
Copy link

rpmcginty commented Jun 19, 2024

certainly! I installed via vcs url but was met with the same issue.

Here is a script I used to test. The working directory is on an AWS EFS file system (NFS)

import portalocker

with portalocker.Lock('test.lock', 'wb') as fh:
    print('wb Locked')
    fh.write('Locked\n'.encode())
print('wb Unlocked')

with portalocker.Lock('test.lock', 'r') as fh:
    print('r Locked')
    print(fh.read())
print('r Unlocked')

I received the following error

wb Locked
--
wb Unlocked
Traceback (most recent call last):
File "/init/test.py", line 12, in <module>
with portalocker.Lock('test.lock', 'rb') as fh:
File "/home/user/merlin_env/lib/python3.9/site-packages/portalocker/utils.py", line 163, in __enter__
return self.acquire()
File "/home/user/merlin_env/lib/python3.9/site-packages/portalocker/utils.py", line 290, in acquire
raise exceptions.LockException(exception)
portalocker.exceptions.LockException: [Errno 9] Bad file descriptor

It appears to fail only on the readonly locks. I should note that it happens for both 'r' and 'rb'

@wolph do we need to modify the usage at all?

@wolph
Copy link
Owner

wolph commented Jun 19, 2024

That looks like the current "fix" is at least effective in that it throws the correct exception now.

Beyond that I'm not sure what else I can do honestly. It appears that the filesystem doesn't support read only locks for this case.

For cases like these I created the RedisLock that work across multiple networked systems and is not limited by filesystems at all.

There's also a small chance that lockf as opposed to flock works in this case, it can be changed by overriding the LOCKER attribute:

LOCKER = fcntl.flock

It's should be noted that different systems behave differently with lockf (Linux vs bsd for example)

@wolph
Copy link
Owner

wolph commented Jun 22, 2024

I've published a new release with the fix that gives the correct exceptions. I'm not sure if there's a better fix available here... the underlying filesystem simply doesn't support these locks so there's little else we can do.

A nice and safe option could be to use Redis locks, those always work: https://github.com/wolph/portalocker/?tab=readme-ov-file#redis-locks

@wolph wolph closed this as completed Jun 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants