Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add faster build router #11952

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 88 additions & 33 deletions readthedocs/builds/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,60 @@
from readthedocs.projects.constants import GITHUB_BRAND, GITLAB_BRAND
from readthedocs.projects.models import Project, WebHookEvent
from readthedocs.storage import build_commands_storage
from readthedocs.subscriptions.constants import TYPE_FASTER_BUILDS
from readthedocs.subscriptions.products import get_feature
from readthedocs.worker import app

log = structlog.get_logger(__name__)


class TaskRouter:
class BaseBuildRouter:

"""
Base class for Celery tasks routers.

Contains common constants and methods for routing tasks.
"""

BUILD_LARGE_QUEUE = "build:large"
BUILD_DEFAULT_QUEUE = "build:default"

def _get_version(self, task, args, kwargs):
tasks = [
"readthedocs.projects.tasks.builds.update_docs_task",
"readthedocs.projects.tasks.builds.sync_repository_task",
]
version = None
if task in tasks:
version_pk = args[0]
try:
version = Version.objects.get(pk=version_pk)
except Version.DoesNotExist:
log.debug(
"Version does not exist. Routing task to default queue.",
version_id=version_pk,
)
return version

def _setup_router(self, task, args, kwargs):
log.debug("Executing router.", task=task)
if task not in (
"readthedocs.projects.tasks.builds.update_docs_task",
"readthedocs.projects.tasks.builds.sync_repository_task",
):
log.debug("Skipping routing non-build task.", task=task)
return None, None

version = self._get_version(task, args, kwargs)
if not version:
log.debug("No Build/Version found. No routing task.", task=task)
return None, None

project = version.project
return project, version


class TaskRouter(BaseBuildRouter):

"""
Celery tasks router.
Expand All @@ -65,25 +113,11 @@ class TaskRouter:
N_LAST_BUILDS = 15
TIME_AVERAGE = 350

BUILD_DEFAULT_QUEUE = "build:default"
BUILD_LARGE_QUEUE = "build:large"

def route_for_task(self, task, args, kwargs, **__):
log.debug("Executing TaskRouter.", task=task)
if task not in (
"readthedocs.projects.tasks.builds.update_docs_task",
"readthedocs.projects.tasks.builds.sync_repository_task",
):
log.debug("Skipping routing non-build task.", task=task)
project, version = self._setup_router(task, args, kwargs)
if not project or not version:
return

version = self._get_version(task, args, kwargs)
if not version:
log.debug("No Build/Version found. No routing task.", task=task)
return

project = version.project

# Do not override the queue defined in the project itself
if project.build_queue:
log.info(
Expand All @@ -93,6 +127,15 @@ def route_for_task(self, task, args, kwargs, **__):
)
return project.build_queue

# Check if the project has the faster builds feature enabled
if project.has_feature(TYPE_FASTER_BUILDS):
Comment on lines +130 to +131
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

has_feature refers to feature flags, not to constants from subscriptions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed this locally, but somehow lost the change 🙃

log.info(
"Routing task because project has faster builds feature enabled.",
project_slug=project.slug,
queue=self.BUILD_LARGE_QUEUE,
)
return self.BUILD_LARGE_QUEUE

# Use last queue used by the default version for external versions
# We always want the same queue as the previous default version,
# so that users will have the same outcome for PR's as normal builds.
Expand Down Expand Up @@ -162,22 +205,34 @@ def route_for_task(self, task, args, kwargs, **__):
)
return

def _get_version(self, task, args, kwargs):
tasks = [
"readthedocs.projects.tasks.builds.update_docs_task",
"readthedocs.projects.tasks.builds.sync_repository_task",
]
version = None
if task in tasks:
version_pk = args[0]
try:
version = Version.objects.get(pk=version_pk)
except Version.DoesNotExist:
log.debug(
"Version does not exist. Routing task to default queue.",
version_id=version_pk,
)
return version

class FeatureBasedBuildRouter(BaseBuildRouter):

"""
Celery tasks router based on project features.

Routes builds to the `build:large` queue if the `TYPE_FASTER_BUILDS` feature is enabled.
"""

def route_for_task(self, task, args, kwargs, **__):
project, version = self._setup_router(task, args, kwargs)
if not project or not version:
return

# Check if the project has the faster builds feature enabled
if get_feature(project, TYPE_FASTER_BUILDS):
log.info(
"Routing task because project has faster builds feature enabled.",
project_slug=project.slug,
queue=self.BUILD_LARGE_QUEUE,
)
return self.BUILD_LARGE_QUEUE

log.debug(
"No routing task because no conditions were met.",
project_slug=project.slug,
)
return


@app.task(queue="web", bind=True)
Expand Down
2 changes: 2 additions & 0 deletions readthedocs/subscriptions/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
TYPE_AUDIT_PAGEVIEWS = "audit-pageviews"
TYPE_REDIRECTS_LIMIT = "redirects-limit"
TYPE_SSO_SAML = "sso-saml"
TYPE_FASTER_BUILDS = "faster_builds"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
TYPE_FASTER_BUILDS = "faster_builds"
TYPE_FASTER_BUILDS = "faster-builds"

Looks like we are using hyphens for new variables


FEATURE_TYPES = (
(TYPE_CNAME, _("Custom domain")),
Expand All @@ -37,4 +38,5 @@
(TYPE_AUDIT_LOGS, _("Audit logs")),
(TYPE_AUDIT_PAGEVIEWS, _("Audit logs for every page view")),
(TYPE_REDIRECTS_LIMIT, _("Redirects limit")),
(TYPE_FASTER_BUILDS, _("Faster builds")),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we want a better name for this or a more granular description when listing this item in the UI about what it exactly it offers, "how much faster".

)