From 53469a0ce108af53121c5b42d271d0b4b91760e2 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Thu, 17 Sep 2020 12:13:59 +0200 Subject: [PATCH 1/3] Limit concurrency per-organization Allow us to limit concurrency per-organization besides per-project. Both limits are compatible and can be used together, giving more priority to the limits per-project. This means we can limit a whole organization to 10 concurrent builds but a particular project from the organization could be limited to only 2. --- readthedocs/builds/querysets.py | 19 +++- .../builds/tests/test_build_queryset.py | 86 +++++++++++++++++++ ...0004_organization_max_concurrent_builds.py | 18 ++++ readthedocs/organizations/models.py | 5 ++ 4 files changed, 126 insertions(+), 2 deletions(-) create mode 100644 readthedocs/organizations/migrations/0004_organization_max_concurrent_builds.py diff --git a/readthedocs/builds/querysets.py b/readthedocs/builds/querysets.py index 6e39700d87f..1416e10cfbd 100644 --- a/readthedocs/builds/querysets.py +++ b/readthedocs/builds/querysets.py @@ -134,6 +134,11 @@ def concurrent(self, project): - translation: concurrent builds of all the translations + builds of main project + .. note:: + + If the project/translation belongs to an organization, we count all concurrent + builds for all the projects from the organization. + :rtype: tuple :returns: limit_reached, number of concurrent builds, number of max concurrent """ @@ -149,12 +154,22 @@ def concurrent(self, project): # The project has translations, counts their builds as well query |= Q(project__in=project.translations.all()) + # If the project belongs to an organization, count all the projects + # from this organization as well + organization = project.organizations.first() + if organization: + query |= Q(project__in=organization.projects.all()) + concurrent = ( self.filter(query) .exclude(state__in=[BUILD_STATE_TRIGGERED, BUILD_STATE_FINISHED]) - ).count() + ).distinct().count() - max_concurrent = project.max_concurrent_builds or settings.RTD_MAX_CONCURRENT_BUILDS + max_concurrent = ( + project.max_concurrent_builds or + (organization.max_concurrent_builds if organization else 0) or + settings.RTD_MAX_CONCURRENT_BUILDS + ) log.info( 'Concurrent builds. project=%s running=%s max=%s', project.slug, diff --git a/readthedocs/builds/tests/test_build_queryset.py b/readthedocs/builds/tests/test_build_queryset.py index f9c9b19a3aa..67d4e616a8b 100644 --- a/readthedocs/builds/tests/test_build_queryset.py +++ b/readthedocs/builds/tests/test_build_queryset.py @@ -5,6 +5,7 @@ from readthedocs.builds.querysets import BuildQuerySet from readthedocs.builds.models import Build, Version +from readthedocs.organizations.models import Organization from readthedocs.projects.models import Project, Feature @@ -73,3 +74,88 @@ def test_concurrent_builds_translations(self): ) assert (True, 4, 4) == Build.objects.concurrent(translation) assert (True, 4, 4) == Build.objects.concurrent(project) + + def test_concurrent_builds_organization(self): + organization = fixture.get( + Organization, + max_concurrent_builds=None, + ) + + for _ in range(2): + project = fixture.get( + Project, + max_concurrent_builds=None, + main_language_project=None, + ) + organization.projects.add(project) + + for project in organization.projects.all(): + for state in ('triggered', 'building', 'cloning', 'finished'): + fixture.get( + Build, + project=project, + state=state, + ) + + project = organization.projects.first() + assert (True, 4, 4) == Build.objects.concurrent(project) + for state in ('building', 'cloning'): + fixture.get( + Build, + project=project, + state=state, + ) + assert (True, 6, 4) == Build.objects.concurrent(project) + + def test_concurrent_builds_organization_limited(self): + organization = fixture.get( + Organization, + max_concurrent_builds=10, + ) + project_with_builds = fixture.get( + Project, + max_concurrent_builds=None, + main_language_project=None, + ) + project_without_builds = fixture.get( + Project, + max_concurrent_builds=None, + main_language_project=None, + ) + organization.projects.add(project_with_builds) + organization.projects.add(project_without_builds) + for state in ('triggered', 'building', 'cloning', 'finished'): + fixture.get( + Build, + project=project_with_builds, + state=state, + ) + # Calling it with ``project_without_builds`` should count the builds + # from ``project_with_builds`` as well + assert (False, 2, 10) == Build.objects.concurrent(project_without_builds) + + def test_concurrent_builds_organization_and_project_limited(self): + organization = fixture.get( + Organization, + max_concurrent_builds=10, + ) + project_limited = fixture.get( + Project, + max_concurrent_builds=2, + main_language_project=None, + ) + project_not_limited = fixture.get( + Project, + max_concurrent_builds=None, + main_language_project=None, + ) + organization.projects.add(project_limited) + organization.projects.add(project_not_limited) + for state in ('triggered', 'building', 'cloning', 'finished'): + fixture.get( + Build, + project=project_limited, + state=state, + ) + assert (True, 2, 2) == Build.objects.concurrent(project_limited) + assert (False, 2, 10) == Build.objects.concurrent(project_not_limited) diff --git a/readthedocs/organizations/migrations/0004_organization_max_concurrent_builds.py b/readthedocs/organizations/migrations/0004_organization_max_concurrent_builds.py new file mode 100644 index 00000000000..466b89684bf --- /dev/null +++ b/readthedocs/organizations/migrations/0004_organization_max_concurrent_builds.py @@ -0,0 +1,18 @@ +# Generated by Django 2.2.16 on 2020-09-17 09:54 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('organizations', '0003_team_auto_join_email_users'), + ] + + operations = [ + migrations.AddField( + model_name='organization', + name='max_concurrent_builds', + field=models.IntegerField(blank=True, null=True, verbose_name='Maximum concurrent builds allowed for this organization'), + ), + ] diff --git a/readthedocs/organizations/models.py b/readthedocs/organizations/models.py index 3b7b11511b0..0f65bf62cde 100644 --- a/readthedocs/organizations/models.py +++ b/readthedocs/organizations/models.py @@ -69,6 +69,11 @@ class Organization(models.Model): help_text='Docs and builds are disabled for this organization', default=False, ) + max_concurrent_builds = models.IntegerField( + _('Maximum concurrent builds allowed for this organization'), + null=True, + blank=True, + ) stripe_id = models.CharField( _('Stripe customer ID'), From 00e050d52aa85d679a1ff2a3577ee310fef1ac50 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 21 Sep 2020 11:26:59 +0200 Subject: [PATCH 2/3] Use a QuerySet method for Project.max_concurrent_builds This QuerySet takes into account the project limit, then the organization limit and finally the default setting. Using a QuerySet allows us to extend it from corporate and use `Plan.max_concurrent_builds` field. --- readthedocs/builds/querysets.py | 8 ++------ readthedocs/projects/querysets.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/readthedocs/builds/querysets.py b/readthedocs/builds/querysets.py index 1416e10cfbd..c915fb0ab2e 100644 --- a/readthedocs/builds/querysets.py +++ b/readthedocs/builds/querysets.py @@ -1,12 +1,12 @@ """Build and Version QuerySet classes.""" import logging -from django.conf import settings from django.db import models from django.db.models import Q from readthedocs.core.utils.extend import SettingsOverrideObject from readthedocs.projects import constants +from readthedocs.projects.models import Project from .constants import BUILD_STATE_TRIGGERED, BUILD_STATE_FINISHED @@ -165,11 +165,7 @@ def concurrent(self, project): .exclude(state__in=[BUILD_STATE_TRIGGERED, BUILD_STATE_FINISHED]) ).distinct().count() - max_concurrent = ( - project.max_concurrent_builds or - (organization.max_concurrent_builds if organization else 0) or - settings.RTD_MAX_CONCURRENT_BUILDS - ) + max_concurrent = Project.objects.max_concurrent_builds(project) log.info( 'Concurrent builds. project=%s running=%s max=%s', project.slug, diff --git a/readthedocs/projects/querysets.py b/readthedocs/projects/querysets.py index 22d87d1eeee..e92de083023 100644 --- a/readthedocs/projects/querysets.py +++ b/readthedocs/projects/querysets.py @@ -2,6 +2,7 @@ from django.db import models from django.db.models import OuterRef, Prefetch, Q, Subquery +from django.conf import settings from readthedocs.builds.constants import EXTERNAL from readthedocs.core.utils.extend import SettingsOverrideObject @@ -75,6 +76,34 @@ def is_active(self, project): return True + def max_concurrent_builds(self, project): + """ + Return the max concurrent builds allowed for the project. + + Max concurrency build priority: + + - project + - organization + - default setting + + :param project: project to be checked + :type project: readthedocs.projects.models.Project + + :returns: number of max concurrent builds for the project + :rtype: int + """ + max_concurrent_organization = None + organization = project.organizations.first() + if organization: + max_concurrent_organization = organization.max_concurrent_builds + + return ( + project.max_concurrent_builds or + max_concurrent_organization or + settings.RTD_MAX_CONCURRENT_BUILDS + ) + + def prefetch_latest_build(self): """ Prefetch "latest build" for each project. From 05e6af88e3cdc79240962df13075fbf812cbda1b Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 21 Sep 2020 12:02:23 +0200 Subject: [PATCH 3/3] Lint :( --- readthedocs/projects/querysets.py | 1 - 1 file changed, 1 deletion(-) diff --git a/readthedocs/projects/querysets.py b/readthedocs/projects/querysets.py index e92de083023..4c5acda0baa 100644 --- a/readthedocs/projects/querysets.py +++ b/readthedocs/projects/querysets.py @@ -103,7 +103,6 @@ def max_concurrent_builds(self, project): settings.RTD_MAX_CONCURRENT_BUILDS ) - def prefetch_latest_build(self): """ Prefetch "latest build" for each project.