-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
memoize: add user feedback and configurable timeout for flock #9262
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
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughModifies the memoization cache file locking mechanism in a single script to replace immediate blocking locks with non-blocking, timed locks featuring configurable retry intervals and maximum wait durations, with periodic status alerts during the waiting period. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
When the memoize cache lock is held by another process (e.g., a stale Docker container from a previous interrupted build), the build would hang indefinitely without any feedback to the user. This change: - First tries non-blocking flock, acquiring immediately if available - If lock is busy, informs user and waits with periodic status messages - Adds MEMOIZE_FLOCK_WAIT_INTERVAL (default 10s) for message frequency - Adds MEMOIZE_FLOCK_MAX_WAIT (default 0=infinite) for optional timeout - Allows user to interrupt with Ctrl+C - Suggests checking for stale containers: docker ps -a | grep armbian 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
2cd603a to
a968504
Compare
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @iav. * #9262 (comment) The following files were modified: * `lib/functions/general/memoize-cached.sh`
|
While this is nice addition... I'd also like to understand how "When a build is interrupted..., Docker containers may continue running and hold flock locks on memoize cache" happens, and maybe handle it too (add a cleanup handler or such...)? |
I encounter this quite rarely, and it happens during multiple manual starts and manual interruptions of the build process. I decided to investigate the situation after my Armbian build suddenly and strangely froze twice within a week. I spent almost an hour trying to figure out what was going on. It was working just a moment ago, and now it gets to a certain point and just freezes, WTF!!! And I can't figure out how to fix it! However, I don't think something like this could happen anywhere on runners. That is, I believe this situation could only arise with human intervention, and therefore, warning that person about what exactly happened is quite sufficient. Another reason not to automate the cleanup is that I don't know what else could cause such a blockage, so I don't want to break something I have no idea about. |
rpardini
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, this doesn't hurt, and unless MEMOIZE_FLOCK_MAX_WAIT is set, doesn't change anything.
Later, we could investigate why the hang further (probably on the remote call to obtain some version, we could call the memoized function with timeout ...) and maybe in this same spot add a cleanup handler (equivalent to using a bash trap).
|
✅ This PR has been reviewed and approved — all set for merge! |
|
If it turns out that long blockages are caused by something other than manual build interruptions, then this could be investigated. |
Summary
MEMOIZE_FLOCK_MAX_WAITenvironment variableProblem
When a build is interrupted (e.g., via Ctrl+C or system crash), Docker containers may continue running and hold flock locks on memoize cache files in
cache/memoize/. Subsequent builds would hang indefinitely at the "artifact [ kernel :: kernel() ]" stage without any feedback to the user.Solution
Instead of blocking indefinitely on
flock, this change:flock -n- acquires immediately if availabledocker ps -a | grep armbianNew Configuration Variables
MEMOIZE_FLOCK_WAIT_INTERVALMEMOIZE_FLOCK_MAX_WAITTest plan
MEMOIZE_FLOCK_MAX_WAIT=30and test timeout behavior🤖 Generated with Claude Code