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.

Sign up for GitHub

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

Jump to bottom

flufl.lock sleeps random amount of time #2860

Closed
efiop opened this issue Nov 28, 2019 · 3 comments
Closed

flufl.lock sleeps random amount of time #2860

efiop opened this issue Nov 28, 2019 · 3 comments
Labels
bug Did we break something? p3-nice-to-have It should be done this or next sprint research ui user interface / interaction

Comments

@efiop
Copy link
Contributor

efiop commented Nov 28, 2019

https://gitlab.com/warsaw/flufl.lock/blob/master/flufl/lock/_lockfile.py#L274 Happens whenever lock is taken. So this results in dvc freezeing at the begging of any operation, when it checks the updater lock. Related to #2831 . Need to consider bringing back zc.lockfile as described there and only falling back to flufl when flock() is not working.
@efiop efiop added bug Did we break something? ui user interface / interaction p1-important Important, aka current backlog of things to do labels Nov 28, 2019
@weekly-digest weekly-digest bot mentioned this issue Dec 1, 2019
Closed
@efiop efiop mentioned this issue Dec 8, 2019
Merged
3 tasks
efiop added a commit to efiop/dvc that referenced this issue Dec 13, 2019
@efiop
dvc: make flufl.lock opt-in and use zc.lockfile
25807f2 
As it turned out (see issue numbers down below), we can't really take
hardlinks for granted, so `flufl.lock` is not a panacea for all
filesystems. Considering that the vast majority of filesystems that our
users use support `zc.lockfile`(flock-based) and it has benefits like
more reliable mechanism, auto-delete when process exits, more sturdy
implementation, etc, it makes more sense to bring it back and use by
default again. For filesystems that don't support `flock()`, users will
be able to manually enable `flufl.lock` use through the config option.
It would be ideal if we could auto-detect that flock is not supported,
but in the real world, it turned out to be non-trivial, as it might hang
forever in a kernel context, which makes the implementation way too
complex for our purposes. So what we're doing instead is showing a
message before locking with `zc.lockfile` that, under normal
circumstances will disappear once the lock is taken or failed, otherwise
it will point users to the related documentation where they can learn
about how to opt-in for `flufl.lock`.

Fixes iterative#2831
Fixes iterative#2897
Related iterative#2860
@Suor
Copy link
Contributor

Suor commented Jan 18, 2020 •
edited
Loading

Is this still important or actually an issue anymore?

@efiop efiop added p2-medium Medium priority, should be done, but less important research and removed p1-important Important, aka current backlog of things to do labels Jan 19, 2020
@efiop
Copy link
Contributor Author

efiop commented Jan 19, 2020

@Suor Good point. It is an issue to investigate, but no longer p1, since we are not using flufl by default anymore.

@efiop efiop added p3-nice-to-have It should be done this or next sprint and removed p2-medium Medium priority, should be done, but less important labels Jan 19, 2020
@efiop
Copy link
Contributor Author

efiop commented Dec 4, 2020

Closing for now, since this is no longer relevant.

@efiop efiop closed this as completed Dec 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Did we break something? p3-nice-to-have It should be done this or next sprint research ui user interface / interaction
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Suor @efiop