[desktop] Reattempt indexing if needed (#5160)

web/packages/new/photos/services/migration.ts

@@ -3,6 +3,7 @@ import { getKVN, removeKV, setKV } from "@/base/kv";
 import log from "@/base/log";
 import localForage from "@ente/shared/storage/localForage";
 import { deleteDB } from "idb";
+import { retryIndexingFailuresIfNeeded } from "./ml";
 
 /**
  * App specific migrations.
@@ -29,19 +30,22 @@ import { deleteDB } from "idb";
  */
 export const runMigrations = async () => {
     const m = (await getKVN("migrationLevel")) ?? 0;
-    const latest = 4;
+    const isNewInstall = m == 0;
+    const latest = 5;
     if (m < latest) {
         log.info(`Running migrations ${m} => ${latest}`);
         if (m < 1 && isDesktop) await m1();
         if (m < 2) await m2();
         if (m < 3) await m3();
         if (m < 4) m4();
+        if (m < 5) m5(isNewInstall);
         await setKV("migrationLevel", latest);
     }
 };
 
 // Some of these (indicated by "Prunable") can be no-oped in the future when
-// almost all clients would've migrated over.
+// almost all clients would've migrated over, and there wouldn't be any critical
+// impact if the few remaining outliers never ran that specific migration.
 
 // Added: Aug 2024 (v1.7.3). Prunable.
 const m1 = () =>
@@ -102,7 +106,7 @@ const m3 = () =>
         removeKV("latestUpdatedAt/location"),
     ]);
 
-// Added: Nov 2025 (v1.7.7-beta). Prunable.
+// Added: Nov 2024 (v1.7.7-beta). Prunable.
 const m4 = () => {
     // Delete old local storage keys that have been subsumed elsewhere.
     localStorage.removeItem("mapEnabled");
@@ -111,11 +115,14 @@ const m4 = () => {
     localStorage.removeItem("familyData");
 };
 
-// Future cleanup. Prunable so far.
-/*
-const m5 = () => {
+// Added: Feb 2025 (v1.7.10). Prunable.
+const m5 = (isNewInstall: boolean) => {
     // MUI now persists the color scheme (also in local storage). This was
     // anyway never released, was only ever an internal user flag.
     localStorage.removeItem("theme");
+    if (!isNewInstall) {
+        // Let the indexer have another go at the files, the new vips conversion
+        // logic added since 1.7.9 might be able to convert more outliers.
+        retryIndexingFailuresIfNeeded();
+    }
 };
-*/

web/packages/new/photos/services/ml/db.ts

@@ -314,6 +314,20 @@ export const updateAssumingLocalFiles = async (
     );
 };
 
+/**
+ * Remove all "failed" file status entries so that we again attempt to index
+ * those files the next time indexing happens.
+ */
+export const resetFailedFileStatuses = async () => {
+    const db = await mlDB();
+    const tx = db.transaction("file-status", "readwrite");
+    const ids = await tx.store
+        .index("status")
+        .getAllKeys(IDBKeyRange.only("failed"));
+
+    await Promise.all([ids.map((id) => tx.store.delete(id)), tx.done].flat());
+};
+
 /**
  * Return the count of files that can be, and that have been, indexed.
  *

web/packages/new/photos/services/ml/index.ts

@@ -25,7 +25,12 @@ import {
 import { deleteUserEntity } from "../user-entity/remote";
 import type { FaceCluster } from "./cluster";
 import { regenerateFaceCrops } from "./crop";
-import { clearMLDB, getIndexableAndIndexedCounts, savedFaceIndex } from "./db";
+import {
+    clearMLDB,
+    getIndexableAndIndexedCounts,
+    resetFailedFileStatuses,
+    savedFaceIndex,
+} from "./db";
 import {
     _applyPersonSuggestionUpdates,
     filterNamedPeople,
@@ -97,6 +102,12 @@ class MLState {
      */
     peopleStateSnapshot: PeopleState | undefined;
 
+    /**
+     * `true` if a reset has been requested via
+     * {@link retryIndexingFailuresIfNeeded}.
+     */
+    needsResetFailures = false;
+
     /**
      * In flight face crop regeneration promises indexed by the IDs of the files
      * whose faces we are regenerating.
@@ -294,6 +305,30 @@ const getIsMLEnabledRemote = () => getRemoteFlag(mlRemoteKey);
 const updateIsMLEnabledRemote = (enabled: boolean) =>
     updateRemoteFlag(mlRemoteKey, enabled);
 
+/**
+ * Reset failures so that indexing is attempted again.
+ *
+ * When indexing of some individual files fails for non-retriable reasons, we
+ * mark those as failures locally.
+ *
+ * See: [Note: Transient and permanent indexing failures].
+ *
+ * Sometimes we might wish to reattempt these though (e.g. when adding support
+ * for more file formats).
+ *
+ * In such cases, this function can be called early on (during an app version
+ * upgrade) to set an in-memory flag which tell us that before attemepting a
+ * sync, we should reset existing failed statii.
+ *
+ * Since this is not a critical operation, we only keep this as an in-memory
+ * flag, failure to honor this will not have permanent repercussions (e.g. the
+ * file would eventually get indexed on mobile, or during logout / login, or
+ * during the next time an reattempt is made).
+ */
+export const retryIndexingFailuresIfNeeded = () => {
+    _state.needsResetFailures = true;
+};
+
 /**
  * Sync the ML status with remote.
  *
@@ -330,6 +365,13 @@ export const mlSync = async () => {
     if (_state.isSyncing) return;
     _state.isSyncing = true;
 
+    if (_state.needsResetFailures) {
+        // CAS. See documentation for retryIndexingFailures why swapping the
+        // flag before performing the operation is fine.
+        _state.needsResetFailures = false;
+        await resetFailedFileStatuses();
+    }
+
     // Dependency order for the sync
     //
     //     files -> faces -> cgroups -> clusters -> people