Skip to content

Commit a5e7062

Browse files
authored
Add option to allow SDK create cache indexes automatically to improve query execution locally (#4987)
* Add counter * address feedback 1 * add copyright * fix concurrency bug * implement autoClientIndexing * Add tests and fix bugs for BuildTargetIndex * hide getter from public API * move the flag from IndexManager to QueryEngine * Address feedback * move auto index flag to runtime * Support old way to enable persistent for PersistentCacheManager * Polish Tests * Add hide and copyright * clean up unused function * Rename PersistentCacheManager to PersistentCacheIndexManager * Remove unused QueryContext * Address feedbacks other than adding tests and comments * Change the api to match the update * Add tests * Increase tests coverage * Add comments * add configurable min documents to create indexes * Address Denver's feedback * Address feedback * address more feedbacks * use the number getting from 100 ~ 1000 documents experiment * Address feedbacks * improve debug log
1 parent 24d1983 commit a5e7062

19 files changed

+897
-16
lines changed

firebase-firestore/src/androidTest/java/com/google/firebase/firestore/IndexingTest.java

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,13 @@
1414

1515
package com.google.firebase.firestore;
1616

17+
import static com.google.firebase.firestore.testutil.IntegrationTestUtil.testCollectionWithDocs;
1718
import static com.google.firebase.firestore.testutil.IntegrationTestUtil.testFirestore;
19+
import static com.google.firebase.firestore.testutil.IntegrationTestUtil.waitFor;
20+
import static com.google.firebase.firestore.testutil.TestUtil.assertDoesNotThrow;
21+
import static com.google.firebase.firestore.testutil.TestUtil.expectError;
22+
import static com.google.firebase.firestore.testutil.TestUtil.map;
23+
import static org.junit.Assert.assertEquals;
1824

1925
import androidx.test.ext.junit.runners.AndroidJUnit4;
2026
import com.google.android.gms.tasks.Task;
@@ -101,4 +107,73 @@ public void testBadIndexDoesNotCrashClient() {
101107
+ " \"fieldOverrides\": []\n"
102108
+ "}"));
103109
}
110+
111+
@Test
112+
public void testAutoIndexCreationSetSuccessfully() {
113+
// Use persistent disk cache (default)
114+
FirebaseFirestore db = testFirestore();
115+
FirebaseFirestoreSettings settings =
116+
new FirebaseFirestoreSettings.Builder(db.getFirestoreSettings())
117+
.setLocalCacheSettings(PersistentCacheSettings.newBuilder().build())
118+
.build();
119+
db.setFirestoreSettings(settings);
120+
121+
CollectionReference collection =
122+
testCollectionWithDocs(
123+
map(
124+
"a", map("match", true),
125+
"b", map("match", false),
126+
"c", map("match", false)));
127+
QuerySnapshot results = waitFor(collection.whereEqualTo("match", true).get());
128+
assertEquals(1, results.size());
129+
130+
assertDoesNotThrow(() -> db.getPersistentCacheIndexManager().enableIndexAutoCreation());
131+
132+
results = waitFor(collection.whereEqualTo("match", true).get());
133+
assertEquals(1, results.size());
134+
135+
assertDoesNotThrow(() -> db.getPersistentCacheIndexManager().disableIndexAutoCreation());
136+
137+
results = waitFor(collection.whereEqualTo("match", true).get());
138+
assertEquals(1, results.size());
139+
}
140+
141+
@Test
142+
public void testAutoIndexCreationSetSuccessfullyUsingDefault() {
143+
// Use persistent disk cache (default)
144+
FirebaseFirestore db = testFirestore();
145+
146+
CollectionReference collection =
147+
testCollectionWithDocs(
148+
map(
149+
"a", map("match", true),
150+
"b", map("match", false),
151+
"c", map("match", false)));
152+
QuerySnapshot results = waitFor(collection.whereEqualTo("match", true).get());
153+
assertEquals(1, results.size());
154+
155+
assertDoesNotThrow(() -> db.getPersistentCacheIndexManager().enableIndexAutoCreation());
156+
157+
results = waitFor(collection.whereEqualTo("match", true).get());
158+
assertEquals(1, results.size());
159+
160+
assertDoesNotThrow(() -> db.getPersistentCacheIndexManager().disableIndexAutoCreation());
161+
162+
results = waitFor(collection.whereEqualTo("match", true).get());
163+
assertEquals(1, results.size());
164+
}
165+
166+
@Test
167+
public void testAutoIndexCreationAfterFailsTermination() {
168+
FirebaseFirestore db = testFirestore();
169+
waitFor(db.terminate());
170+
171+
expectError(
172+
() -> db.getPersistentCacheIndexManager().enableIndexAutoCreation(),
173+
"The client has already been terminated");
174+
175+
expectError(
176+
() -> db.getPersistentCacheIndexManager().disableIndexAutoCreation(),
177+
"The client has already been terminated");
178+
}
104179
}

firebase-firestore/src/main/java/com/google/firebase/firestore/FirebaseFirestore.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import androidx.annotation.Keep;
2424
import androidx.annotation.NonNull;
2525
import androidx.annotation.Nullable;
26+
import androidx.annotation.RestrictTo;
2627
import androidx.annotation.VisibleForTesting;
2728
import com.google.android.gms.tasks.Task;
2829
import com.google.android.gms.tasks.TaskCompletionSource;
@@ -104,6 +105,8 @@ public interface InstanceRegistry {
104105
private volatile FirestoreClient client;
105106
private final GrpcMetadataProvider metadataProvider;
106107

108+
@Nullable private PersistentCacheIndexManager persistentCacheIndexManager;
109+
107110
@NonNull
108111
private static FirebaseApp getDefaultFirebaseApp() {
109112
FirebaseApp app = FirebaseApp.getInstance();
@@ -403,6 +406,26 @@ public Task<Void> setIndexConfiguration(@NonNull String json) {
403406
return client.configureFieldIndexes(parsedIndexes);
404407
}
405408

409+
/**
410+
* Returns the PersistentCache Index Manager used by this {@code FirebaseFirestore} object.
411+
*
412+
* @return The {@code PersistentCacheIndexManager} instance or null if local persistent storage is
413+
* not in use.
414+
*/
415+
// TODO(csi): Remove the `hide` and scope annotations.
416+
/** @hide */
417+
@RestrictTo(RestrictTo.Scope.LIBRARY)
418+
@Nullable
419+
public synchronized PersistentCacheIndexManager getPersistentCacheIndexManager() {
420+
ensureClientConfigured();
421+
if (persistentCacheIndexManager == null
422+
&& (settings.isPersistenceEnabled()
423+
|| settings.getCacheSettings() instanceof PersistentCacheSettings)) {
424+
persistentCacheIndexManager = new PersistentCacheIndexManager(client);
425+
}
426+
return persistentCacheIndexManager;
427+
}
428+
406429
/**
407430
* Gets a {@code CollectionReference} instance that refers to the collection at the specified path
408431
* within the database.
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
// Copyright 2023 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package com.google.firebase.firestore;
16+
17+
import androidx.annotation.NonNull;
18+
import androidx.annotation.RestrictTo;
19+
import com.google.firebase.firestore.core.FirestoreClient;
20+
21+
/**
22+
* A {@code PersistentCacheIndexManager} which you can config persistent cache indexes used for
23+
* local query execution.
24+
*
25+
* <p>To use, call {@link FirebaseFirestore#getPersistentCacheIndexManager()} to get an instance.
26+
*/
27+
// TODO(csi): Remove the `hide` and scope annotations.
28+
/** @hide */
29+
@RestrictTo(RestrictTo.Scope.LIBRARY)
30+
public final class PersistentCacheIndexManager {
31+
@NonNull private FirestoreClient client;
32+
33+
PersistentCacheIndexManager(FirestoreClient client) {
34+
this.client = client;
35+
}
36+
37+
/**
38+
* Enables SDK to create persistent cache indexes automatically for local query execution when SDK
39+
* believes cache indexes can help improves performance.
40+
*
41+
* <p>This feature is disabled by default.
42+
*/
43+
public void enableIndexAutoCreation() {
44+
client.setIndexAutoCreationEnabled(true);
45+
}
46+
47+
/**
48+
* Stops creating persistent cache indexes automatically for local query execution. The indexes
49+
* which have been created by calling {@link #enableIndexAutoCreation()} still take effect.
50+
*/
51+
public void disableIndexAutoCreation() {
52+
client.setIndexAutoCreationEnabled(false);
53+
}
54+
}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,11 @@ public Task<Void> configureFieldIndexes(List<FieldIndex> fieldIndices) {
353353
return asyncQueue.enqueue(() -> localStore.configureFieldIndexes(fieldIndices));
354354
}
355355

356+
public void setIndexAutoCreationEnabled(boolean enabled) {
357+
verifyNotTerminated();
358+
asyncQueue.enqueueAndForget(() -> localStore.setIndexAutoCreationEnabled(enabled));
359+
}
360+
356361
public void removeSnapshotsInSyncListener(EventListener<Void> listener) {
357362
// Checks for shutdown but does not raise error, allowing remove after shutdown to be a no-op.
358363
if (isTerminated()) {

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,9 @@ enum IndexType {
7878
/** Removes the given field index and deletes all index values. */
7979
void deleteFieldIndex(FieldIndex index);
8080

81+
/** Creates a full matched field index which serves the given target. */
82+
void createTargetIndexes(Target target);
83+
8184
/**
8285
* Returns a list of field indexes that correspond to the specified collection group.
8386
*

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

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -258,19 +258,32 @@ void recalculateAndSaveOverlays(Set<DocumentKey> documentKeys) {
258258
*
259259
* @param query The query to match documents against.
260260
* @param offset Read time and key to start scanning by (exclusive).
261+
* @param context A optional tracker to keep a record of important details during database local
262+
* query execution.
261263
*/
262264
ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingQuery(
263-
Query query, IndexOffset offset) {
265+
Query query, IndexOffset offset, @Nullable QueryContext context) {
264266
ResourcePath path = query.getPath();
265267
if (query.isDocumentQuery()) {
266268
return getDocumentsMatchingDocumentQuery(path);
267269
} else if (query.isCollectionGroupQuery()) {
268-
return getDocumentsMatchingCollectionGroupQuery(query, offset);
270+
return getDocumentsMatchingCollectionGroupQuery(query, offset, context);
269271
} else {
270-
return getDocumentsMatchingCollectionQuery(query, offset);
272+
return getDocumentsMatchingCollectionQuery(query, offset, context);
271273
}
272274
}
273275

276+
/**
277+
* Performs a query against the local view of all documents.
278+
*
279+
* @param query The query to match documents against.
280+
* @param offset Read time and key to start scanning by (exclusive).
281+
*/
282+
ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingQuery(
283+
Query query, IndexOffset offset) {
284+
return getDocumentsMatchingQuery(query, offset, /*context*/ null);
285+
}
286+
274287
/** Performs a simple document lookup for the given path. */
275288
private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingDocumentQuery(
276289
ResourcePath path) {
@@ -284,7 +297,7 @@ private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingDocumentQu
284297
}
285298

286299
private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingCollectionGroupQuery(
287-
Query query, IndexOffset offset) {
300+
Query query, IndexOffset offset, @Nullable QueryContext context) {
288301
hardAssert(
289302
query.getPath().isEmpty(),
290303
"Currently we only support collection group queries at the root.");
@@ -297,7 +310,7 @@ private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingCollection
297310
for (ResourcePath parent : parents) {
298311
Query collectionQuery = query.asCollectionQueryAtPath(parent.append(collectionId));
299312
ImmutableSortedMap<DocumentKey, Document> collectionResults =
300-
getDocumentsMatchingCollectionQuery(collectionQuery, offset);
313+
getDocumentsMatchingCollectionQuery(collectionQuery, offset, context);
301314
for (Map.Entry<DocumentKey, Document> docEntry : collectionResults) {
302315
results = results.insert(docEntry.getKey(), docEntry.getValue());
303316
}
@@ -362,11 +375,11 @@ private void populateOverlays(Map<DocumentKey, Overlay> overlays, Set<DocumentKe
362375
}
363376

364377
private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingCollectionQuery(
365-
Query query, IndexOffset offset) {
378+
Query query, IndexOffset offset, @Nullable QueryContext context) {
366379
Map<DocumentKey, Overlay> overlays =
367380
documentOverlayCache.getOverlays(query.getPath(), offset.getLargestBatchId());
368381
Map<DocumentKey, MutableDocument> remoteDocuments =
369-
remoteDocumentCache.getDocumentsMatchingQuery(query, offset, overlays.keySet());
382+
remoteDocumentCache.getDocumentsMatchingQuery(query, offset, overlays.keySet(), context);
370383

371384
// As documents might match the query because of their overlay we need to include documents
372385
// for all overlays in the initial document set.

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -802,6 +802,10 @@ public void configureFieldIndexes(List<FieldIndex> newFieldIndexes) {
802802
});
803803
}
804804

805+
public void setIndexAutoCreationEnabled(boolean enabled) {
806+
queryEngine.setIndexAutoCreationEnabled(enabled);
807+
}
808+
805809
/** Mutable state for the transaction in allocateQuery. */
806810
private static class AllocateQueryHolder {
807811
TargetData cached;

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,9 @@ public void deleteFieldIndex(FieldIndex index) {
6060
// Field indices are not supported with memory persistence.
6161
}
6262

63+
@Override
64+
public void createTargetIndexes(Target target) {}
65+
6366
@Override
6467
@Nullable
6568
public List<DocumentKey> getDocumentsMatchingTarget(Target target) {

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import static com.google.firebase.firestore.util.Assert.hardAssert;
1919

2020
import androidx.annotation.NonNull;
21+
import androidx.annotation.Nullable;
2122
import com.google.firebase.database.collection.ImmutableSortedMap;
2223
import com.google.firebase.firestore.core.Query;
2324
import com.google.firebase.firestore.model.Document;
@@ -97,7 +98,10 @@ public Map<DocumentKey, MutableDocument> getAll(
9798

9899
@Override
99100
public Map<DocumentKey, MutableDocument> getDocumentsMatchingQuery(
100-
Query query, IndexOffset offset, @Nonnull Set<DocumentKey> mutatedKeys) {
101+
Query query,
102+
IndexOffset offset,
103+
@Nonnull Set<DocumentKey> mutatedKeys,
104+
@Nullable QueryContext context) {
101105
Map<DocumentKey, MutableDocument> result = new HashMap<>();
102106

103107
// Documents are ordered by key, so we can use a prefix scan to narrow down the documents
@@ -135,6 +139,12 @@ public Map<DocumentKey, MutableDocument> getDocumentsMatchingQuery(
135139
return result;
136140
}
137141

142+
@Override
143+
public Map<DocumentKey, MutableDocument> getDocumentsMatchingQuery(
144+
Query query, IndexOffset offset, @Nonnull Set<DocumentKey> mutatedKeys) {
145+
return getDocumentsMatchingQuery(query, offset, mutatedKeys, /*context*/ null);
146+
}
147+
138148
Iterable<Document> getDocuments() {
139149
return new DocumentIterable();
140150
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// Copyright 2023 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package com.google.firebase.firestore.local;
16+
17+
/** A tracker to keep a record of important details during database local query execution. */
18+
public class QueryContext {
19+
/** Counts the number of documents passed through during local query execution. */
20+
private int documentReadCount = 0;
21+
22+
public int getDocumentReadCount() {
23+
return documentReadCount;
24+
}
25+
26+
public void incrementDocumentReadCount() {
27+
documentReadCount++;
28+
}
29+
}

0 commit comments

Comments
 (0)