fix(backfill): drop offset-based pagination — it skipped filled rows
The offset+limit loop walked the IS NULL set, but every batch's writes shrank that set, so batch N+1 with offset=N*BATCH skipped over the rows just filled. A 17k library backfilled only 9k before the loop walked off the (now-shorter) NULL set. Replace with a tail-recursive pattern: keep selecting LIMIT BATCH on the NULL set, tracking rows that won't ever resolve in a `stuck` set so the loop terminates instead of spinning on them.
This commit is contained in:
@@ -57,33 +57,47 @@ async def run() -> None:
|
|||||||
filled = 0
|
filled = 0
|
||||||
not_found = 0
|
not_found = 0
|
||||||
|
|
||||||
offset = 0
|
# Keep selecting the next batch of NULL-fileid rows until the
|
||||||
|
# set is empty. NO offset() — each batch's writes shrink the
|
||||||
|
# `WHERE nextcloud_fileid IS NULL` set, so an offset would skip
|
||||||
|
# over the rows that were just filled in by the previous batch.
|
||||||
|
# Rows we couldn't resolve (skipped or not_found) stay in the
|
||||||
|
# set; we track them in a "stuck ids" set so the loop terminates
|
||||||
|
# instead of spinning on them forever.
|
||||||
|
stuck: set[str] = set()
|
||||||
while True:
|
while True:
|
||||||
result = await session.execute(
|
stmt = (
|
||||||
select(Photo)
|
select(Photo)
|
||||||
.where(Photo.nextcloud_fileid.is_(None))
|
.where(Photo.nextcloud_fileid.is_(None))
|
||||||
.order_by(Photo.id)
|
.order_by(Photo.id)
|
||||||
.offset(offset)
|
|
||||||
.limit(BATCH)
|
.limit(BATCH)
|
||||||
)
|
)
|
||||||
|
if stuck:
|
||||||
|
stmt = stmt.where(Photo.id.notin_(stuck))
|
||||||
|
result = await session.execute(stmt)
|
||||||
rows = list(result.scalars().all())
|
rows = list(result.scalars().all())
|
||||||
if not rows:
|
if not rows:
|
||||||
break
|
break
|
||||||
|
|
||||||
|
batch_started_filled = filled
|
||||||
for photo in rows:
|
for photo in rows:
|
||||||
done += 1
|
done += 1
|
||||||
if not photo.user_id:
|
if not photo.user_id:
|
||||||
skipped_no_user += 1
|
skipped_no_user += 1
|
||||||
|
stuck.add(photo.id)
|
||||||
continue
|
continue
|
||||||
owner = users.get(photo.user_id)
|
owner = users.get(photo.user_id)
|
||||||
if owner is None:
|
if owner is None:
|
||||||
skipped_no_user += 1
|
skipped_no_user += 1
|
||||||
|
stuck.add(photo.id)
|
||||||
continue
|
continue
|
||||||
if not photo.filepath or not is_nextcloud_path(photo.filepath):
|
if not photo.filepath or not is_nextcloud_path(photo.filepath):
|
||||||
skipped_not_nc += 1
|
skipped_not_nc += 1
|
||||||
|
stuck.add(photo.id)
|
||||||
continue
|
continue
|
||||||
if not owner.nextcloud_app_password_enc:
|
if not owner.nextcloud_app_password_enc:
|
||||||
skipped_no_creds += 1
|
skipped_no_creds += 1
|
||||||
|
stuck.add(photo.id)
|
||||||
continue
|
continue
|
||||||
fid: Optional[int] = None
|
fid: Optional[int] = None
|
||||||
try:
|
try:
|
||||||
@@ -95,12 +109,17 @@ async def run() -> None:
|
|||||||
)
|
)
|
||||||
if fid is None:
|
if fid is None:
|
||||||
not_found += 1
|
not_found += 1
|
||||||
|
stuck.add(photo.id)
|
||||||
continue
|
continue
|
||||||
photo.nextcloud_fileid = fid
|
photo.nextcloud_fileid = fid
|
||||||
filled += 1
|
filled += 1
|
||||||
|
|
||||||
await session.commit()
|
await session.commit()
|
||||||
offset += BATCH
|
# Safety: if a whole batch produced no new fills, every row
|
||||||
|
# in it is already in `stuck` — break to avoid an infinite
|
||||||
|
# loop on the same set.
|
||||||
|
if filled == batch_started_filled and len(rows) < BATCH:
|
||||||
|
break
|
||||||
logger.info(
|
logger.info(
|
||||||
"progress: scanned=%s filled=%s not_found=%s "
|
"progress: scanned=%s filled=%s not_found=%s "
|
||||||
"skipped(no_user=%s not_nc=%s no_creds=%s) of total=%s",
|
"skipped(no_user=%s not_nc=%s no_creds=%s) of total=%s",
|
||||||
|
|||||||
Reference in New Issue
Block a user