diff --git a/readthedocs/builds/constants.py b/readthedocs/builds/constants.py index 343ef2bff05..c8d44ad4e41 100644 --- a/readthedocs/builds/constants.py +++ b/readthedocs/builds/constants.py @@ -126,3 +126,5 @@ ALL_VERSIONS: ALL_VERSIONS_REGEX, SEMVER_VERSIONS: SEMVER_VERSIONS_REGEX, } + +BUILD_STATUS_CODE_DUPLICATED_BUILD = 0 diff --git a/readthedocs/builds/migrations/0023_add_status_code.py b/readthedocs/builds/migrations/0023_add_status_code.py new file mode 100644 index 00000000000..5eb94244dd7 --- /dev/null +++ b/readthedocs/builds/migrations/0023_add_status_code.py @@ -0,0 +1,18 @@ +# Generated by Django 2.2.12 on 2020-06-03 15:17 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('builds', '0022_migrate_protected_versions'), + ] + + operations = [ + migrations.AddField( + model_name='build', + name='status_code', + field=models.BooleanField(blank=True, default=None, null=True, verbose_name='Status code'), + ), + ] diff --git a/readthedocs/builds/models.py b/readthedocs/builds/models.py index 99ae7824de9..53466aa3e7c 100644 --- a/readthedocs/builds/models.py +++ b/readthedocs/builds/models.py @@ -615,6 +615,7 @@ class Build(models.Model): choices=BUILD_STATE, default='finished', ) + status_code = models.BooleanField(_('Status code'), null=True, default=None, blank=True) date = models.DateTimeField(_('Date'), auto_now_add=True) success = models.BooleanField(_('Success'), default=True) diff --git a/readthedocs/core/utils/__init__.py b/readthedocs/core/utils/__init__.py index c257470406b..95207b1f2c1 100644 --- a/readthedocs/core/utils/__init__.py +++ b/readthedocs/core/utils/__init__.py @@ -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__) @@ -165,8 +165,50 @@ 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, + ).exclude( + pk=build.pk, + ).exists() + ) + else: + skip_build = Build.objects.filter( + project=project, + version=version, + state=BUILD_STATE_TRIGGERED, + ).count() > 1 + + if not project.has_feature(Feature.DEDUPLICATE_BUILDS): + log.debug('Skipping deduplication of builds. Feature not enabled. project=%s', project.slug) + skip_build = False + + 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.status_code = DuplicatedBuildError.status_code + build.exit_code = DuplicatedBuildError.exit_code + build.success = False + 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) diff --git a/readthedocs/doc_builder/exceptions.py b/readthedocs/doc_builder/exceptions.py index d7a511430a4..68dd5e10476 100644 --- a/readthedocs/doc_builder/exceptions.py +++ b/readthedocs/doc_builder/exceptions.py @@ -3,6 +3,7 @@ """Exceptions raised when building documentation.""" from django.utils.translation import ugettext_noop +from readthedocs.builds.constants import BUILD_STATUS_CODE_DUPLICATED_BUILD class BuildEnvironmentException(Exception): @@ -57,6 +58,12 @@ class BuildMaxConcurrencyError(BuildEnvironmentError): message = ugettext_noop('Concurrency limit reached ({limit}), retrying in 5 minutes.') +class DuplicatedBuildError(BuildEnvironmentError): + message = ugettext_noop('Duplicated build.') + exit_code = 1 + status_code = BUILD_STATUS_CODE_DUPLICATED_BUILD + + class BuildEnvironmentWarning(BuildEnvironmentException): pass diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index e58d7ac89c1..fa516f47e74 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -1597,6 +1597,7 @@ def add_features(sender, **kwargs): STORE_PAGEVIEWS = 'store_pageviews' SPHINX_PARALLEL = 'sphinx_parallel' USE_SPHINX_BUILDERS = 'use_sphinx_builders' + DEDUPLICATE_BUILDS = 'deduplicate_builds' FEATURES = ( (USE_SPHINX_LATEST, _('Use latest version of Sphinx')), @@ -1705,7 +1706,11 @@ def add_features(sender, **kwargs): ( USE_SPHINX_BUILDERS, _('Use regular sphinx builders instead of custom RTD builders'), - ) + ), + ( + DEDUPLICATE_BUILDS, + _('Mark duplicated builds as NOOP to be skipped by builders'), + ), ) projects = models.ManyToManyField( diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index 812324f3c11..e8d47318ab1 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -58,6 +58,7 @@ BuildEnvironmentWarning, BuildMaxConcurrencyError, BuildTimeoutError, + DuplicatedBuildError, MkDocsYAMLParseError, ProjectBuildsSkippedError, VersionLockedError, @@ -542,6 +543,16 @@ def run( self.commit = commit self.config = None + if self.build.get('status_code') == DuplicatedBuildError.status_code: + 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) diff --git a/readthedocs/rtd_tests/tests/test_builds.py b/readthedocs/rtd_tests/tests/test_builds.py index 366a5b4da3d..48895ebc391 100644 --- a/readthedocs/rtd_tests/tests/test_builds.py +++ b/readthedocs/rtd_tests/tests/test_builds.py @@ -18,11 +18,13 @@ GENERIC_EXTERNAL_VERSION_NAME ) from readthedocs.builds.models import Build, Version +from readthedocs.core.utils import trigger_build from readthedocs.doc_builder.config import load_yaml_config from readthedocs.doc_builder.environments import LocalBuildEnvironment +from readthedocs.doc_builder.exceptions import DuplicatedBuildError from readthedocs.doc_builder.python_environments import Virtualenv from readthedocs.oauth.models import RemoteRepository -from readthedocs.projects.models import EnvironmentVariable, Project +from readthedocs.projects.models import EnvironmentVariable, Feature, Project from readthedocs.projects.tasks import UpdateDocsTaskStep from readthedocs.rtd_tests.tests.test_config_integration import create_load @@ -748,3 +750,112 @@ def test_get_commit_url_internal_version(self): sha=build.commit ) self.assertEqual(build.get_commit_url(), expected_url) + + + +@mock.patch('readthedocs.projects.tasks.update_docs_task') +class DeDuplicateBuildTests(TestCase): + + def setUp(self): + self.project = get(Project) + self.version = get( + Version, + project=self.project + ) + + get( + Feature, + feature_id=Feature.DEDUPLICATE_BUILDS, + projects=[self.project], + ) + + def test_trigger_duplicated_build_by_commit(self, update_docs_task): + """ + Trigger a build for the same commit twice. + + The second build should be marked as NOOP. + """ + self.assertEqual(Build.objects.count(), 0) + trigger_build(self.project, self.version, commit='a1b2c3') + self.assertEqual(Build.objects.count(), 1) + build = Build.objects.first() + self.assertEqual(build.state, 'triggered') + + trigger_build(self.project, self.version, commit='a1b2c3') + self.assertEqual(Build.objects.count(), 2) + build = Build.objects.first() + self.assertEqual(build.error, DuplicatedBuildError.message) + self.assertEqual(build.success, False) + self.assertEqual(build.exit_code, DuplicatedBuildError.exit_code) + self.assertEqual(build.status_code, DuplicatedBuildError.status_code) + self.assertEqual(build.state, 'finished') + + def test_trigger_duplicated_finshed_build_by_commit(self, update_docs_task): + """ + Trigger a build for the same commit twice. + + The second build should not be marked as NOOP if the previous + duplicated builds are in 'finished' state. + """ + self.assertEqual(Build.objects.count(), 0) + trigger_build(self.project, self.version, commit='a1b2c3') + self.assertEqual(Build.objects.count(), 1) + + # Mark the build as finished + build = Build.objects.first() + build.state = 'finished' + build.save() + build.refresh_from_db() + + trigger_build(self.project, self.version, commit='a1b2c3') + self.assertEqual(Build.objects.count(), 2) + build = Build.objects.first() + self.assertEqual(build.state, 'triggered') + self.assertIsNone(build.status_code) + + def test_trigger_duplicated_build_by_version(self, update_docs_task): + """ + Trigger a build for the same version. + + The second build should be marked as NOOP if there is already a build + for the same project and version on 'triggered' state. + """ + self.assertEqual(Build.objects.count(), 0) + trigger_build(self.project, self.version, commit=None) + self.assertEqual(Build.objects.count(), 1) + build = Build.objects.first() + self.assertEqual(build.state, 'triggered') + + trigger_build(self.project, self.version, commit=None) + self.assertEqual(Build.objects.count(), 2) + build = Build.objects.first() + self.assertEqual(build.error, DuplicatedBuildError.message) + self.assertEqual(build.success, False) + self.assertEqual(build.exit_code, DuplicatedBuildError.exit_code) + self.assertEqual(build.status_code, DuplicatedBuildError.status_code) + self.assertEqual(build.state, 'finished') + + def test_trigger_duplicated_non_triggered_build_by_version(self, update_docs_task): + """ + Trigger a build for the same version. + + The second build should not be marked as NOOP because the previous build + for the same project and version is on 'building' state (any non 'triggered') + """ + self.assertEqual(Build.objects.count(), 0) + trigger_build(self.project, self.version, commit=None) + self.assertEqual(Build.objects.count(), 1) + build = Build.objects.first() + self.assertEqual(build.state, 'triggered') + + # Mark the build as building + build = Build.objects.first() + build.state = 'building' + build.save() + build.refresh_from_db() + + trigger_build(self.project, self.version, commit=None) + self.assertEqual(Build.objects.count(), 2) + build = Build.objects.first() + self.assertEqual(build.state, 'triggered') + self.assertIsNone(build.status_code)