Merge remote-tracking branch 'origin/mila/BloomFilter' into BloomFilterComplexIntegrationTest

Author: dconeybe

firebase-firestore/src/main/java/com/google/firebase/firestore/core/SyncEngine.java

@@ -428,7 +428,7 @@ public void handleRejectedListen(int targetId, Status error) {
           new RemoteEvent(
               SnapshotVersion.NONE,
               /* targetChanges= */ Collections.emptyMap(),
-              /* targetMismatches= */ Collections.emptySet(),
+              /* targetMismatches= */ Collections.emptyMap(),
               documentUpdates,
               limboDocuments);
       handleRemoteEvent(event);

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

@@ -425,7 +425,7 @@ public ImmutableSortedMap<DocumentKey, Document> applyRemoteEvent(RemoteEvent re
             targetCache.addMatchingKeys(change.getAddedDocuments(), targetId);
 
             TargetData newTargetData = oldTargetData.withSequenceNumber(sequenceNumber);
-            if (remoteEvent.getTargetMismatches().contains(targetId)) {
+            if (remoteEvent.getTargetMismatches().containsKey(targetId)) {
               newTargetData =
                   newTargetData
                       .withResumeToken(ByteString.EMPTY, SnapshotVersion.NONE)

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

@@ -22,6 +22,11 @@ public enum QueryPurpose {
   /** The query was used to refill a query after an existence filter mismatch. */
   EXISTENCE_FILTER_MISMATCH,
 
+  /**
+   * The query target was used if the query is the result of a false positive in the bloom filter.
+   */
+  EXISTENCE_FILTER_MISMATCH_BLOOM,
+
   /** The query was used to resolve a limbo document. */
   LIMBO_RESOLUTION,
 }

firebase-firestore/src/main/java/com/google/firebase/firestore/remote/RemoteEvent.java

@@ -14,6 +14,7 @@
 
 package com.google.firebase.firestore.remote;
 
+import com.google.firebase.firestore.local.QueryPurpose;
 import com.google.firebase.firestore.model.DocumentKey;
 import com.google.firebase.firestore.model.MutableDocument;
 import com.google.firebase.firestore.model.SnapshotVersion;
@@ -27,14 +28,14 @@
 public final class RemoteEvent {
   private final SnapshotVersion snapshotVersion;
   private final Map<Integer, TargetChange> targetChanges;
-  private final Set<Integer> targetMismatches;
+  private final Map<Integer, QueryPurpose> targetMismatches;
   private final Map<DocumentKey, MutableDocument> documentUpdates;
   private final Set<DocumentKey> resolvedLimboDocuments;
 
   public RemoteEvent(
       SnapshotVersion snapshotVersion,
       Map<Integer, TargetChange> targetChanges,
-      Set<Integer> targetMismatches,
+      Map<Integer, QueryPurpose> targetMismatches,
       Map<DocumentKey, MutableDocument> documentUpdates,
       Set<DocumentKey> resolvedLimboDocuments) {
     this.snapshotVersion = snapshotVersion;
@@ -55,10 +56,10 @@ public Map<Integer, TargetChange> getTargetChanges() {
   }
 
   /**
-   * Returns a set of targets that is known to be inconsistent. Listens for these targets should be
-   * re-established without resume tokens.
+   * Returns a map of targets that is known to be inconsistent, and the purpose for re-listening.
+   * Listens for these targets should be re-established without resume tokens.
    */
-  public Set<Integer> getTargetMismatches() {
+  public Map<Integer, QueryPurpose> getTargetMismatches() {
     return targetMismatches;
   }
 

firebase-firestore/src/main/java/com/google/firebase/firestore/remote/RemoteSerializer.java

@@ -471,6 +471,8 @@ private String encodeLabel(QueryPurpose purpose) {
         return null;
       case EXISTENCE_FILTER_MISMATCH:
         return "existence-filter-mismatch";
+      case EXISTENCE_FILTER_MISMATCH_BLOOM:
+        return "existence-filter-mismatch-bloom";
       case LIMBO_RESOLUTION:
         return "limbo-document";
       default:

firebase-firestore/src/main/java/com/google/firebase/firestore/remote/RemoteStore.java

@@ -547,7 +547,8 @@ private void raiseWatchSnapshot(SnapshotVersion snapshotVersion) {
 
     // Re-establish listens for the targets that have been invalidated by existence filter
     // mismatches.
-    for (int targetId : remoteEvent.getTargetMismatches()) {
+    for (Map.Entry<Integer, QueryPurpose> entry : remoteEvent.getTargetMismatches().entrySet()) {
+      int targetId = entry.getKey();
       TargetData targetData = this.listenTargets.get(targetId);
       // A watched target might have been removed already.
       if (targetData != null) {
@@ -569,7 +570,7 @@ private void raiseWatchSnapshot(SnapshotVersion snapshotVersion) {
                 targetData.getTarget(),
                 targetId,
                 targetData.getSequenceNumber(),
-                QueryPurpose.EXISTENCE_FILTER_MISMATCH);
+                /*purpose=*/ entry.getValue());
         this.sendWatchRequest(requestTargetData);
       }
     }

firebase-firestore/src/main/java/com/google/firebase/firestore/remote/WatchChangeAggregator.java

@@ -75,14 +75,21 @@ public interface TargetMetadataProvider {
   private Map<DocumentKey, Set<Integer>> pendingDocumentTargetMapping = new HashMap<>();
 
   /**
-   * A list of targets with existence filter mismatches. These targets are known to be inconsistent
+   * A map of targets with existence filter mismatches. These targets are known to be inconsistent
    * and their listens needs to be re-established by RemoteStore.
    */
-  private Set<Integer> pendingTargetResets = new HashSet<>();
+  private Map<Integer, QueryPurpose> pendingTargetResets = new HashMap<>();
 
   /** The log tag to use for this class. */
   private static final String LOG_TAG = "WatchChangeAggregator";
 
+  /** The bloom filter application status while handling existence filter mismatch. */
+  private enum BloomFilterApplicationStatus {
+    SUCCESS,
+    SKIPPED,
+    FALSE_POSITIVE
+  }
+
   public WatchChangeAggregator(TargetMetadataProvider targetMetadataProvider) {
     this.targetMetadataProvider = targetMetadataProvider;
   }
@@ -208,31 +215,40 @@ public void handleExistenceFilter(ExistenceFilterWatchChange watchChange) {
         if (currentSize != expectedCount) {
 
           // Apply bloom filter to identify and mark removed documents.
-          boolean bloomFilterApplied = this.applyBloomFilter(watchChange, currentSize);
+          BloomFilterApplicationStatus status = this.applyBloomFilter(watchChange, currentSize);
 
-          if (!bloomFilterApplied) {
+          if (status != BloomFilterApplicationStatus.SUCCESS) {
             // If bloom filter application fails, we reset the mapping and
             // trigger re-run of the query.
             resetTarget(targetId);
-            pendingTargetResets.add(targetId);
+
+            QueryPurpose purpose =
+                status == BloomFilterApplicationStatus.FALSE_POSITIVE
+                    ? QueryPurpose.EXISTENCE_FILTER_MISMATCH_BLOOM
+                    : QueryPurpose.EXISTENCE_FILTER_MISMATCH;
+
+            pendingTargetResets.put(targetId, purpose);
           }
 
           WatchChangeAggregatorTestingHooks.notifyOnExistenceFilterMismatch(
               WatchChangeAggregatorTestingHooks.ExistenceFilterMismatchInfo.from(
-                  bloomFilterApplied, currentSize, watchChange.getExistenceFilter()));
+                  status == BloomFilterApplicationStatus.SUCCESS,
+                  currentSize,
+                  watchChange.getExistenceFilter()));
         }
       }
     }
   }
 
-  /** Returns whether a bloom filter removed the deleted documents successfully. */
-  private boolean applyBloomFilter(ExistenceFilterWatchChange watchChange, int currentCount) {
+  /** Apply bloom filter to remove the deleted documents, and return the application status. */
+  private BloomFilterApplicationStatus applyBloomFilter(
+      ExistenceFilterWatchChange watchChange, int currentCount) {
     int expectedCount = watchChange.getExistenceFilter().getCount();
     com.google.firestore.v1.BloomFilter unchangedNames =
         watchChange.getExistenceFilter().getUnchangedNames();
 
     if (unchangedNames == null || !unchangedNames.hasBits()) {
-      return false;
+      return BloomFilterApplicationStatus.SKIPPED;
     }
 
     byte[] bitmap = unchangedNames.getBits().getBitmap().toByteArray();
@@ -248,12 +264,20 @@ private boolean applyBloomFilter(ExistenceFilterWatchChange watchChange, int cur
           "Applying bloom filter failed: ("
               + e.getMessage()
               + "); ignoring the bloom filter and falling back to full re-query.");
-      return false;
+      return BloomFilterApplicationStatus.SKIPPED;
+    }
+
+    if (bloomFilter.getBitCount() == 0) {
+      return BloomFilterApplicationStatus.SKIPPED;
     }
 
     int removedDocumentCount = this.filterRemovedDocuments(bloomFilter, watchChange.getTargetId());
 
-    return expectedCount == (currentCount - removedDocumentCount);
+    if (expectedCount != (currentCount - removedDocumentCount)) {
+      return BloomFilterApplicationStatus.FALSE_POSITIVE;
+    }
+
+    return BloomFilterApplicationStatus.SUCCESS;
   }
 
   /**
@@ -345,14 +369,14 @@ public RemoteEvent createRemoteEvent(SnapshotVersion snapshotVersion) {
         new RemoteEvent(
             snapshotVersion,
             Collections.unmodifiableMap(targetChanges),
-            Collections.unmodifiableSet(pendingTargetResets),
+            Collections.unmodifiableMap(pendingTargetResets),
             Collections.unmodifiableMap(pendingDocumentUpdates),
             Collections.unmodifiableSet(resolvedLimboDocuments));
 
     // Re-initialize the current state to ensure that we do not modify the generated RemoteEvent.
     pendingDocumentUpdates = new HashMap<>();
     pendingDocumentTargetMapping = new HashMap<>();
-    pendingTargetResets = new HashSet<>();
+    pendingTargetResets = new HashMap<>();
 
     return remoteEvent;
   }

firebase-firestore/src/test/resources/json/bundle_spec_test.json

@@ -1166,7 +1166,7 @@
                 }
               ],
               "resumeToken": "",
-              "targetPurpose": 2
+              "targetPurpose": 3
             },
             "2": {
               "queries": [