Firestore: Split schema migrations into batches (#374)

Author: schmidt-sebastian

firebase-firestore/CHANGELOG.md

@@ -1,4 +1,8 @@
 # Unreleased
+- [fixed] Fixed an issue that prevented schema migrations for clients with
+  large offline datasets (#370).
+
+# To be released - 19.0.0
 - [feature] You can now query across all collections in your database with a
   given collection ID using the `FirebaseFirestore.collectionGroup()` method.
 - [changed] The garbage collection process for on-disk persistence that

firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteSchema.java

@@ -50,6 +50,15 @@ class SQLiteSchema {
   // Remove this constant and increment VERSION to enable indexing support
   static final int INDEXING_SUPPORT_VERSION = VERSION + 1;
 
+  /**
+   * The batch size for the sequence number migration in `ensureSequenceNumbers()`.
+   *
+   * <p>This addresses https://github.com/firebase/firebase-android-sdk/issues/370, where a customer
+   * reported that schema migrations failed for clients with thousands of documents. The number has
+   * been chosen based on manual experiments.
+   */
+  private static final int SEQUENCE_NUMBER_BATCH_SIZE = 100;
+
   private final SQLiteDatabase db;
 
   // PORTING NOTE: The Android client doesn't need to use a serializer to remove held write acks.
@@ -359,17 +368,30 @@ private void ensureSequenceNumbers() {
     SQLiteStatement tagDocument =
         db.compileStatement(
             "INSERT INTO target_documents (target_id, path, sequence_number) VALUES (0, ?, ?)");
+
     SQLitePersistence.Query untaggedDocumentsQuery =
         new SQLitePersistence.Query(
-            db,
-            "SELECT RD.path FROM remote_documents AS RD WHERE NOT EXISTS (SELECT TD.path FROM target_documents AS TD WHERE RD.path = TD.path AND TD.target_id = 0)");
-    untaggedDocumentsQuery.forEach(
-        row -> {
-          tagDocument.clearBindings();
-          tagDocument.bindString(1, row.getString(0));
-          tagDocument.bindLong(2, sequenceNumber);
-          hardAssert(tagDocument.executeInsert() != -1, "Failed to insert a sentinel row");
-        });
+                db,
+                "SELECT RD.path FROM remote_documents AS RD WHERE NOT EXISTS ("
+                    + "SELECT TD.path FROM target_documents AS TD "
+                    + "WHERE RD.path = TD.path AND TD.target_id = 0"
+                    + ") LIMIT ?")
+            .binding(SEQUENCE_NUMBER_BATCH_SIZE);
+
+    boolean[] resultsRemaining = new boolean[1];
+
+    do {
+      resultsRemaining[0] = false;
+
+      untaggedDocumentsQuery.forEach(
+          row -> {
+            resultsRemaining[0] = true;
+            tagDocument.clearBindings();
+            tagDocument.bindString(1, row.getString(0));
+            tagDocument.bindLong(2, sequenceNumber);
+            hardAssert(tagDocument.executeInsert() != -1, "Failed to insert a sentinel row");
+          });
+    } while (resultsRemaining[0]);
   }
 
   private void createV8CollectionParentsIndex() {

firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteSchemaTest.java

@@ -279,12 +279,12 @@ public void addsSentinelRows() {
     // Set up some documents (we only need the keys)
     // For the odd ones, add sentinel rows.
     for (int i = 0; i < 10; i++) {
-      String path = "docs/doc_" + i;
-      db.execSQL("INSERT INTO remote_documents (path) VALUES (?)", new String[] {path});
+      String encodedPath = encode(path("docs/doc_" + i));
+      db.execSQL("INSERT INTO remote_documents (path) VALUES (?)", new String[] {encodedPath});
       if (i % 2 == 1) {
         db.execSQL(
             "INSERT INTO target_documents (target_id, path, sequence_number) VALUES (0, ?, ?)",
-            new Object[] {path, oldSequenceNumber});
+            new Object[] {encodedPath, oldSequenceNumber});
       }
     }
 
@@ -295,7 +295,7 @@ public void addsSentinelRows() {
             db, "SELECT path, sequence_number FROM target_documents WHERE target_id = 0")
         .forEach(
             row -> {
-              String path = row.getString(0);
+              String path = decodeResourcePath(row.getString(0)).getLastSegment();
               long sequenceNumber = row.getLong(1);
 
               int docNum = Integer.parseInt(path.split("_", -1)[1]);
@@ -307,6 +307,42 @@ public void addsSentinelRows() {
             });
   }
 
+  @Test
+  public void addsSentinelRowsForLargeNumberOfDocuments() {
+    // This test only verifies that we add sentinal rows for documents even if the number of
+    // documents exceed our batch size. It does not verify that we do not hit a
+    // `SQLiteBlobTooBigException`, since this exception is not thrown when using SQLite through
+    // Robolectric.
+
+    // PORTING NOTE: This test only exists on Android since other clients do not need to split
+    // large data sets during schema migration.
+
+    schema.runMigrations(0, 6);
+
+    // Set up some documents (we only need the keys). Note this count is higher than the batch size
+    // during migration, which is 100.
+    int documentCount = 250;
+    for (int i = 0; i < documentCount; i++) {
+      String path = "coll/doc_" + i;
+      db.execSQL(
+          "INSERT INTO remote_documents (path) VALUES (?)", new String[] {encode(path(path))});
+    }
+
+    new SQLitePersistence.Query(db, "SELECT COUNT(*) FROM target_documents")
+        .first(
+            row -> {
+              assertEquals(0, row.getLong(0));
+            });
+
+    schema.runMigrations(6, 7);
+
+    new SQLitePersistence.Query(db, "SELECT COUNT(*) FROM target_documents")
+        .first(
+            row -> {
+              assertEquals(documentCount, row.getLong(0));
+            });
+  }
+
   @Test
   public void canCreateCollectionParentsIndex() {
     // This test creates a database with schema version 7 that has a few