Let cache lock fail after 24h and warn after 2s #498
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's Guide by SourceryThis pull request introduces a timeout mechanism for acquiring folder locks in the cache, preventing indefinite waiting. It sets a default timeout of 24 hours and warns the user after 2 seconds if the lock cannot be acquired, suggesting manual deletion of the lock file if necessary. The Sequence diagram for acquiring a folder lock with timeout and warningsequenceDiagram
participant User
participant FolderLock
participant FileLock
User->>FolderLock: FolderLock(folders, timeout, warning_timeout)
FolderLock->>FileLock: SoftFileLock(lock_file)
User->>FolderLock: __enter__()
alt warning_timeout < timeout
FolderLock->>FileLock: acquire(timeout=warning_timeout)
alt Lock acquired within warning_timeout
FileLock-->>FolderLock: returns
FolderLock-->>User: returns
else Lock not acquired within warning_timeout
FileLock-->>FolderLock: Timeout
FolderLock->>User: issues warning
FolderLock->>FileLock: acquire(timeout=remaining_time)
FileLock-->>FolderLock: returns
FolderLock-->>User: returns
end
else timeout < warning_timeout
FolderLock->>FileLock: acquire(timeout=timeout)
FileLock-->>FolderLock: returns
FolderLock-->>User: returns
end
Updated class diagram for FolderLockclassDiagram
class FolderLock {
-folders: str | Sequence[str]
-timeout: float
-warning_timeout: float
-lock_files: list
-locks: list
+__init__(folders: str | Sequence[str], timeout: float, warning_timeout: float)
+__enter__() : FolderLock
+__exit__()
}
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @hagenw - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a brief explanation in the description why the default timeout was reduced to 24 hours.
- The warning message in
FolderLockis very long; consider shortening it or breaking it into multiple lines for better readability.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| accessing the database. If timeout is reached, ``None`` is | ||
| returned. If timeout < 0 the method will block until the | ||
| database can be accessed | ||
| timeout: maximum wait time in seconds |
There was a problem hiding this comment.
Would you mention what is being waited for? My understanding is that you want to acquire the lock yourself and are willing to wait for timeout before you give up.
There was a problem hiding this comment.
The full text is:
timeout: maximum wait time in seconds
if another thread or process is already
accessing the database.
If timeout is reached,
``None`` is returned
So, we are waiting that we can get access to the database. This needs to be blocked if another user is currently accessing it, as this means the cache might be not fully filled yet.
What would be your suggestion as additional text?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #497
This reduces the default waiting time for acquiring the folder lock in cache from infinity to 24 h (handled by the
timeoutargument inaudb.load(),audb.load_media(), andaudb.stream()). This seems to me a better approach as we have the problem of leftover lock files in shared cache, for whichaudb.load()was stuck forever before.It also displays a warning if the lock can not be acquired after 2 s, mentioning that the user might need to delete the lock file manually.
To test it locally, you can try:
This first returns after 2 s:
and after additional 8 s it fails with
Summary by Sourcery
Bug Fixes: