diff --git a/PROJECT.org b/PROJECT.org index c6fb915..feb2e6d 100644 --- a/PROJECT.org +++ b/PROJECT.org @@ -88,7 +88,7 @@ fetching and simple saving. *** Metadata sources **** Scraper -* Backlog [0/20] :vrobbler:project:personal: +* Backlog [1/21] :vrobbler:project:personal: ** TODO [#C] Create small utility to clean up tracks scrobbled with wonky playback times :bug:music:scrobbles: :PROPERTIES: :ID: 702462cf-d54b-48c6-8a7c-78b8de751deb @@ -594,6 +594,10 @@ named constants for maintainability. - ~vrobbler/apps/webpages/models.py~ (line 290) -- ="url"= - ~vrobbler/apps/scrobbles/importers/tsv.py~ (line 55) -- ="S"= completion status +** DONE [#A] Fix bug in recomputing long play seconds taking forever :bug:longplay:commands: +:PROPERTIES: +:ID: 0a813cf9-17fb-dbd7-b5a7-7410d9bd4d8c +:END: * Version 52.1 [1/1] ** DONE [#C] Show time per scrobble in long play lists and total time playing :templates:longplay:scrobbles: :PROPERTIES: diff --git a/vrobbler/apps/scrobbles/management/commands/recompute_long_play_seconds.py b/vrobbler/apps/scrobbles/management/commands/recompute_long_play_seconds.py index 7430f60..729cb0f 100644 --- a/vrobbler/apps/scrobbles/management/commands/recompute_long_play_seconds.py +++ b/vrobbler/apps/scrobbles/management/commands/recompute_long_play_seconds.py @@ -4,11 +4,14 @@ from django.db import connection from scrobbles.constants import LONG_PLAY_MEDIA from scrobbles.models import Scrobble +BATCH_SIZE = 1000 + class Command(BaseCommand): help = ( "Backfill long_play_last_scrobble FK chains, then recompute " - "long_play_seconds by walking backward through the chain." + "long_play_seconds by walking forward through scrobbles in " + "timestamp order with a running accumulator." ) def add_arguments(self, parser): @@ -61,49 +64,74 @@ class Command(BaseCommand): # Step 2: recompute long_play_seconds self.stdout.write( - "\nStep 2: Recomputing long_play_seconds via FK chain..." + "\nStep 2: Recomputing long_play_seconds in timestamp order..." ) - scrobbles = Scrobble.objects.filter( - media_type__in=media_types, - playback_position_seconds__isnull=False, - ).order_by("-timestamp") - - total = scrobbles.count() - self.stdout.write(f" Found {total} long play scrobbles to process") - - to_update = [] - for scrobble in scrobbles.iterator(): - accumulated = scrobble.playback_position_seconds or 0 - current = scrobble.long_play_last_scrobble - while current and not current.long_play_complete: - accumulated += current.playback_position_seconds or 0 - current = current.long_play_last_scrobble - - if scrobble.long_play_seconds != accumulated: - self.stdout.write( - f" Scrobble {scrobble.id} ({scrobble.media_type}): " - f"{scrobble.long_play_seconds or 0} -> {accumulated}" - ) - if not dry_run: - scrobble.long_play_seconds = accumulated - to_update.append(scrobble) - - if to_update: - Scrobble.objects.bulk_update(to_update, ["long_play_seconds"]) + total_updated = 0 + for mt in media_types: + n = self._recompute_for_media_type(mt, dry_run) + total_updated += n + self.stdout.write(f" {mt}: {n} scrobbles updated") if dry_run: self.stdout.write( self.style.WARNING( - f"Dry run: would update {len(to_update)} scrobbles. " + f"Dry run: would update {total_updated} scrobbles total. " "Use without --dry-run to apply." ) ) else: self.stdout.write( - self.style.SUCCESS(f"Updated {len(to_update)} scrobbles") + self.style.SUCCESS(f"Updated {total_updated} scrobbles") ) + def _recompute_for_media_type(self, media_type: str, dry_run: bool) -> int: + """Process scrobbles for a single media type in timestamp order with a + running accumulator, avoiding O(n2) FK chain walks.""" + fk = _media_type_to_fk(media_type) + fk_id = f"{fk}_id" + + scrobbles = Scrobble.objects.filter( + media_type=media_type, + **{f"{fk}__isnull": False}, + playback_position_seconds__isnull=False, + ).order_by(fk_id, "user_id", "timestamp") + + total = scrobbles.count() + if not total: + return 0 + + updated = 0 + batch = [] + last_key = None + running_total = 0 + + for scrobble in scrobbles.iterator(): + key = (getattr(scrobble, fk_id), scrobble.user_id) + + if key != last_key: + running_total = 0 + last_key = key + + running_total += scrobble.playback_position_seconds or 0 + + if scrobble.long_play_seconds != running_total: + updated += 1 + if not dry_run: + scrobble.long_play_seconds = running_total + batch.append(scrobble) + if len(batch) >= BATCH_SIZE: + Scrobble.objects.bulk_update(batch, ["long_play_seconds"]) + batch = [] + + if scrobble.long_play_complete: + running_total = 0 + + if batch: + Scrobble.objects.bulk_update(batch, ["long_play_seconds"]) + + return updated + def _backfill_chain(self, media_type: str, dry_run: bool) -> int: """Set long_play_last_scrobble on each scrobble to the previous scrobble for the same media+user using a single UPDATE with a