Builds: delete old build objects by stsewd · Pull Request #12800 · readthedocs/readthedocs.org

stsewd · 2026-02-19T21:40:56Z

Archiving builds was refactored, so I could re-use how to get the build path.
Since we want to keep x builds for each version, we need to iterate over each project or version. Doing that will take a long time for the task to run, and trying to filter by an aggregation will result in a slow query that goes over the 2-second threshold we have. So I'm processing a slice of projects over each run, I'm using the cache to keep track of the last slice.
Commands from build objects are also now deleted from storage.
Had to change the delete_in_batches functions to support a slice instead of just a limit, so I could use them with the new code, since django doesn't allow any filtering or deletion on a sliced queryset.
Tested the querysets in production (.org), and they are fast, but hard to tell how fast they would be when actually deleting stuff. I did a count/select only.

Community

Around 500 projects make up 30% of the builds in the database.

If we are very conservative, we can delete:

delete_old_build_objects(days=360 * 3, keep_recent=250)
Found 5513191 builds to delete (~22%).

If we are a little bit more aggressive, we can delete:

delete_old_build_objects(days=360, keep_recent=100)
Found 12488385 builds to delete (~50%).

Business

Around 90 projects make up 56% of the builds in the database.

If we are very conservative, we can delete:

delete_old_build_objects(days=360 * 3, keep_recent=250)
Found 621459 builds to delete (~19%).

If we are a little bit more aggressive, we can delete:

delete_old_build_objects(days=360, keep_recent=100)
Found 1847314 builds to delete (~56%).

Full analysis at https://readthedocs.slack.com/archives/G81T0N8S3/p1771531958918869?thread_ts=1771531854.785459&cid=G81T0N8S3

Closes #12712

humitos

I think this is going in a good direction 👍🏼 . We should change the logic to keep the N builds for each version as we discussed in the meeting, tho.

If we are very conservative

Starting conservative is fine. There is no reason to rush on this and delete all the objects at once.

readthedocs/projects/models.py

readthedocs/builds/tasks.py

read-the-docs-community · 2026-02-25T23:14:58Z

Documentation build overview

📚 dev | 🛠️ Build #31631151 | 📁 Comparing a681538 against latest (d7b9779)

🔍 Preview build

No files changed.

stsewd · 2026-02-26T22:51:51Z

@copilot write tests for the delete_old_build_objects function

Copilot · 2026-02-26T22:51:58Z

@stsewd I've opened a new pull request, #12820, to work on those changes. Once the pull request is ready, I'll request review from you.

…n `_delete_builds` (#12820) - [x] Write tests for `delete_old_build_objects` function - [x] Fix `KeyError` in `_delete_builds` when no builds are deleted - [x] Use `assert foo == bar` syntax instead of `self.assertEqual` in `TestDeleteOldBuildObjects`  --- 🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. [Learn more about Advanced Security.](https://gh.io/cca-advanced-security) --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: stsewd <4975310+stsewd@users.noreply.github.com>

humitos

This is looking good. However, I think there are a few things we should do before moving forward and merging. There are some performance issues here and also a tweak needed in the deletion of objects formula.

We should be cautious here since we will be deleting a lot of objects. Another person should review this as well.

readthedocs/builds/models.py

readthedocs/builds/tasks.py

humitos · 2026-03-02T11:59:14Z

readthedocs/builds/tasks.py

+            builds_to_delete = version.builds.filter(
+                state__in=BUILD_FINAL_STATES,
+                date__lt=cutoff_date,
+            ).order_by("-date")
+            limit -= _delete_builds(builds_to_delete, start=keep_recent, end=keep_recent + limit)
+            if limit <= 0:


Why are not taking the slice here? I looks weird that we are adding all the builds to builds_to_delete but we are filtering them again and not deleting them all.

Suggested change

builds_to_delete = version.builds.filter(

state__in=BUILD_FINAL_STATES,

date__lt=cutoff_date,

).order_by("-date")

limit -= _delete_builds(builds_to_delete, start=keep_recent, end=keep_recent + limit)

if limit <= 0:

builds_to_delete = version.builds.filter(

state__in=BUILD_FINAL_STATES,

date__lt=cutoff_date,

).order_by("-date")[keep_recent:keep_recent + limit]

limit -= _delete_builds(builds_to_delete)

I prefer if we perform the slice here since it will be clearer and avoid confusions.

Deleting in batches doesn't work over a sliced queryset. It's explained in the PR description.

Yeah, but this is pretty confusing to me. We are calling _delete_builds(builds_to_delete) but the result is that those builds are not deleted? They are filtered instead and just a few of them are deleted? 🤔

Well, we are calling _delete_builds(queryset, start, end), that seems explicit?

humitos · 2026-03-02T12:01:59Z

readthedocs/builds/tasks.py

+        # Delete builds that are not associated with any version,
+        # keeping the most recent `keep_recent` builds per project.
+        builds_to_delete = project.builds.filter(
+            version=None, state__in=BUILD_FINAL_STATES, date__lt=cutoff_date
+        ).order_by("-date")
+        limit -= _delete_builds(builds_to_delete, start=keep_recent, end=keep_recent + limit)
+        if limit <= 0:
+            return


If the builds are not associated to any version, this is because the version was deleted. We shouldn't keep only keep_recent for all of those builds here, but keep_recent builds per build.version_slug instead to be consistent with the formula.

Is it worth it complicating this more? If the version is deleted, the build is never referenced by any version. We could keep 2x or 3x the builds here if we want to keep more builds, but honestly I think we are fine here.

What are we saving this builds for if they are not reference by any version?

So their builds page wouldn't immediately 404, and users had a history of previous builds. I'll be fine deleting builds when a version is deleted.

Yeah, I think we either want:

delete these build objects, OR

apply the same formula we are applying to the other builds

but I wouldn't create another logic for this particular use case.

but I wouldn't create another logic for this particular use case.

It's simpler and faster to apply a global limit than grouping the query for versions and then delete builds for each one of them...

readthedocs/builds/tasks.py

readthedocs/projects/models.py

humitos · 2026-03-03T10:09:31Z

readthedocs/storage/s3_storage.py

+        """
+        for batch in batched(paths, 1000):
+            objects = [{"Key": path} for path in batch]
+            self.bucket.delete_objects(Delete={"Objects": objects, "Quiet": True})


Do you know what Quiet means. I read:

Quiet (boolean) –
Element to enable quiet mode for the request. When you add this element, you must set its value to true.

but I don't understand it. Would this call raise an exception if it fails?

It reports the result of each path that was deleted, if it failed or not. We aren't doing anything with the result, so we don't need it.

humitos · 2026-03-03T10:12:36Z

readthedocs/builds/storage.py

+    def delete_paths(self, paths):
+        """
+        Delete multiple paths from storage.
+
+        This is a convenience method to delete multiple paths at once, which can be more efficient
+        for some storage backends that support batch deletion (eg. S3).
+        """
+        for path in paths:
+            self.delete(path)


I don't understand why we need this method here, 🤔 ?

I saw that you created RTDS3Boto3Storage.delete_paths below that looks 👍🏼 -- why do we need to override it here?

This is needed for testing, we use a file system backend there. I actually want to refactor how we inherit these classes, currently not all storage backends inherit these classes.

humitos · 2026-03-03T10:27:21Z

We should be cautious here since we will be deleting a lot of objects. Another person should review this as well.

Pinging @agjohnson here so he can perform another review. The pattern to delete objects has been increasing its complexity in the last weeks and I don't understand it anymore. We have a lot going on here in multiple functions:

delete_old_build_objects
delete_in_batches
_delete_builds
raw_delete_in_batches

Why do we need 4 functions to perform the deletion and what does each of them? I can't explain them by myself.

@agjohnson If you don't want to review all the PR, these are my main concerns:

multiple *delete_* functions that all seem to do the same but I don't understand each of them (above comment)
call a function to delete builds that don't delete them all
what to do with builds not associated to a version
perform path calculation at db level to speed things up

stsewd · 2026-03-03T17:28:08Z

Why do we need 4 functions to perform the deletion and what does each of them? I can't explain them by myself.

delete_old_build_objects: that's the task, it doesn't have anything to do with deletion in general.
_delete_builds: that's a helper to allow deleting builds from the DB and storage.
delete_in_batches/raw_delete_in_batches: these are the main functions to delete objecst in batches, one uses a normal deletion, the other one a raw delete. The exact use case is described in their docstring.

stsewd added 2 commits February 19, 2026 16:39

Builds: delete old build objects

5a3886f

Delete when deleting a project

c99a6a4

humitos reviewed Feb 23, 2026

View reviewed changes

Delete in batches of projects

9fd3444

stsewd added 6 commits February 26, 2026 16:30

Better way to delete old build objects

2633272

Optimize

a9d312d

Rename

f70cc74

Merge branch 'main' into delete-old-build-objects

b5ff69f

Updates

180975b

Fix circular imports

6034157

Copilot AI mentioned this pull request Feb 26, 2026

Builds: add tests for delete_old_build_objects and fix KeyError in _delete_builds #12820

Merged

3 tasks

Copilot AI and others added 3 commits February 26, 2026 19:12

Fix tests

25f014b

Fix test

66233cd

stsewd marked this pull request as ready for review February 27, 2026 01:49

stsewd requested a review from a team as a code owner February 27, 2026 01:49

stsewd requested a review from humitos February 27, 2026 01:49

auto-assign bot assigned stsewd Feb 27, 2026

humitos requested changes Mar 2, 2026

View reviewed changes

stsewd added 2 commits March 2, 2026 15:57

Delete paths in bulk

739af82

Merge branch 'main' into delete-old-build-objects

a681538

stsewd requested a review from humitos March 2, 2026 21:26

humitos reviewed Mar 3, 2026

View reviewed changes

humitos requested a review from agjohnson March 3, 2026 10:27

Uh oh!

Conversation

stsewd commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Community

Business

Uh oh!

humitos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

read-the-docs-community bot commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Documentation build overview

Uh oh!

stsewd commented Feb 26, 2026

Uh oh!

Copilot AI commented Feb 26, 2026

Uh oh!

humitos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stsewd Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

humitos commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stsewd commented Mar 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

stsewd commented Feb 19, 2026 •

edited

Loading

read-the-docs-community bot commented Feb 25, 2026 •

edited

Loading

stsewd Mar 2, 2026 •

edited

Loading

humitos commented Mar 3, 2026 •

edited

Loading