Skip to content

Add new goog-listen-tag for bloom filter #4777

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Mar 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -569,7 +570,7 @@ private void raiseWatchSnapshot(SnapshotVersion snapshotVersion) {
targetData.getTarget(),
targetId,
targetData.getSequenceNumber(),
QueryPurpose.EXISTENCE_FILTER_MISMATCH);
/*purpose=*/ entry.getValue());
this.sendWatchRequest(requestTargetData);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -208,27 +215,34 @@ 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);
}
}
}
}
}

/** 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();
Expand All @@ -244,12 +258,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;
}

/**
Expand Down Expand Up @@ -341,14 +363,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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1166,7 +1166,7 @@
}
],
"resumeToken": "",
"targetPurpose": 2
"targetPurpose": 3
},
"2": {
"queries": [
Expand Down
Loading