Build: use only one setting for build time limit

Closes #12347

Author: humitos

docs/dev/settings.rst

@@ -1,23 +1,19 @@
 Interesting settings
 ====================
 
-DOCKER_LIMITS
--------------
+BUILD_MEMORY_LIMIT
+------------------
 
-A dictionary of limits to virtual machines. These limits include:
+The maximum memory allocated to the virtual machine.
+If this limit is hit, build processes will be automatically killed.
+Examples: '200m' for 200MB of total memory, or '2g' for 2GB of total memory.
 
-time
-    An integer representing the total allowed time limit (in
-    seconds) of build processes. This time limit affects the parent
-    process to the virtual machine and will force a virtual machine
-    to die if a build is still running after the allotted time
-    expires.
+BUILD_TIME_LIMIT
+----------------
 
-memory
-    The maximum memory allocated to the virtual machine. If this
-    limit is hit, build processes will be automatically killed.
-    Examples: '200m' for 200MB of total memory, or '2g' for 2GB of
-    total memory.
+An integer representing the total allowed time limit (in seconds) of build processes.
+This time limit affects the parent process to the virtual machine and will force a virtual machine
+to die if a build is still running after the allotted time expires.
 
 PRODUCTION_DOMAIN
 ------------------

readthedocs/api/v2/models.py

@@ -18,7 +18,8 @@ def create_key(self, project):
         Build API keys are valid for 3 hours,
         and can be revoked at any time by hitting the /api/v2/revoke/ endpoint.
         """
-        expiry_date = timezone.now() + timedelta(hours=3)
+        # Use the build time limit + 15% for the API token
+        expiry_date = timezone.now() + timedelta(seconds=settings.BUILD_TIME_LIMIT * 1.25)
         name_max_length = self.model._meta.get_field("name").max_length
         return super().create_key(
             # Name is required, so we use the project slug for it.

readthedocs/core/utils/__init__.py

@@ -86,18 +86,15 @@ def prepare_build(
         options["queue"] = project.build_queue
 
     # Set per-task time limit
-    # TODO remove the use of Docker limits or replace the logic here. This
-    # was pulling the Docker limits that were set on each stack, but we moved
-    # to dynamic setting of the Docker limits. This sets a failsafe higher
-    # limit, but if no builds hit this limit, it should be safe to remove and
-    # rely on Docker to terminate things on time.
-    # time_limit = DOCKER_LIMITS['time']
-    time_limit = 7200
+    time_limit = settings.BUILD_TIME_LIMIT
     try:
         if project.container_time_limit:
             time_limit = int(project.container_time_limit)
     except ValueError:
-        log.warning("Invalid time_limit for project.")
+        log.warning(
+            "Invalid time_limit for project.",
+            time_limit=project.container_time_limit,
+        )
 
     # Add 20% overhead to task, to ensure the build can timeout and the task
     # will cleanly finish.

readthedocs/doc_builder/constants.py

@@ -14,7 +14,6 @@
 DOCKER_SOCKET = settings.DOCKER_SOCKET
 DOCKER_VERSION = settings.DOCKER_VERSION
 DOCKER_IMAGE = settings.DOCKER_IMAGE
-DOCKER_LIMITS = settings.DOCKER_LIMITS
 DOCKER_TIMEOUT_EXIT_CODE = 42
 DOCKER_OOM_EXIT_CODE = 137
 

readthedocs/doc_builder/environments.py

@@ -25,7 +25,6 @@
 
 from .constants import DOCKER_HOSTNAME_MAX_LEN
 from .constants import DOCKER_IMAGE
-from .constants import DOCKER_LIMITS
 from .constants import DOCKER_OOM_EXIT_CODE
 from .constants import DOCKER_SOCKET
 from .constants import DOCKER_TIMEOUT_EXIT_CODE
@@ -581,8 +580,8 @@ class DockerBuildEnvironment(BaseBuildEnvironment):
 
     command_class = DockerBuildCommand
     container_image = DOCKER_IMAGE
-    container_mem_limit = DOCKER_LIMITS.get("memory")
-    container_time_limit = DOCKER_LIMITS.get("time")
+    container_mem_limit = settings.BUILD_MEMORY_LIMIT
+    container_time_limit = settings.BUILD_TIME_LIMIT
 
     def __init__(self, *args, **kwargs):
         container_image = kwargs.pop("container_image", None)

readthedocs/projects/tasks/utils.py

@@ -167,6 +167,8 @@ def finish_inactive_builds():
     These inactive builds will be marked as ``success`` and ``CANCELLED`` with an
     ``error`` to be communicated to the user.
     """
+    # TODO: delete this task once we are fully migrated to ``BUILD_HEALTHCHECK``
+    #
     # TODO similar to the celery task time limit, we can't infer this from
     # Docker settings anymore, because Docker settings are determined on the
     # build servers dynamically.

readthedocs/settings/base.py

@@ -605,7 +605,7 @@ def TEMPLATES(self):
     # https://github.com/readthedocs/readthedocs.org/issues/12317#issuecomment-3070950434
     # https://docs.celeryq.dev/en/stable/getting-started/backends-and-brokers/redis.html#visibility-timeout
     BROKER_TRANSPORT_OPTIONS = {
-        'visibility_timeout': 18000,  # 5 hours
+        'visibility_timeout': self.BUILD_TIME_LIMIT * 1.15,  # 15% more than the build time limit
     }
 
     CELERY_DEFAULT_QUEUE = "celery"
@@ -721,7 +721,13 @@ def TEMPLATES(self):
     # since we can't read their config file image choice before cloning
     RTD_DOCKER_CLONE_IMAGE = RTD_DOCKER_BUILD_SETTINGS["os"]["ubuntu-22.04"]
 
-    def _get_docker_memory_limit(self):
+    def _get_build_memory_limit(self):
+        """
+        Return the buld memory limit based on available system memory.
+
+        We subtract ~1000Mb for overhead of processes and base system, and set
+        the build time as proportional to the memory limit.
+        """
         try:
             total_memory = int(
                 subprocess.check_output(
@@ -735,46 +741,31 @@ def _get_docker_memory_limit(self):
             # int and raise a ValueError
             log.exception("Failed to get memory size, using defaults Docker limits.")
 
-    # Coefficient used to determine build time limit, as a percentage of total
-    # memory. Historical values here were 0.225 to 0.3.
-    DOCKER_TIME_LIMIT_COEFF = 0.25
+    BUILD_TIME_LIMIT = 900  # seconds
 
     @property
-    def DOCKER_LIMITS(self):
+    def BUILD_MEMORY_LIMIT(self):
         """
-        Set docker limits dynamically, if in production, based on system memory.
+        Set build memory limit dynamically, if in production, based on system memory.
 
         We do this to avoid having separate build images. This assumes 1 build
         process per server, which will be allowed to consume all available
         memory.
-
-        We subtract 750MiB for overhead of processes and base system, and set
-        the build time as proportional to the memory limit.
         """
         # Our normal default
-        limits = {
-            "memory": "2g",
-            "time": 900,
-        }
+        default_memory_limit = "7g"
 
         # Only run on our servers
         if self.RTD_IS_PRODUCTION:
-            total_memory, memory_limit = self._get_docker_memory_limit()
-            if memory_limit:
-                limits = {
-                    "memory": f"{memory_limit}m",
-                    "time": max(
-                        limits["time"],
-                        round(total_memory * self.DOCKER_TIME_LIMIT_COEFF, -2),
-                    ),
-                }
+            total_memory, memory_limit = self._get_build_memory_limit()
+
+        memory_limit = memory_limit or default_memory_limit
         log.info(
-            "Using dynamic docker limits.",
+            "Using dynamic build limits.",
             hostname=socket.gethostname(),
-            memory=limits["memory"],
-            time=limits["time"],
+            memory=memory_limit,
         )
-        return limits
+        return memory_limit
 
     # Allauth
     ACCOUNT_ADAPTER = "readthedocs.core.adapters.AccountAdapter"

readthedocs/settings/docker_compose.py

@@ -15,7 +15,7 @@ class DockerBaseSettings(CommunityBaseSettings):
     RTD_DOCKER_COMPOSE_NETWORK = "community_readthedocs"
     RTD_DOCKER_COMPOSE_VOLUME = "community_build-user-builds"
     RTD_DOCKER_USER = f"{os.geteuid()}:{os.getegid()}"
-    DOCKER_LIMITS = {"memory": "2g", "time": 900}
+    BUILD_MEMORY_LIMIT = "2g"
 
     PRODUCTION_DOMAIN = os.environ.get("RTD_PRODUCTION_DOMAIN", "devthedocs.org")
     PUBLIC_DOMAIN = os.environ.get("RTD_PUBLIC_DOMAIN", "devthedocs.org")

readthedocs/settings/test.py

@@ -26,7 +26,8 @@ class CommunityTestSettings(CommunityBaseSettings):
     CELERY_ALWAYS_EAGER = True
 
     # Skip automatic detection of Docker limits for testing
-    DOCKER_LIMITS = {"memory": "200m", "time": 600}
+    BUILD_TIME_LIMIT = 600
+    BUILD_MEMORY_LIMIT = "200m"
 
     CACHES = {
         "default": {