Skip to content

Firestore: Optimize local cache sync when resuming a query that had docs deleted #4982

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 49 commits into from
May 8, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
b58e756
Update protos to include bloom filter (#4564)
milaGGL Jan 17, 2023
1b0b3ed
Merge branch 'master' into mila/BloomFilter
milaGGL Jan 17, 2023
fd907e5
Merge branch 'master' into mila/BloomFilter
milaGGL Jan 18, 2023
b306135
Implement BloomFilter class (#4524)
milaGGL Jan 20, 2023
82280ca
Merge branch 'master' into mila/BloomFilter
milaGGL Jan 26, 2023
2139f5e
Add expected count to target (#4574)
milaGGL Jan 27, 2023
078c414
Apply bloom filter on existence filter mismatch (#4601)
milaGGL Feb 9, 2023
3862731
Merge branch 'master' into mila/BloomFilter
milaGGL Feb 9, 2023
9e7023d
Merge branch 'master' into mila/BloomFilter
milaGGL Feb 17, 2023
7ea3c34
Update TargetData.java
milaGGL Feb 27, 2023
8696c17
Add integration test for bloom filter (#4696)
milaGGL Feb 28, 2023
a9a2b42
Merge branch 'master' into mila/BloomFilter
milaGGL Mar 9, 2023
8db1af7
Merge branch 'master' into mila/BloomFilter
milaGGL Mar 9, 2023
9f66dea
fromat
milaGGL Mar 9, 2023
70326ee
Update MockDatastore.java
milaGGL Mar 10, 2023
3d406f2
Merge remote-tracking branch 'origin/master' into HEAD
dconeybe Mar 11, 2023
9e889fe
Merge branch 'master' into mila/BloomFilter
milaGGL Mar 13, 2023
97dafee
Update WatchChangeAggregator.java
milaGGL Mar 14, 2023
90d57dd
Merge branch 'master' into mila/BloomFilter
milaGGL Mar 14, 2023
1bbf61b
update queryTest to be consistent with master
milaGGL Mar 14, 2023
06479a0
Add new goog-listen-tag for bloom filter (#4777)
milaGGL Mar 16, 2023
59f2859
Update the integration test to verify that bloom filter averted full …
dconeybe Mar 16, 2023
443289c
Merge remote-tracking branch 'origin/master' into mila/BloomFilter
dconeybe Mar 18, 2023
9716326
Merge remote-tracking branch 'origin/master' into mila/BloomFilter
dconeybe Mar 20, 2023
36a1643
Merge commit '687e079d401db1a75d2559038b879726a476ca3e' into mila/Blo…
dconeybe Mar 24, 2023
ecda2ba
Merge remote-tracking branch 'origin/master' into mila/BloomFilter
dconeybe Mar 24, 2023
79cc918
Merge branch 'master' into mila/BloomFilter
milaGGL Mar 27, 2023
d63be2b
Improve bloom filter application test coverage (#4828)
milaGGL Mar 30, 2023
8c6677f
Merge remote-tracking branch 'origin/master' into mila/BloomFilter
dconeybe Apr 7, 2023
158c084
QueryTest.java: Remove check for `getTargetBackend() != NIGHTLY` sinc…
dconeybe Apr 11, 2023
c76b67c
Port spec test changes from https://github.com/firebase/firebase-js-s…
dconeybe Apr 24, 2023
0f870e9
Port spec test changes from https://github.com/firebase/firebase-js-s…
dconeybe Apr 24, 2023
7202f9d
Merge branch 'master' into dconeybe/SpecTestPortJsSdkPr7021
dconeybe Apr 24, 2023
c8562d4
Merge remote-tracking branch 'remotes/origin/dconeybe/SpecTestPortJsS…
dconeybe Apr 24, 2023
e1ac631
regenerate spec tests json from js sdk
dconeybe Apr 25, 2023
766f99a
SpecTestCase.java: update to handle `targetPurpose` as a string, not …
dconeybe Apr 25, 2023
48a082b
Merge remote-tracking branch 'remotes/origin/dconeybe/SpecTestPortJsS…
dconeybe Apr 25, 2023
fa399de
Merge remote-tracking branch 'origin/master' into SpecTestPortJsSdkPr…
dconeybe Apr 25, 2023
0e9601d
Merge remote-tracking branch 'remotes/origin/dconeybe/SpecTestPortJsS…
dconeybe Apr 25, 2023
793bfc4
Merge remote-tracking branch 'remotes/origin/dconeybe/SpecTestPortJsS…
dconeybe Apr 25, 2023
105000a
Merge remote-tracking branch 'origin/master' into mila/BloomFilter
dconeybe Apr 25, 2023
98f62ad
Merge remote-tracking branch 'origin/master' into mila/BloomFilter
dconeybe Apr 27, 2023
6b22865
Merge remote-tracking branch 'origin/master' into mila/BloomFilter
dconeybe May 2, 2023
0a67ba1
Merge remote-tracking branch 'origin/master' into mila/BloomFilter
dconeybe May 5, 2023
9c12d19
Merge branch 'master' into mila/BloomFilter
milaGGL May 5, 2023
2573baa
Update CHANGELOG.md
milaGGL May 5, 2023
cc0428a
update CHANGELOG + format
milaGGL May 8, 2023
d0849f4
Random improvements. (#4986)
dconeybe May 8, 2023
d9a59e2
Update CHANGELOG.md
milaGGL May 8, 2023
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
Prev Previous commit
Next Next commit
Add new goog-listen-tag for bloom filter (#4777)
  • Loading branch information
milaGGL authored Mar 16, 2023
commit 06479a02ecf8635667ffe5bc9ef4e921264d6d52
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