Skip to content

Only fetch 50 documents per backfill #3193

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 22 commits into from
Dec 8, 2021
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
5 changes: 3 additions & 2 deletions firebase-firestore/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@ by opting into a release at
[go/firebase-android-release](http:go/firebase-android-release) (Googlers only).

# 24.1.0
- [changed] Performance optimization for offline usage.
- [changed] Improved performance for queries with collections that contain
- [changed] Improved performance for databases that contain many document
updates that have not yet been synced with the backend.
- [changed] Improved performance for queries against collections that contain
subcollections.

# 24.0.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,15 @@
import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting;
import com.google.firebase.database.collection.ImmutableSortedMap;
import com.google.firebase.firestore.core.Query;
import com.google.firebase.firestore.model.Document;
import com.google.firebase.firestore.model.DocumentKey;
import com.google.firebase.firestore.model.FieldIndex;
import com.google.firebase.firestore.model.FieldIndex.IndexOffset;
import com.google.firebase.firestore.model.ResourcePath;
import com.google.firebase.firestore.util.AsyncQueue;
import com.google.firebase.firestore.util.Logger;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Iterator;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.TimeUnit;
Expand Down Expand Up @@ -138,22 +134,18 @@ private int writeIndexEntries(LocalDocumentsView localDocumentsView) {
/** Writes entries for the fetched field indexes. */
private int writeEntriesForCollectionGroup(
LocalDocumentsView localDocumentsView, String collectionGroup, int entriesRemainingUnderCap) {
Query query = new Query(ResourcePath.EMPTY, collectionGroup);
// TODO(indexing): Support mutation batch Ids when sorting and writing indexes.

// Use the earliest offset of all field indexes to query the local cache.
IndexOffset existingOffset = getExistingOffset(indexManager.getFieldIndexes(collectionGroup));

// TODO(indexing): Use limit queries to only fetch the required number of entries.
// TODO(indexing): Support mutation batch Ids when sorting and writing indexes.
ImmutableSortedMap<DocumentKey, Document> documents =
localDocumentsView.getDocumentsMatchingQuery(query, existingOffset);

List<Document> oldestDocuments = getOldestDocuments(documents, entriesRemainingUnderCap);
indexManager.updateIndexEntries(oldestDocuments);
localDocumentsView.getDocuments(collectionGroup, existingOffset, entriesRemainingUnderCap);
indexManager.updateIndexEntries(documents);

IndexOffset newOffset = getNewOffset(oldestDocuments, existingOffset);
IndexOffset newOffset = getNewOffset(documents, existingOffset);
indexManager.updateCollectionGroup(collectionGroup, newOffset);
return oldestDocuments.size();

return documents.size();
}

/** Returns the lowest offset for the provided index group. */
Expand All @@ -168,37 +160,22 @@ private IndexOffset getExistingOffset(Collection<FieldIndex> fieldIndexes) {
return lowestOffset == null ? IndexOffset.NONE : lowestOffset;
}

/**
* Returns the offset for the index based on the newly indexed documents.
*
* @param documents a list of documents sorted by read time and key (ascending)
* @param currentOffset the current offset of the index group
*/
private IndexOffset getNewOffset(List<Document> documents, IndexOffset currentOffset) {
IndexOffset latestOffset =
documents.isEmpty()
? IndexOffset.create(remoteDocumentCache.getLatestReadTime())
: IndexOffset.create(
documents.get(documents.size() - 1).getReadTime(),
documents.get(documents.size() - 1).getKey());
// Make sure the index does not go back in time
latestOffset = latestOffset.compareTo(currentOffset) > 0 ? latestOffset : currentOffset;
return latestOffset;
}

/** Returns up to {@code count} documents sorted by read time and key. */
private List<Document> getOldestDocuments(
ImmutableSortedMap<DocumentKey, Document> documents, int count) {
List<Document> oldestDocuments = new ArrayList<>();
for (Map.Entry<DocumentKey, Document> entry : documents) {
oldestDocuments.add(entry.getValue());
/** Returns the offset for the index based on the newly indexed documents. */
private IndexOffset getNewOffset(
ImmutableSortedMap<DocumentKey, Document> documents, IndexOffset currentOffset) {
if (documents.isEmpty()) {
return IndexOffset.create(remoteDocumentCache.getLatestReadTime());
} else {
IndexOffset latestOffset = currentOffset;
Iterator<Map.Entry<DocumentKey, Document>> it = documents.iterator();
while (it.hasNext()) {
IndexOffset newOffset = IndexOffset.fromDocument(it.next().getValue());
if (newOffset.compareTo(latestOffset) > 0) {
latestOffset = newOffset;
}
}
return latestOffset;
}
Collections.sort(
oldestDocuments,
(l, r) ->
IndexOffset.create(l.getReadTime(), l.getKey())
.compareTo(IndexOffset.create(r.getReadTime(), r.getKey())));
return oldestDocuments.subList(0, Math.min(count, oldestDocuments.size()));
}

@VisibleForTesting
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package com.google.firebase.firestore.local;

import androidx.annotation.Nullable;
import com.google.firebase.database.collection.ImmutableSortedMap;
import com.google.firebase.firestore.core.Target;
import com.google.firebase.firestore.model.Document;
import com.google.firebase.firestore.model.DocumentKey;
Expand Down Expand Up @@ -97,5 +98,5 @@ public interface IndexManager {
void updateCollectionGroup(String collectionGroup, FieldIndex.IndexOffset offset);

/** Updates the index entries for the provided documents. */
void updateIndexEntries(Collection<Document> documents);
void updateIndexEntries(ImmutableSortedMap<DocumentKey, Document> documents);
}
Original file line number Diff line number Diff line change
Expand Up @@ -238,10 +238,13 @@ void recalculateAndSaveOverlays(Set<DocumentKey> documentKeys) {
recalculateAndSaveOverlays(docs);
}

// TODO: The Querying implementation here should move 100% to the query engines.
// Instead, we should just provide a getCollectionDocuments() method here that return all the
// documents in a given collection so that query engine can do that and then filter in
// memory.
/** Gets the local view of the next {@code count} documents based on their read time. */
ImmutableSortedMap<DocumentKey, Document> getDocuments(
String collectionGroup, IndexOffset offset, int count) {
Map<DocumentKey, MutableDocument> docs =
remoteDocumentCache.getAll(collectionGroup, offset, count);
return getLocalViewOfDocuments(docs, new HashSet<>());
}

/**
* Performs a query against the local view of all documents.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import static com.google.firebase.firestore.util.Assert.hardAssert;

import androidx.annotation.Nullable;
import com.google.firebase.database.collection.ImmutableSortedMap;
import com.google.firebase.firestore.core.Target;
import com.google.firebase.firestore.model.Document;
import com.google.firebase.firestore.model.DocumentKey;
Expand Down Expand Up @@ -98,7 +99,7 @@ public Collection<FieldIndex> getFieldIndexes() {
}

@Override
public void updateIndexEntries(Collection<Document> documents) {
public void updateIndexEntries(ImmutableSortedMap<DocumentKey, Document> documents) {
// Field indices are not supported with memory persistence.
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,13 @@ public Map<DocumentKey, MutableDocument> getAll(Iterable<DocumentKey> keys) {
return result;
}

@Override
public Map<DocumentKey, MutableDocument> getAll(
String collectionGroup, IndexOffset offset, int limit) {
// This method should only be called from the IndexBackfiller if SQLite is enabled.
throw new UnsupportedOperationException("getAll(String, IndexOffset, int) is not supported.");
}

@Override
public ImmutableSortedMap<DocumentKey, MutableDocument> getAllDocumentsMatchingQuery(
Query query, IndexOffset offset) {
Expand Down Expand Up @@ -108,7 +115,7 @@ public ImmutableSortedMap<DocumentKey, MutableDocument> getAllDocumentsMatchingQ
continue;
}

if (IndexOffset.create(doc.getReadTime(), doc.getKey()).compareTo(offset) <= 0) {
if (IndexOffset.fromDocument(doc).compareTo(offset) <= 0) {
continue;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import com.google.firebase.database.collection.ImmutableSortedMap;
import com.google.firebase.firestore.core.Query;
import com.google.firebase.firestore.model.DocumentKey;
import com.google.firebase.firestore.model.FieldIndex;
import com.google.firebase.firestore.model.FieldIndex.IndexOffset;
import com.google.firebase.firestore.model.MutableDocument;
import com.google.firebase.firestore.model.SnapshotVersion;
import java.util.Map;
Expand Down Expand Up @@ -65,6 +65,17 @@ interface RemoteDocumentCache {
*/
Map<DocumentKey, MutableDocument> getAll(Iterable<DocumentKey> documentKeys);

/**
* Looks up the next {@code limit} documents for a collection group based on the provided offset.
* The ordering is based on the document's read time and key.
*
* @param collectionGroup The collection group to scan.
* @param offset The offset to start the scan at.
* @param limit The maximum number of results to return.
* @return A map with next set of documents.
*/
Map<DocumentKey, MutableDocument> getAll(String collectionGroup, IndexOffset offset, int limit);

/**
* Executes a query against the cached Document entries
*
Expand All @@ -78,7 +89,7 @@ interface RemoteDocumentCache {
* @return The set of matching documents.
*/
ImmutableSortedMap<DocumentKey, MutableDocument> getAllDocumentsMatchingQuery(
Query query, FieldIndex.IndexOffset offset);
Query query, IndexOffset offset);

/** Returns the latest read time of any document in the cache. */
SnapshotVersion getLatestReadTime();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

import androidx.annotation.Nullable;
import com.google.firebase.Timestamp;
import com.google.firebase.database.collection.ImmutableSortedMap;
import com.google.firebase.firestore.auth.User;
import com.google.firebase.firestore.core.Bound;
import com.google.firebase.firestore.core.FieldFilter;
Expand Down Expand Up @@ -222,16 +223,15 @@ public void deleteFieldIndex(FieldIndex index) {
}

@Override
public void updateIndexEntries(Collection<Document> documents) {
public void updateIndexEntries(ImmutableSortedMap<DocumentKey, Document> documents) {
hardAssert(started, "IndexManager not started");
for (Document document : documents) {
Collection<FieldIndex> fieldIndexes = getFieldIndexes(document.getKey().getCollectionGroup());
for (Map.Entry<DocumentKey, Document> entry : documents) {
Collection<FieldIndex> fieldIndexes = getFieldIndexes(entry.getKey().getCollectionGroup());
for (FieldIndex fieldIndex : fieldIndexes) {
SortedSet<IndexEntry> existingEntries =
getExistingIndexEntries(document.getKey(), fieldIndex);
SortedSet<IndexEntry> newEntries = computeIndexEntries(document, fieldIndex);
SortedSet<IndexEntry> existingEntries = getExistingIndexEntries(entry.getKey(), fieldIndex);
SortedSet<IndexEntry> newEntries = computeIndexEntries(entry.getValue(), fieldIndex);
if (!existingEntries.equals(newEntries)) {
updateEntries(document, existingEntries, newEntries);
updateEntries(entry.getValue(), existingEntries, newEntries);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@
* helper routines that make dealing with SQLite much more pleasant.
*/
public final class SQLitePersistence extends Persistence {
/**
* The maximum number of bind args for a single statement. Set to 900 instead of 999 for safety.
*/
public static final int MAX_ARGS = 900;

/**
* Creates the database name that is used to identify the database to be used with a Firestore
Expand Down
Loading