Skip to content

Commit 7e205a7

Browse files
Merging the latest merge into the previous merge (#1077)
* Implement global resume token (#1052) * Add a spec test that shows correct global resume token handling * Minimum implementation to handle global resume tokens * Remove unused QueryView.resumeToken * Avoid persisting the resume token unless required * Persist the resume token on unlisten * Add a type parameter to Persistence (#1047) * Cherry pick sequence number starting point * Working on typed transactions * Start plumbing in sequence number * Back out sequence number changes * [AUTOMATED]: Prettier Code Styling * Fix tests * [AUTOMATED]: Prettier Code Styling * Fix lint * [AUTOMATED]: Prettier Code Styling * Uncomment line * MemoryPersistenceTransaction -> MemoryTransaction * [AUTOMATED]: Prettier Code Styling * Review updates * Style * Lint and style * Review feedback * [AUTOMATED]: Prettier Code Styling * Revert some unintentional import churn * Line 44 should definitely be empty * Checkpoint before adding helper function for stores * Use a helper for casting PersistenceTransaction to IndexedDbTransaction * [AUTOMATED]: Prettier Code Styling * Remove errant generic type * Lint * Fix typo * Port optimizations to LocalDocumentsView from iOS (#1055) * add a method to find batches affecting a set of keys (port of [1479](firebase/firebase-ios-sdk#1479)); * use the newly-added method to avoid rereading batches when getting documents in `LocalDocumentsView` (port of [1505](firebase/firebase-ios-sdk#1505)); * avoid rereading batches when searching for documents in a collection (port of [1533](firebase/firebase-ios-sdk#1533)). Speedup was measured by running tests in browser and checking time spent writing 10 batches of 500 mutations each, and then querying the resulting 5K docs collection from cache in offline mode. For this case, the writing speedup is about 3x, and querying speedup is about 6x (see PR for more details). * Add a CHANGELOG entry for #1052 (#1071) * Add a CHANGELOG entry for #1052 * Add notes for #1055 * Rename idleTimer and fix comments. (#1068) * Merge (#1073)
1 parent 38e46a4 commit 7e205a7

25 files changed

+692
-426
lines changed

packages/firestore/CHANGELOG.md

+5
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
11
# Unreleased
2+
- [changed] Improved how Firestore handles idle queries to reduce the cost of
3+
re-listening within 30 minutes.
4+
- [changed] Improved offline performance with many outstanding writes.
5+
6+
# 0.6.0
27
- [fixed] Fixed an issue where queries returned fewer results than they should,
38
caused by documents that were cached as deleted when they should not have
49
been (firebase/firebase-ios-sdk#1548). Because some cache data is cleared,

packages/firestore/src/core/sync_engine.ts

+1-13
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ import {
5050
MutationBatchState,
5151
OnlineState,
5252
OnlineStateSource,
53-
ProtoByteString,
5453
TargetId
5554
} from './types';
5655
import {
@@ -88,12 +87,6 @@ class QueryView {
8887
* stream to identify this query.
8988
*/
9089
public targetId: TargetId,
91-
/**
92-
* An identifier from the datastore backend that indicates the last state
93-
* of the results that was received. This can be used to indicate where
94-
* to continue receiving new doc changes for the query.
95-
*/
96-
public resumeToken: ProtoByteString,
9790
/**
9891
* The view is responsible for computing the final merged truth of what
9992
* docs are in the query. It gets notified of local and remote changes,
@@ -274,12 +267,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
274267
'applyChanges for new view should always return a snapshot'
275268
);
276269

277-
const data = new QueryView(
278-
query,
279-
queryData.targetId,
280-
queryData.resumeToken,
281-
view
282-
);
270+
const data = new QueryView(query, queryData.targetId, view);
283271
this.queryViewsByQuery.set(query, data);
284272
this.queryViewsByTarget[queryData.targetId] = data;
285273
return viewChange.snapshot!;

packages/firestore/src/local/indexeddb_mutation_queue.ts

+86-36
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import { Timestamp } from '../api/timestamp';
1818
import { User } from '../auth/user';
1919
import { Query } from '../core/query';
2020
import { BatchId, ProtoByteString } from '../core/types';
21+
import { DocumentKeySet } from '../model/collections';
2122
import { DocumentKey } from '../model/document_key';
2223
import { Mutation } from '../model/mutation';
2324
import { BATCHID_UNKNOWN, MutationBatch } from '../model/mutation_batch';
@@ -40,8 +41,8 @@ import { LocalSerializer } from './local_serializer';
4041
import { MutationQueue } from './mutation_queue';
4142
import { PersistenceTransaction } from './persistence';
4243
import { PersistencePromise } from './persistence_promise';
43-
import { SimpleDb, SimpleDbStore } from './simple_db';
44-
import { DocumentKeySet } from '../model/collections';
44+
import { SimpleDbStore } from './simple_db';
45+
import { IndexedDbPersistence } from './indexeddb_persistence';
4546

4647
/** A mutation queue for a specific user, backed by IndexedDB. */
4748
export class IndexedDbMutationQueue implements MutationQueue {
@@ -342,6 +343,50 @@ export class IndexedDbMutationQueue implements MutationQueue {
342343
.next(() => results);
343344
}
344345

346+
getAllMutationBatchesAffectingDocumentKeys(
347+
transaction: PersistenceTransaction,
348+
documentKeys: DocumentKeySet
349+
): PersistencePromise<MutationBatch[]> {
350+
let uniqueBatchIDs = new SortedSet<BatchId>(primitiveComparator);
351+
352+
const promises: Array<PersistencePromise<void>> = [];
353+
documentKeys.forEach(documentKey => {
354+
const indexStart = DbDocumentMutation.prefixForPath(
355+
this.userId,
356+
documentKey.path
357+
);
358+
const range = IDBKeyRange.lowerBound(indexStart);
359+
360+
const promise = documentMutationsStore(transaction).iterate(
361+
{ range },
362+
(indexKey, _, control) => {
363+
const [userID, encodedPath, batchID] = indexKey;
364+
365+
// Only consider rows matching exactly the specific key of
366+
// interest. Note that because we order by path first, and we
367+
// order terminators before path separators, we'll encounter all
368+
// the index rows for documentKey contiguously. In particular, all
369+
// the rows for documentKey will occur before any rows for
370+
// documents nested in a subcollection beneath documentKey so we
371+
// can stop as soon as we hit any such row.
372+
const path = EncodedResourcePath.decode(encodedPath);
373+
if (userID !== this.userId || !documentKey.path.isEqual(path)) {
374+
control.done();
375+
return;
376+
}
377+
378+
uniqueBatchIDs = uniqueBatchIDs.add(batchID);
379+
}
380+
);
381+
382+
promises.push(promise);
383+
});
384+
385+
return PersistencePromise.waitFor(promises).next(() =>
386+
this.lookupMutationBatches(transaction, uniqueBatchIDs)
387+
);
388+
}
389+
345390
getAllMutationBatchesAffectingQuery(
346391
transaction: PersistenceTransaction,
347392
query: Query
@@ -393,34 +438,39 @@ export class IndexedDbMutationQueue implements MutationQueue {
393438
}
394439
uniqueBatchIDs = uniqueBatchIDs.add(batchID);
395440
})
396-
.next(() => {
397-
const results: MutationBatch[] = [];
398-
const promises: Array<PersistencePromise<void>> = [];
399-
// TODO(rockwood): Implement this using iterate.
400-
uniqueBatchIDs.forEach(batchId => {
401-
promises.push(
402-
mutationsStore(transaction)
403-
.get(batchId)
404-
.next(mutation => {
405-
if (!mutation) {
406-
fail(
407-
'Dangling document-mutation reference found, ' +
408-
'which points to ' +
409-
batchId
410-
);
411-
}
412-
assert(
413-
mutation.userId === this.userId,
414-
`Unexpected user '${
415-
mutation.userId
416-
}' for mutation batch ${batchId}`
417-
);
418-
results.push(this.serializer.fromDbMutationBatch(mutation!));
419-
})
420-
);
421-
});
422-
return PersistencePromise.waitFor(promises).next(() => results);
423-
});
441+
.next(() => this.lookupMutationBatches(transaction, uniqueBatchIDs));
442+
}
443+
444+
private lookupMutationBatches(
445+
transaction: PersistenceTransaction,
446+
batchIDs: SortedSet<BatchId>
447+
): PersistencePromise<MutationBatch[]> {
448+
const results: MutationBatch[] = [];
449+
const promises: Array<PersistencePromise<void>> = [];
450+
// TODO(rockwood): Implement this using iterate.
451+
batchIDs.forEach(batchId => {
452+
promises.push(
453+
mutationsStore(transaction)
454+
.get(batchId)
455+
.next(mutation => {
456+
if (mutation === null) {
457+
fail(
458+
'Dangling document-mutation reference found, ' +
459+
'which points to ' +
460+
batchId
461+
);
462+
}
463+
assert(
464+
mutation.userId === this.userId,
465+
`Unexpected user '${
466+
mutation.userId
467+
}' for mutation batch ${batchId}`
468+
);
469+
results.push(this.serializer.fromDbMutationBatch(mutation!));
470+
})
471+
);
472+
});
473+
return PersistencePromise.waitFor(promises).next(() => results);
424474
}
425475

426476
removeMutationBatches(
@@ -567,7 +617,7 @@ function convertStreamToken(token: ProtoByteString): string {
567617
function mutationsStore(
568618
txn: PersistenceTransaction
569619
): SimpleDbStore<DbMutationBatchKey, DbMutationBatch> {
570-
return SimpleDb.getStore<DbMutationBatchKey, DbMutationBatch>(
620+
return IndexedDbPersistence.getStore<DbMutationBatchKey, DbMutationBatch>(
571621
txn,
572622
DbMutationBatch.store
573623
);
@@ -579,10 +629,10 @@ function mutationsStore(
579629
function documentMutationsStore(
580630
txn: PersistenceTransaction
581631
): SimpleDbStore<DbDocumentMutationKey, DbDocumentMutation> {
582-
return SimpleDb.getStore<DbDocumentMutationKey, DbDocumentMutation>(
583-
txn,
584-
DbDocumentMutation.store
585-
);
632+
return IndexedDbPersistence.getStore<
633+
DbDocumentMutationKey,
634+
DbDocumentMutation
635+
>(txn, DbDocumentMutation.store);
586636
}
587637

588638
/**
@@ -591,7 +641,7 @@ function documentMutationsStore(
591641
function mutationQueuesStore(
592642
txn: PersistenceTransaction
593643
): SimpleDbStore<DbMutationQueueKey, DbMutationQueue> {
594-
return SimpleDb.getStore<DbMutationQueueKey, DbMutationQueue>(
644+
return IndexedDbPersistence.getStore<DbMutationQueueKey, DbMutationQueue>(
595645
txn,
596646
DbMutationQueue.store
597647
);

packages/firestore/src/local/indexeddb_persistence.ts

+57-32
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
import { User } from '../auth/user';
1818
import { DatabaseInfo } from '../core/database_info';
1919
import { JsonProtoSerializer } from '../remote/serializer';
20-
import { assert } from '../util/assert';
20+
import { assert, fail } from '../util/assert';
2121
import { Code, FirestoreError } from '../util/error';
2222
import * as log from '../util/log';
2323

@@ -83,6 +83,12 @@ const UNSUPPORTED_PLATFORM_ERROR_MSG =
8383
// firestore_zombie_<persistence_prefix>_<instance_key>
8484
const ZOMBIED_CLIENTS_KEY_PREFIX = 'firestore_zombie';
8585

86+
export class IndexedDbTransaction extends PersistenceTransaction {
87+
constructor(readonly simpleDbTransaction: SimpleDbTransaction) {
88+
super();
89+
}
90+
}
91+
8692
/**
8793
* An IndexedDB-backed instance of Persistence. Data is stored persistently
8894
* across sessions.
@@ -115,6 +121,17 @@ const ZOMBIED_CLIENTS_KEY_PREFIX = 'firestore_zombie';
115121
* TODO(multitab): Update this comment with multi-tab changes.
116122
*/
117123
export class IndexedDbPersistence implements Persistence {
124+
static getStore<Key extends IDBValidKey, Value>(
125+
txn: PersistenceTransaction,
126+
store: string
127+
): SimpleDbStore<Key, Value> {
128+
if (txn instanceof IndexedDbTransaction) {
129+
return SimpleDb.getStore<Key, Value>(txn.simpleDbTransaction, store);
130+
} else {
131+
fail('IndexedDbPersistence must use instances of IndexedDbTransaction');
132+
}
133+
}
134+
118135
/**
119136
* The name of the main (and currently only) IndexedDB database. this name is
120137
* appended to the prefix provided to the IndexedDbPersistence constructor.
@@ -470,7 +487,7 @@ export class IndexedDbPersistence implements Persistence {
470487
action: string,
471488
requirePrimaryLease: boolean,
472489
transactionOperation: (
473-
transaction: PersistenceTransaction
490+
transaction: IndexedDbTransaction
474491
) => PersistencePromise<T>
475492
): Promise<T> {
476493
// TODO(multitab): Consider removing `requirePrimaryLease` and exposing
@@ -483,39 +500,47 @@ export class IndexedDbPersistence implements Persistence {
483500

484501
// Do all transactions as readwrite against all object stores, since we
485502
// are the only reader/writer.
486-
return this.simpleDb.runTransaction('readwrite', ALL_STORES, txn => {
487-
if (requirePrimaryLease) {
488-
// While we merely verify that we have (or can acquire) the lease
489-
// immediately, we wait to extend the primary lease until after
490-
// executing transactionOperation(). This ensures that even if the
491-
// transactionOperation takes a long time, we'll use a recent
492-
// leaseTimestampMs in the extended (or newly acquired) lease.
493-
return this.canActAsPrimary(txn)
494-
.next(canActAsPrimary => {
495-
if (!canActAsPrimary) {
496-
// TODO(multitab): Handle this gracefully and transition back to
497-
// secondary state.
498-
log.error(
499-
`Failed to obtain primary lease for action '${action}'.`
503+
return this.simpleDb.runTransaction(
504+
'readwrite',
505+
ALL_STORES,
506+
simpleDbTxn => {
507+
if (requirePrimaryLease) {
508+
// While we merely verify that we have (or can acquire) the lease
509+
// immediately, we wait to extend the primary lease until after
510+
// executing transactionOperation(). This ensures that even if the
511+
// transactionOperation takes a long time, we'll use a recent
512+
// leaseTimestampMs in the extended (or newly acquired) lease.
513+
return this.canActAsPrimary(simpleDbTxn)
514+
.next(canActAsPrimary => {
515+
if (!canActAsPrimary) {
516+
// TODO(multitab): Handle this gracefully and transition back to
517+
// secondary state.
518+
log.error(
519+
`Failed to obtain primary lease for action '${action}'.`
520+
);
521+
this.isPrimary = false;
522+
this.queue.enqueue(() => this.primaryStateListener(false));
523+
throw new FirestoreError(
524+
Code.FAILED_PRECONDITION,
525+
PRIMARY_LEASE_LOST_ERROR_MSG
526+
);
527+
}
528+
return transactionOperation(
529+
new IndexedDbTransaction(simpleDbTxn)
500530
);
501-
this.isPrimary = false;
502-
this.queue.enqueue(() => this.primaryStateListener(false));
503-
throw new FirestoreError(
504-
Code.FAILED_PRECONDITION,
505-
PRIMARY_LEASE_LOST_ERROR_MSG
531+
})
532+
.next(result => {
533+
return this.acquireOrExtendPrimaryLease(simpleDbTxn).next(
534+
() => result
506535
);
507-
}
508-
return transactionOperation(txn);
509-
})
510-
.next(result => {
511-
return this.acquireOrExtendPrimaryLease(txn).next(() => result);
512-
});
513-
} else {
514-
return this.verifyAllowTabSynchronization(txn).next(() =>
515-
transactionOperation(txn)
516-
);
536+
});
537+
} else {
538+
return this.verifyAllowTabSynchronization(simpleDbTxn).next(() =>
539+
transactionOperation(new IndexedDbTransaction(simpleDbTxn))
540+
);
541+
}
517542
}
518-
});
543+
);
519544
}
520545

521546
/**

0 commit comments

Comments
 (0)