From a6c24816eb6f53ece2da064aaf982688e1ab3d24 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Thu, 11 Oct 2018 12:47:00 +0200 Subject: [PATCH] Use Docker time limit for max lock age 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 --- readthedocs/projects/models.py | 24 ++++++++++++++++++++++-- readthedocs/projects/tasks.py | 14 +++----------- 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index bcf60d5a58a..7649d875b85 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -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) diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index c82b561219a..32fc8e058c3 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -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) @@ -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. @@ -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()