Problem
FirestoreStateService.cleanup_old_states() in
src/youtube_extension/services/cloud/firestore_state.py deletes every expired
document one at a time:
# Delete in batch <- the comment is not true
count = 0
for doc in docs:
await doc.reference.delete()
count += 1
Each delete() is an independent network round-trip to Firestore. The loop
awaits each one before starting the next, so cleanup costs N sequential
round-trips and its wall-clock time scales linearly with the size of the
expired backlog. The # Delete in batch comment claims batching that the code
does not do.
There is a second, subtler problem: if any single delete raises, the exception
propagates out of the loop. Every remaining document is skipped, and the count
of documents that were already deleted is lost with the exception.
Reachability
This is live code, not a dead path — the module has 4 importers:
src/youtube_extension/services/cloud/cloud_video_processor.py
tests/unit/test_firestore_state.py
tests/unit/test_cloud_video_processor.py
src/youtube_extension/services/cloud/__init__.py
Acceptance criteria
Scope
- In scope: the delete fan-out inside
cleanup_old_states(), plus tests.
- Out of scope: the query/
where clause, caching behaviour, the module
singleton lifecycle, and any change to cleanup_old_states's signature.
Problem
FirestoreStateService.cleanup_old_states()insrc/youtube_extension/services/cloud/firestore_state.pydeletes every expireddocument one at a time:
Each
delete()is an independent network round-trip to Firestore. The loopawaits each one before starting the next, so cleanup costs N sequential
round-trips and its wall-clock time scales linearly with the size of the
expired backlog. The
# Delete in batchcomment claims batching that the codedoes not do.
There is a second, subtler problem: if any single delete raises, the exception
propagates out of the loop. Every remaining document is skipped, and the count
of documents that were already deleted is lost with the exception.
Reachability
This is live code, not a dead path — the module has 4 importers:
Acceptance criteria
in-flight RPCs against Firestore.
implementation (non-vacuity).
Scope
cleanup_old_states(), plus tests.whereclause, caching behaviour, the modulesingleton lifecycle, and any change to
cleanup_old_states's signature.