Skip to content

Commit

Permalink
De-duplicate builds
Browse files Browse the repository at this point in the history
When a build is triggered, it could be marked as to be skipped by builders if:

* there is already a build running/queued for the same commit

  Multiple builds of the same commit will lead to the same results. So, we skip
  it if there is one already running/queued.

* there is already a build queued for the same version

  When building a version without specifying the commit, the last commit is
  built. In this case, we can only skip the new triggered build if there is one
  build queued because both builds will just pick the same commit.
  • Loading branch information
humitos committed May 25, 2020
1 parent 33d246c commit e76a948
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 2 deletions.
38 changes: 36 additions & 2 deletions readthedocs/core/utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
)
from readthedocs.doc_builder.constants import DOCKER_LIMITS
from readthedocs.projects.constants import CELERY_LOW, CELERY_MEDIUM, CELERY_HIGH
from readthedocs.doc_builder.exceptions import BuildMaxConcurrencyError
from readthedocs.doc_builder.exceptions import BuildMaxConcurrencyError, DuplicatedBuildError


log = logging.getLogger(__name__)
Expand Down Expand Up @@ -165,8 +165,42 @@ def prepare_build(
# External builds should be lower priority.
options['priority'] = CELERY_LOW

skip_build = False
if commit:
skip_build = (
Build.objects
.filter(
project=project,
version=version,
commit=commit,
).exclude(
state=BUILD_STATE_FINISHED,
).exists()
)
else:
skip_build = Build.objects.filter(
project=project,
version=version,
state=BUILD_STATE_TRIGGERED,
).count() > 1
if skip_build:
# TODO: we could mark the old build as duplicated, however we reset our
# position in the queue and go back to the end of it --penalization
log.warning(
'Marking build to be skipped by builder. project=%s version=%s build=%s commit=%s',
project.slug,
version.slug,
build.pk,
commit,
)
build.error = DuplicatedBuildError.message
build.success = False
build.exit_code = 1
build.state = BUILD_STATE_FINISHED
build.save()

# Start the build in X minutes and mark it as limited
if project.has_feature(Feature.LIMIT_CONCURRENT_BUILDS):
if not skip_build and project.has_feature(Feature.LIMIT_CONCURRENT_BUILDS):
running_builds = (
Build.objects
.filter(project__slug=project.slug)
Expand Down
4 changes: 4 additions & 0 deletions readthedocs/doc_builder/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ class BuildMaxConcurrencyError(BuildEnvironmentError):
message = ugettext_noop('Concurrency limit reached ({limit}), retrying in 5 minutes.')


class DuplicatedBuildError(BuildEnvironmentError):
message = ugettext_noop('Duplicated build.')


class BuildEnvironmentWarning(BuildEnvironmentException):
pass

Expand Down
11 changes: 11 additions & 0 deletions readthedocs/projects/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
BuildEnvironmentWarning,
BuildMaxConcurrencyError,
BuildTimeoutError,
DuplicatedBuildError,
MkDocsYAMLParseError,
ProjectBuildsSkippedError,
VersionLockedError,
Expand Down Expand Up @@ -542,6 +543,16 @@ def run(
self.commit = commit
self.config = None

if self.build['state'] == BUILD_STATE_FINISHED and self.build['error'] == DuplicatedBuildError.message:
log.warning(
'NOOP: build is marked as duplicated. project=%s version=%s build=%s commit=%s',
self.project.slug,
self.version.slug,
build_pk,
self.commit,
)
return True

if self.project.has_feature(Feature.LIMIT_CONCURRENT_BUILDS):
response = api_v2.build.running.get(project__slug=self.project.slug)
builds_running = response.get('count', 0)
Expand Down

0 comments on commit e76a948

Please sign in to comment.