Build: remove DEDUPLICATE_BUILDS feature (#9591)

readthedocs/builds/constants.py

@@ -140,10 +140,8 @@
 }
 
 BUILD_STATUS_NORMAL = 'normal'
-BUILD_STATUS_DUPLICATED = 'duplicated'
 BUILD_STATUS_CHOICES = (
     (BUILD_STATUS_NORMAL, 'Normal'),
-    (BUILD_STATUS_DUPLICATED, 'Duplicated'),
 )
 
 

readthedocs/builds/migrations/0045_alter_build_status.py

@@ -0,0 +1,25 @@
+# Generated by Django 3.2.15 on 2022-10-06 09:03
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("builds", "0044_alter_version_documentation_type"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="build",
+            name="status",
+            field=models.CharField(
+                blank=True,
+                choices=[("normal", "Normal")],
+                default=None,
+                max_length=32,
+                null=True,
+                verbose_name="Status",
+            ),
+        ),
+    ]

readthedocs/core/utils/__init__.py

@@ -1,13 +1,11 @@
 """Common utility functions."""
 
-import datetime
 import re
 import signal
 
 import structlog
 from django.conf import settings
 from django.template.loader import render_to_string
-from django.utils import timezone
 from django.utils.functional import keep_lazy
 from django.utils.safestring import SafeText, mark_safe
 from django.utils.text import slugify as slugify_base
@@ -19,11 +17,7 @@
     BUILD_STATUS_PENDING,
     EXTERNAL,
 )
-from readthedocs.doc_builder.exceptions import (
-    BuildCancelled,
-    BuildMaxConcurrencyError,
-    DuplicatedBuildError,
-)
+from readthedocs.doc_builder.exceptions import BuildCancelled, BuildMaxConcurrencyError
 from readthedocs.worker import app
 
 log = structlog.get_logger(__name__)
@@ -123,7 +117,6 @@ def prepare_build(
             event=WebHookEvent.BUILD_TRIGGERED,
         )
 
-    skip_build = False
     # Reduce overhead when doing multiple push on the same version.
     if project.has_feature(Feature.CANCEL_OLD_BUILDS):
         running_builds = (
@@ -147,66 +140,9 @@ def prepare_build(
         # we cancel all of them and trigger a new one for the latest commit received.
         for running_build in running_builds:
             cancel_build(running_build)
-    else:
-        # NOTE: de-duplicate builds won't be required if we enable `CANCEL_OLD_BUILDS`,
-        # since canceling a build is more effective.
-        # However, we are keepting `DEDUPLICATE_BUILDS` code around while we test
-        # `CANCEL_OLD_BUILDS` with a few projects and we are happy with the results.
-        # After that, we can remove `DEDUPLICATE_BUILDS` code
-        # and make `CANCEL_OLD_BUILDS` the default behavior.
-        if commit:
-            skip_build = (
-                Build.objects.filter(
-                    project=project,
-                    version=version,
-                    commit=commit,
-                )
-                .exclude(
-                    state__in=BUILD_FINAL_STATES,
-                )
-                .exclude(
-                    pk=build.pk,
-                )
-                .exists()
-            )
-        else:
-            skip_build = (
-                Build.objects.filter(
-                    project=project,
-                    version=version,
-                    state=BUILD_STATE_TRIGGERED,
-                    # By filtering for builds triggered in the previous 5 minutes we
-                    # avoid false positives for builds that failed for any reason and
-                    # didn't update their state, ending up on blocked builds for that
-                    # version (all the builds are marked as DUPLICATED in that case).
-                    # Adding this date condition, we reduce the risk of hitting this
-                    # problem to 5 minutes only.
-                    date__gte=timezone.now() - datetime.timedelta(minutes=5),
-                ).count()
-                > 1
-            )
-
-        if not project.has_feature(Feature.DEDUPLICATE_BUILDS):
-            log.debug(
-                "Skipping deduplication of builds. Feature not enabled.",
-            )
-            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.",
-            )
-            build.error = DuplicatedBuildError.message
-            build.status = DuplicatedBuildError.status
-            build.exit_code = DuplicatedBuildError.exit_code
-            build.success = False
-            build.state = BUILD_STATE_CANCELLED
-            build.save()
 
     # Start the build in X minutes and mark it as limited
-    if not skip_build and project.has_feature(Feature.LIMIT_CONCURRENT_BUILDS):
+    if project.has_feature(Feature.LIMIT_CONCURRENT_BUILDS):
         limit_reached, _, max_concurrent_builds = Build.objects.concurrent(project)
         if limit_reached:
             log.warning(

readthedocs/doc_builder/exceptions.py

@@ -2,7 +2,7 @@
 
 from django.utils.translation import gettext_noop
 
-from readthedocs.builds.constants import BUILD_STATE_CANCELLED, BUILD_STATUS_DUPLICATED
+from readthedocs.builds.constants import BUILD_STATE_CANCELLED
 from readthedocs.projects.constants import BUILD_COMMANDS_OUTPUT_PATH_HTML
 
 
@@ -56,13 +56,6 @@ class BuildMaxConcurrencyError(BuildUserError):
     message = gettext_noop('Concurrency limit reached ({limit}), retrying in 5 minutes.')
 
 
-class DuplicatedBuildError(BuildUserError):
-    message = gettext_noop('Duplicated build.')
-    exit_code = 1
-    status = BUILD_STATUS_DUPLICATED
-    state = BUILD_STATE_CANCELLED
-
-
 class BuildCancelled(BuildUserError):
     message = gettext_noop('Build cancelled by user.')
     state = BUILD_STATE_CANCELLED

readthedocs/projects/models.py

@@ -1862,7 +1862,6 @@ def add_features(sender, **kwargs):
     VCS_REMOTE_LISTING = "vcs_remote_listing"
     SPHINX_PARALLEL = "sphinx_parallel"
     USE_SPHINX_BUILDERS = "use_sphinx_builders"
-    DEDUPLICATE_BUILDS = "deduplicate_builds"
     CANCEL_OLD_BUILDS = "cancel_old_builds"
     DONT_CREATE_INDEX = "dont_create_index"
 
@@ -2011,10 +2010,6 @@ 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'),
-        ),
         (
             CANCEL_OLD_BUILDS,
             _(

readthedocs/projects/tasks/builds.py

@@ -44,7 +44,6 @@
     BuildCancelled,
     BuildMaxConcurrencyError,
     BuildUserError,
-    DuplicatedBuildError,
     MkDocsYAMLParseError,
     ProjectBuildsSkippedError,
     YAMLParseError,
@@ -276,7 +275,6 @@ class UpdateDocsTask(SyncRepositoryMixin, Task):
     # All exceptions generated by a user miss-configuration should be listed
     # here. Actually, every subclass of ``BuildUserError``.
     throws = (
-        DuplicatedBuildError,
         ProjectBuildsSkippedError,
         ConfigError,
         YAMLParseError,
@@ -291,14 +289,12 @@ class UpdateDocsTask(SyncRepositoryMixin, Task):
     exceptions_without_notifications = (
         BuildCancelled,
         BuildMaxConcurrencyError,
-        DuplicatedBuildError,
         ProjectBuildsSkippedError,
     )
 
     # Do not send external build status on failure builds for these exceptions.
     exceptions_without_external_build_status = (
         BuildMaxConcurrencyError,
-        DuplicatedBuildError,
     )
 
     acks_late = True
@@ -354,11 +350,6 @@ def _check_concurrency_limit(self):
                 )
             )
 
-    def _check_duplicated_build(self):
-        if self.data.build.get('status') == DuplicatedBuildError.status:
-            log.warning('NOOP: build is marked as duplicated.')
-            raise DuplicatedBuildError
-
     def _check_project_disabled(self):
         if self.data.project.skip:
             log.warning('Project build skipped.')
@@ -412,7 +403,6 @@ def before_start(self, task_id, args, kwargs):
         self._setup_sigterm()
 
         self._check_project_disabled()
-        self._check_duplicated_build()
         self._check_concurrency_limit()
         self._reset_build()
 

readthedocs/projects/views/public.py

@@ -22,7 +22,7 @@
 
 from readthedocs.analytics.tasks import analytics_event
 from readthedocs.analytics.utils import get_client_ip
-from readthedocs.builds.constants import BUILD_STATUS_DUPLICATED, LATEST
+from readthedocs.builds.constants import BUILD_STATE_FINISHED, LATEST
 from readthedocs.builds.models import Version
 from readthedocs.builds.views import BuildTriggerMixin
 from readthedocs.core.permissions import AdminPermission
@@ -186,10 +186,8 @@ def get(self, request, project_slug, *args, **kwargs):
 
         if version:
             last_build = (
-                version.builds
-                .filter(type='html', state='finished')
-                .exclude(status=BUILD_STATUS_DUPLICATED)
-                .order_by('-date')
+                version.builds.filter(type="html", state=BUILD_STATE_FINISHED)
+                .order_by("-date")
                 .first()
             )
             if last_build:

readthedocs/rtd_tests/tests/test_api.py

@@ -40,7 +40,6 @@
 from readthedocs.builds.constants import (
     BUILD_STATE_CLONING,
     BUILD_STATE_TRIGGERED,
-    BUILD_STATUS_DUPLICATED,
     EXTERNAL,
     EXTERNAL_VERSION_STATE_CLOSED,
     LATEST,
@@ -105,7 +104,6 @@ def test_reset_build(self):
             project=self.project,
             version=self.version,
             state=BUILD_STATE_CLONING,
-            status=BUILD_STATUS_DUPLICATED,
             success=False,
             output='Output',
             error='Error',