Skip to content

Commit b673130

Browse files
authored
Sync versions: don't fetch/return all versions (#8114)
We were using this only for logging, retrieving all versions from the db and making a string from it may be expensive. This may be related to the OOM error, but hard to say. Didn't see anything else that may be causing this.
1 parent d1e23e1 commit b673130

File tree

3 files changed

+25
-19
lines changed

3 files changed

+25
-19
lines changed

readthedocs/api/v2/utils.py

+10-10
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,10 @@ def sync_versions_to_db(project, versions, type): # pylint: disable=redefined-b
115115
latest_version.verbose_name = LATEST_VERBOSE_NAME
116116
latest_version.save()
117117
if added:
118-
log.info('(Sync Versions) Added Versions: [%s] ', ' '.join(added))
118+
log.info(
119+
'(Sync Versions) Added Versions: versions_count=%d versions=[%s]',
120+
len(added), ' '.join(itertools.islice(added, 100)),
121+
)
119122
return added
120123

121124

@@ -208,15 +211,12 @@ def delete_versions_from_db(project, tags_data, branches_data):
208211
)
209212
.exclude(active=True)
210213
)
211-
deleted_versions = set(to_delete_qs.values_list('slug', flat=True))
212-
if deleted_versions:
213-
log.info(
214-
'(Sync Versions) Deleted Versions: project=%s, versions=[%s]',
215-
project.slug, ' '.join(deleted_versions),
216-
)
217-
to_delete_qs.delete()
218-
219-
return deleted_versions
214+
_, deleted = to_delete_qs.delete()
215+
versions_count = deleted.get('builds.Version')
216+
log.info(
217+
'(Sync Versions) Deleted Versions: project=%s versions_count=[%d]',
218+
project.slug, versions_count,
219+
)
220220

221221

222222
def get_deleted_active_versions(project, tags_data, branches_data):

readthedocs/builds/tasks.py

+5-6
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,10 @@
1515
)
1616
from readthedocs.builds.constants import (
1717
BRANCH,
18-
EXTERNAL,
1918
BUILD_STATUS_FAILURE,
2019
BUILD_STATUS_PENDING,
2120
BUILD_STATUS_SUCCESS,
21+
EXTERNAL,
2222
MAX_BUILD_COMMAND_SIZE,
2323
TAG,
2424
)
@@ -255,7 +255,7 @@ def sync_versions_task(project_pk, tags_data, branches_data, **kwargs):
255255
256256
:param tags_data: List of dictionaries with ``verbose_name`` and ``identifier``.
257257
:param branches_data: Same as ``tags_data`` but for branches.
258-
:returns: the identifiers for the versions that have been deleted.
258+
:returns: `True` or `False` if the task succeeded.
259259
"""
260260
project = Project.objects.get(pk=project_pk)
261261

@@ -284,7 +284,7 @@ def sync_versions_task(project_pk, tags_data, branches_data, **kwargs):
284284
)
285285
added_versions.update(result)
286286

287-
deleted_versions = delete_versions_from_db(
287+
delete_versions_from_db(
288288
project=project,
289289
tags_data=tags_data,
290290
branches_data=branches_data,
@@ -296,7 +296,7 @@ def sync_versions_task(project_pk, tags_data, branches_data, **kwargs):
296296
)
297297
except Exception:
298298
log.exception('Sync Versions Error')
299-
return [], []
299+
return False
300300

301301
try:
302302
# The order of added_versions isn't deterministic.
@@ -333,5 +333,4 @@ def sync_versions_task(project_pk, tags_data, branches_data, **kwargs):
333333
promoted_version.active = True
334334
promoted_version.save()
335335
trigger_build(project=project, version=promoted_version)
336-
337-
return list(added_versions), list(deleted_versions)
336+
return True

readthedocs/rtd_tests/tests/test_sync_versions.py

+10-3
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ def setUp(self):
5656
type=TAG,
5757
)
5858
self.pip.update_stable_version()
59+
self.pip.save()
5960

6061
def test_proper_url_no_slash(self):
6162
branches_data = [
@@ -69,13 +70,19 @@ def test_proper_url_no_slash(self):
6970
},
7071
]
7172

72-
added_versions, deleted_versions = sync_versions_task(
73+
self.assertEqual(
74+
set(self.pip.versions.all().values_list('slug', flat=True)),
75+
{'master', 'latest', 'stable', '0.8.1', '0.8', 'to_delete'},
76+
)
77+
sync_versions_task(
7378
self.pip.pk,
7479
branches_data=branches_data,
7580
tags_data=[],
7681
)
77-
self.assertEqual(deleted_versions, ['to_delete'])
78-
self.assertEqual(added_versions, ['to_add'])
82+
self.assertEqual(
83+
set(self.pip.versions.all().values_list('slug', flat=True)),
84+
{'master', 'latest', 'stable', '0.8.1', '0.8', 'to_add'},
85+
)
7986

8087
def test_new_tag_update_active(self):
8188
Version.objects.create(

0 commit comments

Comments
 (0)