Skip to content

Commit

Permalink
Use Docker time limit for max lock age
Browse files Browse the repository at this point in the history
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
  • Loading branch information
humitos committed Oct 11, 2018
1 parent c01e8eb commit a6c2481
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 13 deletions.
24 changes: 22 additions & 2 deletions readthedocs/projects/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -651,8 +651,28 @@ def vcs_repo(self, version=LATEST, environment=None):
repo = backend(self, version, environment)
return repo

def repo_nonblockinglock(self, version, max_lock_age=5):
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.
:param version: project's version that want to get the lock for.
:param max_lock_age: time (in seconds) to consider the lock's age is old
and grab it anyway. It default to the ``container_time_limit`` of
the project or the default ``DOCKER_LIMITS['time']`` or
``REPO_LOCK_SECONDS`` or 30
"""
if max_lock_age is None:
max_lock_age = (
self.container_time_limit or
getattr(settings, 'DOCKER_LIMITS', {}).get('time') or
getattr(settings, 'REPO_LOCK_SECONDS', 30)
)

return NonBlockingLock(
project=self,
version=version,
max_lock_age=max_lock_age,
)

def repo_lock(self, version, timeout=5, polling_interval=5):
return Lock(self, version, timeout, polling_interval)
Expand Down
14 changes: 3 additions & 11 deletions readthedocs/projects/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,7 @@ def sync_repo(self):
),
)

with self.project.repo_nonblockinglock(
version=self.version,
max_lock_age=getattr(settings, 'REPO_LOCK_SECONDS', 30)):

with self.project.repo_nonblockinglock(version=self.version):
# Get the actual code on disk
try:
before_vcs.send(sender=self.version)
Expand Down Expand Up @@ -649,10 +646,7 @@ def setup_python_environment(self):
"""
self.build_env.update_build(state=BUILD_STATE_INSTALLING)

with self.project.repo_nonblockinglock(
version=self.version,
max_lock_age=getattr(settings, 'REPO_LOCK_SECONDS', 30)):

with self.project.repo_nonblockinglock(version=self.version):
# Check if the python version/build image in the current venv is the
# same to be used in this build and if it differs, wipe the venv to
# avoid conflicts.
Expand Down Expand Up @@ -682,9 +676,7 @@ def build_docs(self):
before_build.send(sender=self.version)

outcomes = defaultdict(lambda: False)
with self.project.repo_nonblockinglock(
version=self.version,
max_lock_age=getattr(settings, 'REPO_LOCK_SECONDS', 30)):
with self.project.repo_nonblockinglock(version=self.version):
outcomes['html'] = self.build_docs_html()
outcomes['search'] = self.build_docs_search()
outcomes['localmedia'] = self.build_docs_localmedia()
Expand Down

0 comments on commit a6c2481

Please sign in to comment.