Use Docker time limit for max lock age #4747
Conversation
|
I was looking into this recently and I had the same doubts, the timing didn't seem right, it only prevents users to trigger multiple builds of the same version at the same time (as the lock if release in only 30 seconds). I like more this timing. We may have to considerer users that push to master fix after fix (maybe trying to fix the build time or a typo), those users will get an error. I think this error We should change the message (or, if I read correctly, is rtd the one who would retry to build again?) |
|
Yea, we reschedule the task: https://github.com/rtfd/readthedocs.org/blob/master/readthedocs/projects/tasks.py#L403 |
When building a project, if it tooks more than `REPO_LOCK_SECONDS` and while building after that time another build is triggered for the same Version and the same builder takes the task the lock will be considered "old" and remove and taken by the new build. This will end up in a collision when accessing the files and it could raise an exception like `IOError: [Errno 26] Text file busy`. Also, it could fail with another unexpected reasons. This PR increases the `max_lock_age` to the same value assigned for the project to end the build in order: * custom container time limit or, * `settings.DOCKER_LIMITS['time']` or, * `settings.REPO_LOCK_SECONDS` or, * 30 seconds Related to #1609
humitos
force-pushed
the
humitos/build/lock-timeout
branch
from
b07c3a3 to
a6c2481
Compare
In production is 300, by the way. This setting is override. |
|
@stsewd are you OK by merging this PR or do you think there is something else missing? |
readthedocs/projects/models.py
Outdated
| return NonBlockingLock(project=self, version=version, max_lock_age=max_lock_age) | ||
| def repo_nonblockinglock(self, version, max_lock_age=None): | ||
| """ | ||
| Return a ``NonBlockingLock`` or raise an exception. |
There was a problem hiding this comment.
I don't think we raise an exception here, it comes from the context manager.
|
I think we are good, I just left a comment on the docstring |
When building a project, if it tooks more than
REPO_LOCK_SECONDSand while building after that time another build is triggered for the same Version and the same builder takes the task the lock will be considered "old" and remove and taken by the new build.This will end up in a collision when accessing the files and it could raise an exception like
IOError: [Errno 26] Text file busy. Also, it could fail with another unexpected reasons.This PR increases the
max_lock_ageto the same value assigned for the project to end the build in order:settings.DOCKER_LIMITS['time']or,settings.REPO_LOCK_SECONDSor,Related to #1609