use sequence numbers instead of read time in collection group

Author: Brian Chen

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

@@ -18,7 +18,6 @@
 
 import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
-import com.google.firebase.Timestamp;
 import com.google.firebase.database.collection.ImmutableSortedMap;
 import com.google.firebase.firestore.core.Query;
 import com.google.firebase.firestore.model.Document;
@@ -140,11 +139,14 @@ public Results backfill() {
   /** Writes index entries until the cap is reached. Returns the number of documents processed. */
   private int writeIndexEntries(LocalDocumentsView localDocumentsView) {
     int documentsProcessed = 0;
-    Timestamp startingTimestamp = Timestamp.now();
+    int currentMaxSequenceNumber = indexManager.getMaxCollectionGroupSequenceNumber();
 
+    // TODO(indexing): Handle pausing and resuming from the correct document if backfilling hits the
+    // max doc limit while processing docs for a certain read time.
     while (documentsProcessed < maxDocumentsToProcess) {
       int documentsRemaining = maxDocumentsToProcess - documentsProcessed;
-      String collectionGroup = indexManager.getNextCollectionGroupToUpdate(startingTimestamp);
+      String collectionGroup =
+          indexManager.getNextCollectionGroupToUpdate(currentMaxSequenceNumber);
       if (collectionGroup == null) {
         break;
       }
@@ -157,7 +159,7 @@ private int writeIndexEntries(LocalDocumentsView localDocumentsView) {
 
   /** Writes entries for the fetched field indexes. */
   private int writeEntriesForCollectionGroup(
-      LocalDocumentsView localDocumentsView, String collectionGroup, int entriesRemainingUnderCap) {
+      LocalDocumentsView localDocumentsView, String collectionGroup, int documentsRemaining) {
     Query query = new Query(ResourcePath.EMPTY, collectionGroup);
 
     // Use the earliest updateTime of all field indexes as the base updateTime.
@@ -169,7 +171,7 @@ private int writeEntriesForCollectionGroup(
     ImmutableSortedMap<DocumentKey, Document> documents =
         localDocumentsView.getDocumentsMatchingQuery(query, earliestUpdateTime);
 
-    Queue<Document> oldestDocuments = getOldestDocuments(documents, entriesRemainingUnderCap);
+    Queue<Document> oldestDocuments = getOldestDocuments(documents, documentsRemaining);
     indexManager.updateIndexEntries(oldestDocuments);
     return oldestDocuments.size();
   }

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

@@ -15,7 +15,6 @@
 package com.google.firebase.firestore.local;
 
 import androidx.annotation.Nullable;
-import com.google.firebase.Timestamp;
 import com.google.firebase.firestore.core.Target;
 import com.google.firebase.firestore.model.Document;
 import com.google.firebase.firestore.model.DocumentKey;
@@ -81,9 +80,15 @@ public interface IndexManager {
   /** Returns the documents that match the given target based on the provided index. */
   Set<DocumentKey> getDocumentsMatchingTarget(FieldIndex fieldIndex, Target target);
 
-  /** Returns the next collection group to update. */
+  /**
+   * Returns the next collection group to update. Returns null after reaching the provided max
+   * sequence number.
+   */
   @Nullable
-  String getNextCollectionGroupToUpdate(Timestamp lastUpdateTime);
+  String getNextCollectionGroupToUpdate(int currentMaxSequenceNumber);
+
+  /** Returns the highest sequence number in the collection groups table. */
+  int getMaxCollectionGroupSequenceNumber();
 
   /**
    * Updates the index entries for the provided documents and corresponding field indexes until the

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

@@ -16,7 +16,6 @@
 import static com.google.firebase.firestore.util.Assert.hardAssert;
 
 import androidx.annotation.Nullable;
-import com.google.firebase.Timestamp;
 import com.google.firebase.firestore.core.Target;
 import com.google.firebase.firestore.model.Document;
 import com.google.firebase.firestore.model.DocumentKey;
@@ -75,11 +74,17 @@ public Set<DocumentKey> getDocumentsMatchingTarget(FieldIndex fieldIndex, Target
   }
 
   @Override
-  public String getNextCollectionGroupToUpdate(Timestamp startingTimestamp) {
+  public String getNextCollectionGroupToUpdate(int currentMaxSequenceNumber) {
     // Field indices are not supported with memory persistence.
     return null;
   }
 
+  @Override
+  public int getMaxCollectionGroupSequenceNumber() {
+    // Field indices are not supported with memory persistence.
+    return -1;
+  }
+
   @Override
   public Collection<FieldIndex> getFieldIndexes(String collectionGroup) {
     // Field indices are not supported with memory persistence.

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

@@ -22,7 +22,6 @@
 
 import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
-import com.google.firebase.Timestamp;
 import com.google.firebase.firestore.auth.User;
 import com.google.firebase.firestore.core.Bound;
 import com.google.firebase.firestore.core.FieldFilter;
@@ -55,6 +54,7 @@
 /** A persisted implementation of IndexManager. */
 final class SQLiteIndexManager implements IndexManager {
   private static final String TAG = SQLiteIndexManager.class.getSimpleName();
+  private static final int NEW_COLLECTION_GROUP_SEQUENCE_NUMBER = 0;
 
   /**
    * An in-memory copy of the index entries we've already written since the SDK launched. Used to
@@ -161,8 +161,7 @@ public void addFieldIndex(FieldIndex index) {
         index.getUpdateTime().getTimestamp().getSeconds(),
         index.getUpdateTime().getTimestamp().getNanoseconds());
 
-    // TODO(indexing): Use a 0-counter rather than the timestamp.
-    setCollectionGroupUpdateTime(index.getCollectionGroup(), SnapshotVersion.NONE.getTimestamp());
+    setCollectionGroup(index.getCollectionGroup(), NEW_COLLECTION_GROUP_SEQUENCE_NUMBER);
 
     memoizeIndex(index);
   }
@@ -184,19 +183,18 @@ private void updateFieldIndex(FieldIndex index) {
     memoizeIndex(index);
   }
 
-  // TODO(indexing): Use a counter rather than the starting timestamp.
   @Override
-  public @Nullable String getNextCollectionGroupToUpdate(Timestamp startingTimestamp) {
+  public @Nullable String getNextCollectionGroupToUpdate(int currentMaxSequenceNumber) {
     hardAssert(started, "IndexManager not started");
 
     final String[] nextCollectionGroup = {null};
     db.query(
             "SELECT collection_group "
                 + "FROM collection_group_update_times "
-                + "WHERE update_time_seconds <= ? AND update_time_nanos <= ?"
-                + "ORDER BY update_time_seconds, update_time_nanos "
+                + "WHERE sequence_number <= ?"
+                + "ORDER BY sequence_number "
                 + "LIMIT 1")
-        .binding(startingTimestamp.getSeconds(), startingTimestamp.getNanoseconds())
+        .binding(currentMaxSequenceNumber)
         .firstValue(
             row -> {
               nextCollectionGroup[0] = row.getString(0);
@@ -208,9 +206,7 @@ private void updateFieldIndex(FieldIndex index) {
     }
 
     // Store that this collection group will updated.
-    // TODO(indexing): Store progress with a counter rather than a timestamp. If using a timestamp,
-    // use the read time of the last document read in the loop.
-    setCollectionGroupUpdateTime(nextCollectionGroup[0], Timestamp.now());
+    setCollectionGroup(nextCollectionGroup[0], getMaxCollectionGroupSequenceNumber() + 1);
 
     return nextCollectionGroup[0];
   }
@@ -238,8 +234,6 @@ public void updateIndexEntries(Collection<Document> documents) {
       }
     }
 
-    // TODO(indexing): Use RemoteDocumentCache's readTime version rather than the document version.
-    // This will require plumbing out the RDC's readTime into the IndexBackfiller.
     for (FieldIndex updatedFieldIndex : updatedFieldIndexes.values()) {
       updateFieldIndex(updatedFieldIndex);
     }
@@ -636,13 +630,20 @@ private byte[] encodeFieldIndex(FieldIndex fieldIndex) {
   }
 
   @VisibleForTesting
-  void setCollectionGroupUpdateTime(String collectionGroup, Timestamp updateTime) {
+  void setCollectionGroup(String collectionGroup, int sequenceNumber) {
     db.execute(
         "INSERT OR REPLACE INTO collection_group_update_times "
-            + "(collection_group, update_time_seconds, update_time_nanos) "
-            + "VALUES (?, ?, ?)",
+            + "(collection_group, sequence_number) "
+            + "VALUES (?, ?)",
         collectionGroup,
-        updateTime.getSeconds(),
-        updateTime.getNanoseconds());
+        sequenceNumber);
+  }
+
+  @Override
+  public int getMaxCollectionGroupSequenceNumber() {
+    final int[] finalValue = {0};
+    db.query("SELECT MAX(sequence_number) FROM collection_group_update_times")
+        .first(value -> finalValue[0] = value.getInt(0));
+    return finalValue[0];
   }
 }

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

@@ -401,8 +401,7 @@ private void createCollectionGroupsTable() {
           db.execSQL(
               "CREATE TABLE collection_group_update_times ("
                   + "collection_group TEXT, " // Name of the collection group.
-                  + "update_time_seconds INTEGER," // Time of last index backfill update
-                  + "update_time_nanos INTEGER,"
+                  + "sequence_number INTEGER, " // Used to establish ordering when indexing.
                   + "PRIMARY KEY (collection_group))");
         });
   }

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

@@ -15,7 +15,7 @@
 package com.google.firebase.firestore.local;
 
 import static com.google.common.truth.Truth.assertThat;
-import static com.google.firebase.firestore.local.SQLiteIndexManagerTest.getCollectionGroupsOrderByUpdateTime;
+import static com.google.firebase.firestore.local.SQLiteIndexManagerTest.getCollectionGroupsOrderBySequenceNumber;
 import static com.google.firebase.firestore.testutil.TestUtil.doc;
 import static com.google.firebase.firestore.testutil.TestUtil.field;
 import static com.google.firebase.firestore.testutil.TestUtil.key;
@@ -25,7 +25,6 @@
 import static com.google.firebase.firestore.testutil.TestUtil.version;
 import static junit.framework.TestCase.assertEquals;
 
-import com.google.firebase.Timestamp;
 import com.google.firebase.firestore.auth.User;
 import com.google.firebase.firestore.core.Target;
 import com.google.firebase.firestore.model.DocumentKey;
@@ -40,7 +39,6 @@
 import org.junit.AfterClass;
 import org.junit.Before;
 import org.junit.BeforeClass;
-import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TestName;
@@ -95,9 +93,7 @@ public void tearDown() {
     persistence.shutdown();
   }
 
-  // TODO(indexing): Re-enable this test once we have counters implemented.
   @Test
-  @Ignore
   public void testBackfillWritesLatestReadTimeToFieldIndexOnCompletion() {
     addFieldIndex("coll1", "foo");
     addFieldIndex("coll2", "bar");
@@ -127,8 +123,6 @@ public void testBackfillWritesLatestReadTimeToFieldIndexOnCompletion() {
   }
 
   @Test
-  @Ignore("Flaky")
-  // TODO(indexing): This test is flaky. Fix.
   public void testBackfillFetchesDocumentsAfterEarliestReadTime() {
     addFieldIndex("coll1", "foo", version(10, 0));
     addFieldIndex("coll1", "boo", version(20, 0));
@@ -196,9 +190,9 @@ public void testBackfillUpdatesCollectionGroups() {
     addFieldIndex("coll1", "foo");
     addFieldIndex("coll2", "foo");
     addFieldIndex("coll3", "foo");
-    addCollectionGroup("coll1", new Timestamp(30, 0));
-    addCollectionGroup("coll2", new Timestamp(30, 30));
-    addCollectionGroup("coll3", new Timestamp(10, 0));
+    addCollectionGroup("coll1", 2);
+    addCollectionGroup("coll2", 3);
+    addCollectionGroup("coll3", 1);
     addDoc("coll1/docA", "foo", version(10, 0));
     addDoc("coll2/docA", "foo", version(10, 0));
     addDoc("coll3/docA", "foo", version(10, 0));
@@ -208,31 +202,29 @@ public void testBackfillUpdatesCollectionGroups() {
 
     // Check that index entries are written in order of the collection group update times by
     // verifying the collection group update times have been updated in the correct order.
-    List<String> collectionGroups = getCollectionGroupsOrderByUpdateTime(persistence);
+    List<String> collectionGroups = getCollectionGroupsOrderBySequenceNumber(persistence);
     assertEquals(3, collectionGroups.size());
     assertEquals("coll3", collectionGroups.get(0));
     assertEquals("coll1", collectionGroups.get(1));
     assertEquals("coll2", collectionGroups.get(2));
   }
 
   @Test
-  @Ignore("Flaky")
-  // TODO(indexing): This test is flaky. Fix.
   public void testBackfillPrioritizesNewCollectionGroups() {
     // In this test case, `coll3` is a new collection group that hasn't been indexed, so it should
     // be processed ahead of the other collection groups.
     addFieldIndex("coll1", "foo");
     addFieldIndex("coll2", "foo");
-    addCollectionGroup("coll1", new Timestamp(1, 0));
-    addCollectionGroup("coll2", new Timestamp(2, 0));
+    addCollectionGroup("coll1", 1);
+    addCollectionGroup("coll2", 2);
     addFieldIndex("coll3", "foo");
 
     IndexBackfiller.Results results = backfiller.backfill();
     assertEquals(0, results.getDocumentsProcessed());
 
     // Check that index entries are written in order of the collection group update times by
     // verifying the collection group update times have been updated in the correct order.
-    List<String> collectionGroups = getCollectionGroupsOrderByUpdateTime(persistence);
+    List<String> collectionGroups = getCollectionGroupsOrderBySequenceNumber(persistence);
     assertEquals(3, collectionGroups.size());
     assertEquals("coll3", collectionGroups.get(0));
     assertEquals("coll1", collectionGroups.get(1));
@@ -244,7 +236,7 @@ public void testBackfillWritesUntilCap() {
     backfiller.setMaxDocumentsToProcess(3);
     addFieldIndex("coll1", "foo");
     addFieldIndex("coll2", "foo");
-    addCollectionGroup("coll1", new Timestamp(1, 0));
+    addCollectionGroup("coll1", 1);
     addDoc("coll1/docA", "foo", version(10, 0));
     addDoc("coll1/docB", "foo", version(10, 0));
     addDoc("coll2/docA", "foo", version(10, 0));
@@ -256,7 +248,7 @@ public void testBackfillWritesUntilCap() {
     // Check that collection groups are updated even if the backfiller hits the write cap. Since
     // `coll1` was already in the table, `coll2` should be processed first, and thus appear first
     // in the ordering.
-    List<String> collectionGroups = getCollectionGroupsOrderByUpdateTime(persistence);
+    List<String> collectionGroups = getCollectionGroupsOrderBySequenceNumber(persistence);
     assertEquals(2, collectionGroups.size());
     assertEquals("coll2", collectionGroups.get(0));
     assertEquals("coll1", collectionGroups.get(1));
@@ -276,8 +268,8 @@ private void addFieldIndex(String collectionGroup, String fieldName, SnapshotVer
             .withUpdateTime(readTime));
   }
 
-  private void addCollectionGroup(String collectionGroup, Timestamp updateTime) {
-    indexManager.setCollectionGroupUpdateTime(collectionGroup, updateTime);
+  private void addCollectionGroup(String collectionGroup, int sequenceNumber) {
+    indexManager.setCollectionGroup(collectionGroup, sequenceNumber);
   }
 
   /** Creates a document and adds it to the RemoteDocumentCache. */

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

@@ -606,18 +606,18 @@ public void testUpdateTime() {
   }
 
   @Test
-  public void testCollectionGroupUpdateTimesCanBeUpdated() {
+  public void testCollectionGroupSequenceNumbersCanBeUpdated() {
     SQLiteIndexManager sqLiteIndexManager = (SQLiteIndexManager) indexManager;
-    sqLiteIndexManager.setCollectionGroupUpdateTime("coll1", new Timestamp(30, 0));
-    sqLiteIndexManager.setCollectionGroupUpdateTime("coll2", new Timestamp(30, 50));
+    sqLiteIndexManager.setCollectionGroup("coll1", 1);
+    sqLiteIndexManager.setCollectionGroup("coll2", 2);
     List<String> orderedCollectionGroups =
-        getCollectionGroupsOrderByUpdateTime((SQLitePersistence) getPersistence());
+        getCollectionGroupsOrderBySequenceNumber((SQLitePersistence) getPersistence());
     List<String> expected = Arrays.asList("coll1", "coll2");
     assertEquals(expected, orderedCollectionGroups);
 
-    sqLiteIndexManager.setCollectionGroupUpdateTime("coll1", new Timestamp(50, 0));
+    sqLiteIndexManager.setCollectionGroup("coll1", 3);
     orderedCollectionGroups =
-        getCollectionGroupsOrderByUpdateTime((SQLitePersistence) getPersistence());
+        getCollectionGroupsOrderBySequenceNumber((SQLitePersistence) getPersistence());
     expected = Arrays.asList("coll2", "coll1");
     assertEquals(expected, orderedCollectionGroups);
   }
@@ -662,12 +662,22 @@ public void testAddFieldIndexWritesToCollectionGroup() {
             .withAddedField(field("value"), FieldIndex.Segment.Kind.CONTAINS)
             .withUpdateTime(SnapshotVersion.NONE));
     List<String> collectionGroups =
-        getCollectionGroupsOrderByUpdateTime((SQLitePersistence) getPersistence());
+        getCollectionGroupsOrderBySequenceNumber((SQLitePersistence) getPersistence());
     assertEquals(2, collectionGroups.size());
     assertEquals("coll1", collectionGroups.get(0));
     assertEquals("coll2", collectionGroups.get(1));
   }
 
+  @Test
+  public void getMaxCollectionSequenceNumberOnEmptyTable() {
+    SQLiteIndexManager sqLiteIndexManager = (SQLiteIndexManager) indexManager;
+    List<String> collectionGroups =
+        getCollectionGroupsOrderBySequenceNumber((SQLitePersistence) getPersistence());
+    assertEquals(0, collectionGroups.size());
+    int value = sqLiteIndexManager.getMaxCollectionGroupSequenceNumber();
+    assertEquals(0, value);
+  }
+
   private void addDoc(String key, Map<String, Object> data) {
     MutableDocument doc = doc(key, 1, data);
     indexManager.handleDocumentChange(null, doc);
@@ -681,13 +691,14 @@ private void verifyResults(Query query, String... documents) {
     assertWithMessage("Result for %s", query).that(results).containsExactlyElementsIn(keys);
   }
 
-  public static List<String> getCollectionGroupsOrderByUpdateTime(SQLitePersistence persistence) {
+  public static List<String> getCollectionGroupsOrderBySequenceNumber(
+      SQLitePersistence persistence) {
     List<String> orderedCollectionGroups = new ArrayList<>();
     persistence
         .query(
             "SELECT collection_group "
                 + "FROM collection_group_update_times "
-                + "ORDER BY update_time_seconds, update_time_nanos")
+                + "ORDER BY sequence_number")
         .forEach(
             row -> {
               orderedCollectionGroups.add(row.getString(0));