From a492edba9ddbd11dd42d1c0458f3fb1e23942dac Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Mon, 12 Feb 2018 15:51:04 -0800 Subject: [PATCH 01/18] Adding Master Election --- .../firestore/src/core/firestore_client.ts | 7 +- packages/firestore/src/core/sync_engine.ts | 13 +- .../src/local/indexeddb_persistence.ts | 367 ++++++++++++------ .../firestore/src/local/indexeddb_schema.ts | 9 +- packages/firestore/src/local/local_store.ts | 159 ++++---- .../firestore/src/local/memory_persistence.ts | 23 +- packages/firestore/src/local/persistence.ts | 41 ++ packages/firestore/src/platform/platform.ts | 6 + .../src/platform_browser/browser_platform.ts | 4 + .../src/platform_node/node_platform.ts | 4 + .../test/unit/local/mutation_queue.test.ts | 4 +- .../unit/local/persistence_test_helpers.ts | 4 +- .../test/unit/local/test_garbage_collector.ts | 2 +- .../test/unit/local/test_mutation_queue.ts | 53 ++- .../test/unit/local/test_query_cache.ts | 18 +- .../unit/local/test_remote_document_cache.ts | 16 +- .../test_remote_document_change_buffer.ts | 4 +- .../test/unit/specs/describe_spec.ts | 32 +- .../test/unit/specs/persistence_spec.test.ts | 48 ++- .../firestore/test/unit/specs/spec_builder.ts | 294 +++++++++++++- .../test/unit/specs/spec_test_runner.ts | 182 +++++++-- 21 files changed, 1027 insertions(+), 263 deletions(-) diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index 32d0cf2ac34..b87cd485539 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -241,7 +241,11 @@ export class FirestoreClient { const serializer = new JsonProtoSerializer(this.databaseInfo.databaseId, { useProto3Json: true }); - this.persistence = new IndexedDbPersistence(storagePrefix, serializer); + this.persistence = new IndexedDbPersistence( + storagePrefix, + this.platform, + serializer + ); return this.persistence.start(); } @@ -299,6 +303,7 @@ export class FirestoreClient { // Setup wiring between sync engine and remote store this.remoteStore.syncEngine = this.syncEngine; + this.persistence.setPrimaryStateListener(this.syncEngine); this.eventMgr = new EventManager(this.syncEngine); diff --git a/packages/firestore/src/core/sync_engine.ts b/packages/firestore/src/core/sync_engine.ts index 7170d1ed5d6..3f6cb626137 100644 --- a/packages/firestore/src/core/sync_engine.ts +++ b/packages/firestore/src/core/sync_engine.ts @@ -51,6 +51,7 @@ import { ViewDocumentChanges } from './view'; import { ViewSnapshot } from './view_snapshot'; +import { PrimaryStateListener } from '../local/persistence'; const LOG_TAG = 'SyncEngine'; @@ -102,7 +103,7 @@ class QueryView { * The SyncEngine’s methods should only ever be called by methods running in the * global async queue. */ -export class SyncEngine implements RemoteSyncer { +export class SyncEngine implements RemoteSyncer, PrimaryStateListener { private viewHandler: ViewHandler | null = null; private errorHandler: ErrorHandler | null = null; @@ -121,6 +122,7 @@ export class SyncEngine implements RemoteSyncer { [uidKey: string]: SortedMap>; }; private targetIdGenerator = TargetIdGenerator.forSyncEngine(); + private isPrimary = false; constructor( private localStore: LocalStore, @@ -128,6 +130,10 @@ export class SyncEngine implements RemoteSyncer { private currentUser: User ) {} + get isPrimaryClient() { + return this.isPrimary; + } + /** Subscribes view and error handler. Can be called only once. */ subscribe(viewHandler: ViewHandler, errorHandler: ErrorHandler): void { assert( @@ -616,4 +622,9 @@ export class SyncEngine implements RemoteSyncer { return this.remoteStore.handleUserChange(user); }); } + + applyPrimaryState(primaryClient: boolean): void { + // TODO(multitab): Apply the primary state internally on the AsyncQueue. + this.isPrimary = primaryClient; + } } diff --git a/packages/firestore/src/local/indexeddb_persistence.ts b/packages/firestore/src/local/indexeddb_persistence.ts index b09548e5e2b..d09c59532a9 100644 --- a/packages/firestore/src/local/indexeddb_persistence.ts +++ b/packages/firestore/src/local/indexeddb_persistence.ts @@ -28,32 +28,42 @@ import { IndexedDbRemoteDocumentCache } from './indexeddb_remote_document_cache' import { ALL_STORES, createOrUpgradeDb, + DbClientMetadataKey, + DbClientMetadata, DbOwner, DbOwnerKey, SCHEMA_VERSION } from './indexeddb_schema'; import { LocalSerializer } from './local_serializer'; import { MutationQueue } from './mutation_queue'; -import { Persistence } from './persistence'; +import { + Persistence, + PersistenceTransaction, + PrimaryStateListener +} from './persistence'; import { PersistencePromise } from './persistence_promise'; import { QueryCache } from './query_cache'; import { RemoteDocumentCache } from './remote_document_cache'; -import { SimpleDb, SimpleDbTransaction } from './simple_db'; +import { SimpleDb, SimpleDbStore, SimpleDbTransaction } from './simple_db'; +import { Platform } from '../platform/platform'; const LOG_TAG = 'IndexedDbPersistence'; -/** If the owner lease is older than 5 seconds, try to take ownership. */ -const OWNER_LEASE_MAX_AGE_MS = 5000; -/** Refresh the owner lease every 4 seconds while owner. */ -const OWNER_LEASE_REFRESH_INTERVAL_MS = 4000; - -/** LocalStorage location to indicate a zombied ownerId (see class comment). */ -const ZOMBIE_OWNER_LOCALSTORAGE_SUFFIX = 'zombiedOwnerId'; -/** Error when the owner lease cannot be acquired or is lost. */ -const EXISTING_OWNER_ERROR_MSG = - 'There is another tab open with offline' + - ' persistence enabled. Only one such tab is allowed at a time. The' + - ' other tab must be closed or persistence must be disabled.'; +/** + * Oldest acceptable age in milliseconds for client states read from IndexedDB. + * Client state and primary leases that are older than 5 seconds are ignored. + */ +const CLIENT_STATE_MAX_AGE_MS = 5000; + +/** Refresh interval for the client state. Currently set to four seconds. */ +const CLIENT_STATE_REFRESH_INTERVAL_MS = 4000; + +/** LocalStorage location to indicate a zombied primary key (see class comment). */ +const ZOMBIE_PRIMARY_LOCALSTORAGE_SUFFIX = 'zombiedOwnerId'; +/** Error when the primary lease is required but not available. */ +const PRIMARY_LEASE_LOST_ERROR_MSG = + 'The current tab is not the required state to perform the current ' + + 'operation. It might be necessary to refresh the browser tab.'; const UNSUPPORTED_PLATFORM_ERROR_MSG = 'This platform is either missing' + ' IndexedDB or is known to have an incomplete implementation. Offline' + @@ -89,17 +99,25 @@ const UNSUPPORTED_PLATFORM_ERROR_MSG = * owner lease immediately regardless of the current lease timestamp. */ export class IndexedDbPersistence implements Persistence { + private static EMPTY_PRIMARY_STATE_LISTENER: PrimaryStateListener = { + applyPrimaryState: () => {} + }; + /** * The name of the main (and currently only) IndexedDB database. this name is * appended to the prefix provided to the IndexedDbPersistence constructor. */ static MAIN_DATABASE = 'main'; + private readonly document: Document | null; + private readonly window: Window | null; + private simpleDb: SimpleDb; private started: boolean; + private isPrimary = false; private dbName: string; private localStoragePrefix: string; - private ownerId: string = this.generateOwnerId(); + private clientKey: string = this.generateClientKey(); /** * Set to an Error object if we encounter an unrecoverable error. All further @@ -108,16 +126,29 @@ export class IndexedDbPersistence implements Persistence { private persistenceError: Error | null; /** The setInterval() handle tied to refreshing the owner lease. */ // tslint:disable-next-line:no-any setTimeout() type differs on browser / node - private ownerLeaseRefreshHandle: any; + private clientStateRefreshHandler: any; /** Our window.unload handler, if registered. */ private windowUnloadHandler: (() => void) | null; + private inForeground = false; private serializer: LocalSerializer; - constructor(prefix: string, serializer: JsonProtoSerializer) { + /** Our 'visibilitychange` listener if registered. */ + private documentVisibilityHandler: ((e: Event) => void) | null; + + /** Callback for primary state notifications. */ + private primaryStateListener = IndexedDbPersistence.EMPTY_PRIMARY_STATE_LISTENER; + + constructor( + prefix: string, + platform: Platform, + serializer: JsonProtoSerializer + ) { this.dbName = prefix + IndexedDbPersistence.MAIN_DATABASE; this.serializer = new LocalSerializer(serializer); this.localStoragePrefix = prefix; + this.document = platform.document; + this.window = platform.window; } start(): Promise { @@ -129,6 +160,21 @@ export class IndexedDbPersistence implements Persistence { return Promise.reject(this.persistenceError); } + if (this.document) { + this.documentVisibilityHandler = () => { + const inForeground = document.visibilityState === 'visible'; + if (inForeground !== this.inForeground) { + this.inForeground = inForeground; + this.refreshClientState(); + } + }; + + this.document.addEventListener( + 'visibilitychange', + this.documentVisibilityHandler + ); + } + assert(!this.started, 'IndexedDbPersistence double-started!'); this.started = true; @@ -136,19 +182,58 @@ export class IndexedDbPersistence implements Persistence { .then(db => { this.simpleDb = db; }) - .then(() => this.tryAcquireOwnerLease()) .then(() => { - this.scheduleOwnerLeaseRefreshes(); + this.refreshClientState(); + this.scheduleClientStateRefresh(); this.attachWindowUnloadHook(); }); } + setPrimaryStateListener(primaryStateListener: PrimaryStateListener) { + this.primaryStateListener = primaryStateListener; + primaryStateListener.applyPrimaryState(this.isPrimary); + } + + tryBecomePrimary(): Promise { + return this.refreshClientState(); + } + + /** + * Updates the state of the client in IndexedDb. This operation may acquire + * the primary lease. + * + * @return Whether the client holds the primary lease. + */ + private refreshClientState(): Promise { + return this.simpleDb + .runTransaction( + 'readwrite', + [DbOwner.store, DbClientMetadata.store], + txn => { + const metadataStore = clientMetadataStore(txn); + metadataStore.put( + new DbClientMetadata(this.clientKey, Date.now(), this.inForeground) + ); + return this.tryAcquirePrimaryLease(txn); + } + ) + .then(isPrimary => { + if (isPrimary !== this.isPrimary) { + this.isPrimary = isPrimary; + this.primaryStateListener.applyPrimaryState(this.isPrimary); + } + }) + .then(() => this.isPrimary); + } + shutdown(): Promise { - assert(this.started, 'IndexedDbPersistence shutdown without start!'); + if (!this.started) { + return Promise.resolve(); + } this.started = false; this.detachWindowUnloadHook(); - this.stopOwnerLeaseRefreshes(); - return this.releaseOwnerLease().then(() => { + this.stopClientStateRefreshes(); + return this.releasePrimaryLease().then(() => { this.simpleDb.close(); }); } @@ -167,7 +252,10 @@ export class IndexedDbPersistence implements Persistence { runTransaction( action: string, - operation: (transaction: SimpleDbTransaction) => PersistencePromise + requireOwnerLease: boolean, + transactionOperation: ( + transaction: PersistenceTransaction + ) => PersistencePromise ): Promise { if (this.persistenceError) { return Promise.reject(this.persistenceError); @@ -178,8 +266,13 @@ export class IndexedDbPersistence implements Persistence { // Do all transactions as readwrite against all object stores, since we // are the only reader/writer. return this.simpleDb.runTransaction('readwrite', ALL_STORES, txn => { - // Verify that we still have the owner lease as part of every transaction. - return this.ensureOwnerLease(txn).next(() => operation(txn)); + if (requireOwnerLease) { + return this.ensurePrimaryLease(txn).next(() => + transactionOperation(txn) + ); + } else { + return transactionOperation(txn); + } }); } @@ -208,50 +301,81 @@ export class IndexedDbPersistence implements Persistence { } /** - * Acquires the owner lease if there's no valid owner. Else returns a rejected - * promise. + * Tries to acquire and extend the primary lease. Returns whether the client + * holds the primary lease. */ - private tryAcquireOwnerLease(): Promise { - // NOTE: Don't use this.runTransaction, since it requires us to already - // have the lease. - return this.simpleDb.runTransaction('readwrite', [DbOwner.store], txn => { - const store = txn.store(DbOwner.store); - return store.get('owner').next(dbOwner => { - if (!this.validOwner(dbOwner)) { - const newDbOwner = new DbOwner(this.ownerId, Date.now()); - log.debug( - LOG_TAG, - 'No valid owner. Acquiring owner lease. Current owner:', - dbOwner, - 'New owner:', - newDbOwner - ); - return store.put('owner', newDbOwner); - } else { - log.debug( - LOG_TAG, - 'Valid owner already. Failing. Current owner:', - dbOwner - ); - this.persistenceError = new FirestoreError( - Code.FAILED_PRECONDITION, - EXISTING_OWNER_ERROR_MSG - ); - return PersistencePromise.reject(this.persistenceError); - } - }); + private tryAcquirePrimaryLease( + txn: SimpleDbTransaction + ): PersistencePromise { + const ownerStore = txn.store(DbOwner.store); + return ownerStore.get('owner').next(dbOwner => { + const currentOwner = this.extractCurrentOwner(dbOwner); + if (currentOwner === null) { + return this.canBecomePrimary(txn).next(canBecomePrimary => { + if (canBecomePrimary) { + const newDbOwner = new DbOwner(this.clientKey, Date.now()); + log.debug( + LOG_TAG, + 'No valid primary. Acquiring primary lease. Current primary client:', + dbOwner, + 'New owner:', + newDbOwner + ); + return ownerStore.put('owner', newDbOwner).next(() => true); + } else { + return PersistencePromise.resolve(false); + } + }); + } else if (currentOwner === this.clientKey) { + // Refresh the primary lease + const newDbOwner = new DbOwner(this.clientKey, Date.now()); + return ownerStore.put('owner', newDbOwner).next(() => true); + } else { + return PersistencePromise.resolve(false); + } }); } - /** Checks the owner lease and deletes it if we are the current owner. */ - private releaseOwnerLease(): Promise { - // NOTE: Don't use this.runTransaction, since it requires us to already - // have the lease. + /** + * Checks if this client is eligible for a primary lease based on its + * visibility state and the visibility state of all active instances. A + * client can obtain the primary lease if it is either in the foreground + * or if it and all other clients are in the background. + */ + private canBecomePrimary( + txn: SimpleDbTransaction + ): PersistencePromise { + if (this.inForeground) { + return PersistencePromise.resolve(true); + } + + let canBecomePrimary = true; + + return clientMetadataStore(txn) + .iterate((key, value, control) => { + if (this.clientKey !== value.clientKey) { + if ( + this.isRecentlyUpdated(value.updateTimeMs) && + value.inForeground + ) { + canBecomePrimary = false; + control.done(); + } + } + }) + .next(() => canBecomePrimary); + } + + /** Checks the primary lease and removes it if we are the current primary. */ + private releasePrimaryLease(): Promise { + // NOTE: Don't use this.runTransaction, since it acquires a lock on all + // object stores. return this.simpleDb.runTransaction('readwrite', [DbOwner.store], txn => { const store = txn.store(DbOwner.store); return store.get('owner').next(dbOwner => { - if (dbOwner !== null && dbOwner.ownerId === this.ownerId) { + if (dbOwner !== null && dbOwner.ownerId === this.clientKey) { log.debug(LOG_TAG, 'Releasing owner lease.'); + this.primaryStateListener.applyPrimaryState(false); return store.delete('owner'); } else { return PersistencePromise.resolve(); @@ -265,13 +389,14 @@ export class IndexedDbPersistence implements Persistence { * current owner. This should be included in every transaction to guard * against losing the owner lease. */ - private ensureOwnerLease(txn: SimpleDbTransaction): PersistencePromise { - const store = txn.store(DbOwner.store); - return store.get('owner').next(dbOwner => { - if (dbOwner === null || dbOwner.ownerId !== this.ownerId) { + private ensurePrimaryLease( + txn: SimpleDbTransaction + ): PersistencePromise { + return this.tryAcquirePrimaryLease(txn).next(owner => { + if (!owner) { this.persistenceError = new FirestoreError( Code.FAILED_PRECONDITION, - EXISTING_OWNER_ERROR_MSG + PRIMARY_LEASE_LOST_ERROR_MSG ); return PersistencePromise.reject(this.persistenceError); } else { @@ -280,6 +405,21 @@ export class IndexedDbPersistence implements Persistence { }); } + /** Verifies that that `updateTimeMs` is within CLIENT_STATE_MAX_AGE_MS. */ + private isRecentlyUpdated(updateTimeMs: number): boolean { + const now = Date.now(); + const minAcceptable = now - CLIENT_STATE_MAX_AGE_MS; + const maxAcceptable = now; + if (updateTimeMs < minAcceptable) { + return false; + } else if (updateTimeMs > maxAcceptable) { + log.error('Detected an update time that is in the future.'); + return false; + } + + return true; + } + /** * Returns true if the provided owner exists, has a recent timestamp, and * isn't zombied. @@ -287,24 +427,15 @@ export class IndexedDbPersistence implements Persistence { * NOTE: To determine if the owner is zombied, this method reads from * LocalStorage which could be mildly expensive. */ - private validOwner(dbOwner: DbOwner | null): boolean { - const now = Date.now(); - const minAcceptable = now - OWNER_LEASE_MAX_AGE_MS; - const maxAcceptable = now; + private extractCurrentOwner(dbOwner: DbOwner): string | null { if (dbOwner === null) { - return false; // no owner. - } else if (dbOwner.leaseTimestampMs < minAcceptable) { - return false; // owner lease has expired. - } else if (dbOwner.leaseTimestampMs > maxAcceptable) { - log.error( - 'Persistence owner-lease is in the future. Discarding.', - dbOwner - ); - return false; + return null; // no owner. + } else if (!this.isRecentlyUpdated(dbOwner.leaseTimestampMs)) { + return null; // primary lease has expired. } else if (dbOwner.ownerId === this.getZombiedOwnerId()) { - return false; // owner's tab closed. + return null; // owner's tab closed. } else { - return true; + return dbOwner.ownerId; } } @@ -312,31 +443,22 @@ export class IndexedDbPersistence implements Persistence { * Schedules a recurring timer to update the owner lease timestamp to prevent * other tabs from taking the lease. */ - private scheduleOwnerLeaseRefreshes(): void { + private scheduleClientStateRefresh(): void { // NOTE: This doesn't need to be scheduled on the async queue and doing so // would increase the chances of us not refreshing on time if the queue is // backed up for some reason. - this.ownerLeaseRefreshHandle = setInterval(() => { - const txResult = this.runTransaction('Refresh owner timestamp', txn => { - // NOTE: We don't need to validate the current owner contents, since - // runTransaction does that automatically. - const store = txn.store(DbOwner.store); - return store.put('owner', new DbOwner(this.ownerId, Date.now())); - }); - txResult.catch(reason => { - // Probably means we lost the lease. Report the error and stop trying to - // refresh the lease. - log.error(reason); - this.stopOwnerLeaseRefreshes(); - }); - }, OWNER_LEASE_REFRESH_INTERVAL_MS); + // TODO(mutlitab): To transition stale clients faster, we should synchronize + // the execution of this callback with primary client's lease expiry. + this.clientStateRefreshHandler = setInterval(() => { + this.refreshClientState(); + }, CLIENT_STATE_REFRESH_INTERVAL_MS); } - private stopOwnerLeaseRefreshes(): void { - if (this.ownerLeaseRefreshHandle) { - clearInterval(this.ownerLeaseRefreshHandle); - this.ownerLeaseRefreshHandle = null; + private stopClientStateRefreshes(): void { + if (this.clientStateRefreshHandler) { + clearInterval(this.clientStateRefreshHandler); + this.clientStateRefreshHandler = null; } } @@ -351,21 +473,29 @@ export class IndexedDbPersistence implements Persistence { */ private attachWindowUnloadHook(): void { this.windowUnloadHandler = () => { - // Record that we're zombied. - this.setZombiedOwnerId(this.ownerId); + if (this.isPrimary) { + this.setZombiePrimaryKey(this.clientKey); + } // Attempt graceful shutdown (including releasing our owner lease), but // there's no guarantee it will complete. this.shutdown(); }; - window.addEventListener('unload', this.windowUnloadHandler); + this.window.addEventListener('unload', this.windowUnloadHandler); } private detachWindowUnloadHook(): void { if (this.windowUnloadHandler) { - window.removeEventListener('unload', this.windowUnloadHandler); + this.window.removeEventListener('unload', this.windowUnloadHandler); this.windowUnloadHandler = null; } + if (this.documentVisibilityHandler) { + this.document.removeEventListener( + 'visibilitychange', + this.documentVisibilityHandler + ); + this.documentVisibilityHandler = null; + } } /** @@ -376,7 +506,7 @@ export class IndexedDbPersistence implements Persistence { private getZombiedOwnerId(): string | null { try { const zombiedOwnerId = window.localStorage.getItem( - this.zombiedOwnerLocalStorageKey() + this.zombiedPrimaryLocalStorageKey() ); log.debug(LOG_TAG, 'Zombied ownerID from LocalStorage:', zombiedOwnerId); return zombiedOwnerId; @@ -388,16 +518,16 @@ export class IndexedDbPersistence implements Persistence { } /** - * Records a zombied owner (an owner that had its tab closed) in LocalStorage - * or, if passed null, deletes any recorded zombied owner. + * Records a zombied primary key (a client that had its tab closed) in + * LocalStorage or, if passed null, deletes any recorded zombied owner. */ - private setZombiedOwnerId(zombieOwnerId: string | null) { + private setZombiePrimaryKey(zombieOwnerId: string | null) { try { if (zombieOwnerId === null) { - window.localStorage.removeItem(this.zombiedOwnerLocalStorageKey()); + window.localStorage.removeItem(this.zombiedPrimaryLocalStorageKey()); } else { window.localStorage.setItem( - this.zombiedOwnerLocalStorageKey(), + this.zombiedPrimaryLocalStorageKey(), zombieOwnerId ); } @@ -407,12 +537,23 @@ export class IndexedDbPersistence implements Persistence { } } - private zombiedOwnerLocalStorageKey(): string { - return this.localStoragePrefix + ZOMBIE_OWNER_LOCALSTORAGE_SUFFIX; + private zombiedPrimaryLocalStorageKey(): string { + return this.localStoragePrefix + ZOMBIE_PRIMARY_LOCALSTORAGE_SUFFIX; } - private generateOwnerId(): string { + private generateClientKey(): string { // For convenience, just use an AutoId. return AutoId.newId(); } } + +/** + * Helper to get a typed SimpleDbStore for the client metadata object store. + */ +function clientMetadataStore( + txn: SimpleDbTransaction +): SimpleDbStore { + return txn.store( + DbClientMetadata.store + ); +} diff --git a/packages/firestore/src/local/indexeddb_schema.ts b/packages/firestore/src/local/indexeddb_schema.ts index d3689ff94f1..5982338a4df 100644 --- a/packages/firestore/src/local/indexeddb_schema.ts +++ b/packages/firestore/src/local/indexeddb_schema.ts @@ -545,21 +545,24 @@ export class DbClientMetadata { static store = 'clientMetadata'; /** Keys are automatically assigned via the clientKey properties. */ - static keyPath = ['clientKey']; + static key = ['clientKey']; constructor( /** The auto-generated client key assigned at client startup. */ public clientKey: string, /** The last time this state was updated. */ - public updateTimeMs: DbTimestamp, + public updateTimeMs: number, /** Whether this client is running in a foreground tab. */ public inForeground: boolean ) {} } +/** Object keys in the 'clientMetadata' store are clientKey strings. */ +export type DbClientMetadataKey = string; + function createClientMetadataStore(db: IDBDatabase): void { db.createObjectStore(DbClientMetadata.store, { - keyPath: DbClientMetadata.keyPath as KeyPath + keyPath: DbClientMetadata.key as KeyPath }); } diff --git a/packages/firestore/src/local/local_store.ts b/packages/firestore/src/local/local_store.ts index d8f1b84e16c..8385bf8be63 100644 --- a/packages/firestore/src/local/local_store.ts +++ b/packages/firestore/src/local/local_store.ts @@ -182,7 +182,8 @@ export class LocalStore { /** Performs any initial startup actions required by the local store. */ start(): Promise { - return this.persistence.runTransaction('Start LocalStore', txn => { + // TODO(multitab): Ensure that we in fact don't need the primary lease. + return this.persistence.runTransaction('Start LocalStore', false, txn => { return this.startMutationQueue(txn).next(() => this.startQueryCache(txn)); }); } @@ -194,7 +195,7 @@ export class LocalStore { * returns any resulting document changes. */ handleUserChange(user: User): Promise { - return this.persistence.runTransaction('Handle user change', txn => { + return this.persistence.runTransaction('Handle user change', true, txn => { // Swap out the mutation queue, grabbing the pending mutation batches // before and after. let oldBatches: MutationBatch[]; @@ -282,23 +283,27 @@ export class LocalStore { /* Accept locally generated Mutations and commit them to storage. */ localWrite(mutations: Mutation[]): Promise { - return this.persistence.runTransaction('Locally write mutations', txn => { - let batch: MutationBatch; - const localWriteTime = Timestamp.now(); - return this.mutationQueue - .addMutationBatch(txn, localWriteTime, mutations) - .next(promisedBatch => { - batch = promisedBatch; - // TODO(koss): This is doing an N^2 update by replaying ALL the - // mutations on each document (instead of just the ones added) in - // this batch. - const keys = batch.keys(); - return this.localDocuments.getDocuments(txn, keys); - }) - .next((changedDocuments: MaybeDocumentMap) => { - return { batchId: batch.batchId, changes: changedDocuments }; - }); - }); + return this.persistence.runTransaction( + 'Locally write mutations', + true, + txn => { + let batch: MutationBatch; + const localWriteTime = Timestamp.now(); + return this.mutationQueue + .addMutationBatch(txn, localWriteTime, mutations) + .next(promisedBatch => { + batch = promisedBatch; + // TODO(koss): This is doing an N^2 update by replaying ALL the + // mutations on each document (instead of just the ones added) in + // this batch. + const keys = batch.keys(); + return this.localDocuments.getDocuments(txn, keys); + }) + .next((changedDocuments: MaybeDocumentMap) => { + return { batchId: batch.batchId, changes: changedDocuments }; + }); + } + ); } /** @@ -318,7 +323,7 @@ export class LocalStore { acknowledgeBatch( batchResult: MutationBatchResult ): Promise { - return this.persistence.runTransaction('Acknowledge batch', txn => { + return this.persistence.runTransaction('Acknowledge batch', true, txn => { let affected: DocumentKeySet; return this.mutationQueue .acknowledgeBatch(txn, batchResult.batch, batchResult.streamToken) @@ -357,7 +362,7 @@ export class LocalStore { * @returns The resulting modified documents. */ rejectBatch(batchId: BatchId): Promise { - return this.persistence.runTransaction('Reject batch', txn => { + return this.persistence.runTransaction('Reject batch', true, txn => { let toReject: MutationBatch; let affectedKeys: DocumentKeySet; return this.mutationQueue @@ -394,9 +399,13 @@ export class LocalStore { /** Returns the last recorded stream token for the current user. */ getLastStreamToken(): Promise { - return this.persistence.runTransaction('Get last stream token', txn => { - return this.mutationQueue.getLastStreamToken(txn); - }); + return this.persistence.runTransaction( + 'Get last stream token', + false, // todo: this does require owner lease + txn => { + return this.mutationQueue.getLastStreamToken(txn); + } + ); } /** @@ -405,9 +414,13 @@ export class LocalStore { * response to an error that requires clearing the stream token. */ setLastStreamToken(streamToken: ProtoByteString): Promise { - return this.persistence.runTransaction('Set last stream token', txn => { - return this.mutationQueue.setLastStreamToken(txn, streamToken); - }); + return this.persistence.runTransaction( + 'Set last stream token', + true, + txn => { + return this.mutationQueue.setLastStreamToken(txn, streamToken); + } + ); } /** @@ -428,7 +441,7 @@ export class LocalStore { */ applyRemoteEvent(remoteEvent: RemoteEvent): Promise { const documentBuffer = new RemoteDocumentChangeBuffer(this.remoteDocuments); - return this.persistence.runTransaction('Apply remote event', txn => { + return this.persistence.runTransaction('Apply remote event', true, txn => { const promises = [] as Array>; objUtils.forEachNumber( remoteEvent.targetChanges, @@ -556,28 +569,35 @@ export class LocalStore { * Notify local store of the changed views to locally pin documents. */ notifyLocalViewChanges(viewChanges: LocalViewChanges[]): Promise { - return this.persistence.runTransaction('Notify local view changes', txn => { - const promises = [] as Array>; - for (const view of viewChanges) { - promises.push( - this.queryCache - .getQueryData(txn, view.query) - .next((queryData: QueryData | null) => { - assert( - queryData !== null, - 'Local view changes contain unallocated query.' - ); - const targetId = queryData!.targetId; - this.localViewReferences.addReferences(view.addedKeys, targetId); - this.localViewReferences.removeReferences( - view.removedKeys, - targetId - ); - }) - ); + return this.persistence.runTransaction( + 'Notify local view changes', + true, + txn => { + const promises = [] as Array>; + for (const view of viewChanges) { + promises.push( + this.queryCache + .getQueryData(txn, view.query) + .next((queryData: QueryData | null) => { + assert( + queryData !== null, + 'Local view changes contain unallocated query.' + ); + const targetId = queryData!.targetId; + this.localViewReferences.addReferences( + view.addedKeys, + targetId + ); + this.localViewReferences.removeReferences( + view.removedKeys, + targetId + ); + }) + ); + } + return PersistencePromise.waitFor(promises); } - return PersistencePromise.waitFor(promises); - }); + ); } /** @@ -587,15 +607,20 @@ export class LocalStore { * @returns The next mutation or null if there wasn't one. */ nextMutationBatch(afterBatchId?: BatchId): Promise { - return this.persistence.runTransaction('Get next mutation batch', txn => { - if (afterBatchId === undefined) { - afterBatchId = BATCHID_UNKNOWN; + // TODO(multitab): This needs to run in O(1). + return this.persistence.runTransaction( + 'Get next mutation batch', + false, + txn => { + if (afterBatchId === undefined) { + afterBatchId = BATCHID_UNKNOWN; + } + return this.mutationQueue.getNextMutationBatchAfterBatchId( + txn, + afterBatchId + ); } - return this.mutationQueue.getNextMutationBatchAfterBatchId( - txn, - afterBatchId - ); - }); + ); } /** @@ -603,7 +628,7 @@ export class LocalStore { * found - used for testing. */ readDocument(key: DocumentKey): Promise { - return this.persistence.runTransaction('read document', txn => { + return this.persistence.runTransaction('read document', true, txn => { return this.localDocuments.getDocument(txn, key); }); } @@ -614,7 +639,7 @@ export class LocalStore { * the store can be used to manage its view. */ allocateQuery(query: Query): Promise { - return this.persistence.runTransaction('Allocate query', txn => { + return this.persistence.runTransaction('Allocate query', true, txn => { let queryData: QueryData; return this.queryCache .getQueryData(txn, query) @@ -644,7 +669,7 @@ export class LocalStore { /** Unpin all the documents associated with the given query. */ releaseQuery(query: Query): Promise { - return this.persistence.runTransaction('Release query', txn => { + return this.persistence.runTransaction('Release query', true, txn => { return this.queryCache .getQueryData(txn, query) .next((queryData: QueryData | null) => { @@ -684,7 +709,7 @@ export class LocalStore { * returns the results. */ executeQuery(query: Query): Promise { - return this.persistence.runTransaction('Execute query', txn => { + return this.persistence.runTransaction('Execute query', true, txn => { return this.localDocuments.getDocumentsMatchingQuery(txn, query); }); } @@ -694,9 +719,13 @@ export class LocalStore { * target id in the remote table. */ remoteDocumentKeys(targetId: TargetId): Promise { - return this.persistence.runTransaction('Remote document keys', txn => { - return this.queryCache.getMatchingKeysForTargetId(txn, targetId); - }); + return this.persistence.runTransaction( + 'Remote document keys', + true, + txn => { + return this.queryCache.getMatchingKeysForTargetId(txn, targetId); + } + ); } /** @@ -708,7 +737,7 @@ export class LocalStore { collectGarbage(): Promise { // Call collectGarbage regardless of whether isGCEnabled so the referenceSet // doesn't continue to accumulate the garbage keys. - return this.persistence.runTransaction('Garbage collection', txn => { + return this.persistence.runTransaction('Garbage collection', true, txn => { return this.garbageCollector.collectGarbage(txn).next(garbage => { const promises = [] as Array>; garbage.forEach(key => { diff --git a/packages/firestore/src/local/memory_persistence.ts b/packages/firestore/src/local/memory_persistence.ts index f496ce5f522..020cada4000 100644 --- a/packages/firestore/src/local/memory_persistence.ts +++ b/packages/firestore/src/local/memory_persistence.ts @@ -22,7 +22,11 @@ import { MemoryMutationQueue } from './memory_mutation_queue'; import { MemoryQueryCache } from './memory_query_cache'; import { MemoryRemoteDocumentCache } from './memory_remote_document_cache'; import { MutationQueue } from './mutation_queue'; -import { Persistence, PersistenceTransaction } from './persistence'; +import { + Persistence, + PersistenceTransaction, + PrimaryStateListener +} from './persistence'; import { PersistencePromise } from './persistence_promise'; import { QueryCache } from './query_cache'; import { RemoteDocumentCache } from './remote_document_cache'; @@ -61,6 +65,16 @@ export class MemoryPersistence implements Persistence { return Promise.resolve(); } + tryBecomePrimary(): Promise { + // All clients using memory persistence act as primary. + return Promise.resolve(true); + } + + setPrimaryStateListener(primaryStateListener: PrimaryStateListener) { + // All clients using memory persistence act as primary. + primaryStateListener.applyPrimaryState(true); + } + getMutationQueue(user: User): MutationQueue { let queue = this.mutationQueues[user.toKey()]; if (!queue) { @@ -80,10 +94,13 @@ export class MemoryPersistence implements Persistence { runTransaction( action: string, - operation: (transaction: PersistenceTransaction) => PersistencePromise + requirePrimaryLease: boolean, + transactionOperation: ( + transaction: PersistenceTransaction + ) => PersistencePromise ): Promise { debug(LOG_TAG, 'Starting transaction:', action); - return operation(new MemoryPersistenceTransaction()).toPromise(); + return transactionOperation(new MemoryPersistenceTransaction()).toPromise(); } } diff --git a/packages/firestore/src/local/persistence.ts b/packages/firestore/src/local/persistence.ts index 499633b5e68..d9624c9d013 100644 --- a/packages/firestore/src/local/persistence.ts +++ b/packages/firestore/src/local/persistence.ts @@ -31,6 +31,26 @@ import { RemoteDocumentCache } from './remote_document_cache'; */ export interface PersistenceTransaction {} +/** + * Interface that defines a callback for primary state notifications. This + * callback can be registered with the persistence layer to get notified + * when we transition from primary to secondary state and vice versa. + * + * Note: Instances can only toggle between Primary and Secondary state if + * IndexedDB persistence is enabled and multiple instances are active. If this + * listener is registered with MemoryPersistence, the callback will be called + * exactly once marking the current instance as Primary. + */ +export interface PrimaryStateListener { + /** + * Transitions this instance between Primary and Secondary state. This + * callback can be called after the fact (for example when an instance loses + * its primary lease unexpectedly) and all necessary state transitions should + * be applied synchronously. + */ + applyPrimaryState(primaryClient: boolean): void; +} + /** * Persistence is the lowest-level shared interface to persistent storage in * Firestore. @@ -78,6 +98,24 @@ export interface Persistence { /** Releases any resources held during eager shutdown. */ shutdown(): Promise; + /** + * Registers a listener that gets called when the primary state of the + * instance changes. Upon registering, this listener is invoked immediately + * with the current primary state. + */ + setPrimaryStateListener(primaryStateListener: PrimaryStateListener); + + /** + * Execute the primary release check immediately (outside of its regular + * scheduled interval). Returns whether the instance is primary. + * + * TODO(multiab): Consider running this on the AsyncQueue and replacing with + * `runDelayedOperationsEarly`. + * + * Used for testing. + */ + tryBecomePrimary(): Promise; + /** * Returns a MutationQueue representing the persisted mutations for the * given user. @@ -121,11 +159,14 @@ export interface Persistence { * * @param action A description of the action performed by this transaction, * used for logging. + * @param requirePrimaryLease Whether this transaction can only be executed + * by the primary client only. * @param transactionOperation The operation to run inside a transaction. * @return A promise that is resolved once the transaction completes. */ runTransaction( action: string, + requirePrimaryLease: boolean, transactionOperation: ( transaction: PersistenceTransaction ) => PersistencePromise diff --git a/packages/firestore/src/platform/platform.ts b/packages/firestore/src/platform/platform.ts index cb2ff6dc5b7..39b12ff5feb 100644 --- a/packages/firestore/src/platform/platform.ts +++ b/packages/firestore/src/platform/platform.ts @@ -40,6 +40,12 @@ export interface Platform { /** Converts a binary string to a Base64 encoded string. */ btoa(raw: string): string; + /** The Platform's 'window' implementation or null if not available. */ + readonly window: Window | null; + + /** The Platform's 'document' implementation or null if not available. */ + readonly document: Document | null; + /** True if and only if the Base64 conversion functions are available. */ readonly base64Available: boolean; diff --git a/packages/firestore/src/platform_browser/browser_platform.ts b/packages/firestore/src/platform_browser/browser_platform.ts index 157af2d7ee0..b3ee09a939a 100644 --- a/packages/firestore/src/platform_browser/browser_platform.ts +++ b/packages/firestore/src/platform_browser/browser_platform.ts @@ -27,6 +27,10 @@ export class BrowserPlatform implements Platform { readonly emptyByteString = ''; + readonly document = document; + + readonly window = window; + constructor() { this.base64Available = typeof atob !== 'undefined'; } diff --git a/packages/firestore/src/platform_node/node_platform.ts b/packages/firestore/src/platform_node/node_platform.ts index 8b507ddf6c5..df34f48fa64 100644 --- a/packages/firestore/src/platform_node/node_platform.ts +++ b/packages/firestore/src/platform_node/node_platform.ts @@ -31,6 +31,10 @@ export class NodePlatform implements Platform { readonly emptyByteString = new Uint8Array(0); + readonly document = null; + + readonly window = null; + loadConnection(databaseInfo: DatabaseInfo): Promise { const protos = loadProtos(); return Promise.resolve(new GrpcConnection(protos, databaseInfo)); diff --git a/packages/firestore/test/unit/local/mutation_queue.test.ts b/packages/firestore/test/unit/local/mutation_queue.test.ts index 98efa331ac0..f2ea4bf85e3 100644 --- a/packages/firestore/test/unit/local/mutation_queue.test.ts +++ b/packages/firestore/test/unit/local/mutation_queue.test.ts @@ -70,7 +70,7 @@ describe('IndexedDbMutationQueue', () => { describe('loadNextBatchIdFromDb', () => { function loadNextBatchId(): Promise { - return persistence.runTransaction('loadNextBatchIdFromDb', txn => { + return persistence.runTransaction('loadNextBatchIdFromDb', true, txn => { return IndexedDbMutationQueue.loadNextBatchIdFromDb(txn).next( batchId => { return batchId; @@ -80,7 +80,7 @@ describe('IndexedDbMutationQueue', () => { } function addDummyBatch(userId: string, batchId: BatchId): Promise { - return persistence.runTransaction('addDummyBatch', transaction => { + return persistence.runTransaction('addDummyBatch', true, transaction => { const txn = transaction as SimpleDbTransaction; const store = txn.store<[string, number], DbMutationBatch>( DbMutationBatch.store diff --git a/packages/firestore/test/unit/local/persistence_test_helpers.ts b/packages/firestore/test/unit/local/persistence_test_helpers.ts index 66936e358d3..c6eb3bcca60 100644 --- a/packages/firestore/test/unit/local/persistence_test_helpers.ts +++ b/packages/firestore/test/unit/local/persistence_test_helpers.ts @@ -25,6 +25,7 @@ import { ClientKey } from '../../../src/local/shared_client_state'; import { BatchId, TargetId } from '../../../src/core/types'; +import { BrowserPlatform } from '../../../src/platform_browser/browser_platform'; /** The persistence prefix used for testing in IndexedBD and LocalStorage. */ export const TEST_PERSISTENCE_PREFIX = 'PersistenceTestHelpers'; @@ -48,7 +49,8 @@ export async function testIndexedDbPersistence(): Promise< const serializer = new JsonProtoSerializer(partition, { useProto3Json: true }); - const persistence = new IndexedDbPersistence(prefix, serializer); + const platform = new BrowserPlatform(); + const persistence = new IndexedDbPersistence(prefix, platform, serializer); await persistence.start(); return persistence; } diff --git a/packages/firestore/test/unit/local/test_garbage_collector.ts b/packages/firestore/test/unit/local/test_garbage_collector.ts index 71bae6e4598..715c4c37d3d 100644 --- a/packages/firestore/test/unit/local/test_garbage_collector.ts +++ b/packages/firestore/test/unit/local/test_garbage_collector.ts @@ -27,7 +27,7 @@ export class TestGarbageCollector { collectGarbage(): Promise { return this.persistence - .runTransaction('garbageCollect', txn => { + .runTransaction('garbageCollect', true, txn => { return this.gc.collectGarbage(txn); }) .then(garbage => { diff --git a/packages/firestore/test/unit/local/test_mutation_queue.ts b/packages/firestore/test/unit/local/test_mutation_queue.ts index 5f5d9c121fd..974f5a9a518 100644 --- a/packages/firestore/test/unit/local/test_mutation_queue.ts +++ b/packages/firestore/test/unit/local/test_mutation_queue.ts @@ -34,27 +34,27 @@ export class TestMutationQueue { constructor(public persistence: Persistence, public queue: MutationQueue) {} start(): Promise { - return this.persistence.runTransaction('start', txn => { + return this.persistence.runTransaction('start', true, txn => { return this.queue.start(txn); }); } checkEmpty(): Promise { - return this.persistence.runTransaction('checkEmpty', txn => { + return this.persistence.runTransaction('checkEmpty', true, txn => { return this.queue.checkEmpty(txn); }); } countBatches(): Promise { return this.persistence - .runTransaction('countBatches', txn => { + .runTransaction('countBatches', true, txn => { return this.queue.getAllMutationBatches(txn); }) .then(batches => batches.length); } getNextBatchId(): Promise { - return this.persistence.runTransaction('getNextBatchId', txn => { + return this.persistence.runTransaction('getNextBatchId', true, txn => { return this.queue.getNextBatchId(txn); }); } @@ -62,6 +62,7 @@ export class TestMutationQueue { getHighestAcknowledgedBatchId(): Promise { return this.persistence.runTransaction( 'getHighestAcknowledgedBatchId', + true, txn => { return this.queue.getHighestAcknowledgedBatchId(txn); } @@ -72,31 +73,35 @@ export class TestMutationQueue { batch: MutationBatch, streamToken: ProtoByteString ): Promise { - return this.persistence.runTransaction('acknowledgeThroughBatchId', txn => { - return this.queue.acknowledgeBatch(txn, batch, streamToken); - }); + return this.persistence.runTransaction( + 'acknowledgeThroughBatchId', + true, + txn => { + return this.queue.acknowledgeBatch(txn, batch, streamToken); + } + ); } getLastStreamToken(): Promise { - return this.persistence.runTransaction('getLastStreamToken', txn => { + return this.persistence.runTransaction('getLastStreamToken', true, txn => { return this.queue.getLastStreamToken(txn); }) as AnyDuringMigration; } setLastStreamToken(streamToken: string): Promise { - return this.persistence.runTransaction('setLastStreamToken', txn => { + return this.persistence.runTransaction('setLastStreamToken', true, txn => { return this.queue.setLastStreamToken(txn, streamToken); }); } addMutationBatch(mutations: Mutation[]): Promise { - return this.persistence.runTransaction('addMutationBatch', txn => { + return this.persistence.runTransaction('addMutationBatch', true, txn => { return this.queue.addMutationBatch(txn, Timestamp.now(), mutations); }); } lookupMutationBatch(batchId: BatchId): Promise { - return this.persistence.runTransaction('lookupMutationBatch', txn => { + return this.persistence.runTransaction('lookupMutationBatch', true, txn => { return this.queue.lookupMutationBatch(txn, batchId); }); } @@ -106,6 +111,7 @@ export class TestMutationQueue { ): Promise { return this.persistence.runTransaction( 'getNextMutationBatchAfterBatchId', + true, txn => { return this.queue.getNextMutationBatchAfterBatchId(txn, batchId); } @@ -113,9 +119,13 @@ export class TestMutationQueue { } getAllMutationBatches(): Promise { - return this.persistence.runTransaction('getAllMutationBatches', txn => { - return this.queue.getAllMutationBatches(txn); - }); + return this.persistence.runTransaction( + 'getAllMutationBatches', + true, + txn => { + return this.queue.getAllMutationBatches(txn); + } + ); } getAllMutationBatchesThroughBatchId( @@ -123,6 +133,7 @@ export class TestMutationQueue { ): Promise { return this.persistence.runTransaction( 'getAllMutationBatchesThroughBatchId', + true, txn => { return this.queue.getAllMutationBatchesThroughBatchId(txn, batchId); } @@ -134,6 +145,7 @@ export class TestMutationQueue { ): Promise { return this.persistence.runTransaction( 'getAllMutationBatchesAffectingDocumentKey', + true, txn => { return this.queue.getAllMutationBatchesAffectingDocumentKey( txn, @@ -146,6 +158,7 @@ export class TestMutationQueue { getAllMutationBatchesAffectingQuery(query: Query): Promise { return this.persistence.runTransaction( 'getAllMutationBatchesAffectingQuery', + true, txn => { return this.queue.getAllMutationBatchesAffectingQuery(txn, query); } @@ -153,13 +166,17 @@ export class TestMutationQueue { } removeMutationBatches(batches: MutationBatch[]): Promise { - return this.persistence.runTransaction('removeMutationBatches', txn => { - return this.queue.removeMutationBatches(txn, batches); - }); + return this.persistence.runTransaction( + 'removeMutationBatches', + true, + txn => { + return this.queue.removeMutationBatches(txn, batches); + } + ); } collectGarbage(gc: GarbageCollector): Promise { - return this.persistence.runTransaction('garbageCollection', txn => { + return this.persistence.runTransaction('garbageCollection', true, txn => { return gc.collectGarbage(txn); }); } diff --git a/packages/firestore/test/unit/local/test_query_cache.ts b/packages/firestore/test/unit/local/test_query_cache.ts index eefe0032cdb..d4439ca52ea 100644 --- a/packages/firestore/test/unit/local/test_query_cache.ts +++ b/packages/firestore/test/unit/local/test_query_cache.ts @@ -31,25 +31,25 @@ export class TestQueryCache { constructor(public persistence: Persistence, public cache: QueryCache) {} start(): Promise { - return this.persistence.runTransaction('start', txn => + return this.persistence.runTransaction('start', true, txn => this.cache.start(txn) ); } addQueryData(queryData: QueryData): Promise { - return this.persistence.runTransaction('addQueryData', txn => { + return this.persistence.runTransaction('addQueryData', true, txn => { return this.cache.addQueryData(txn, queryData); }); } removeQueryData(queryData: QueryData): Promise { - return this.persistence.runTransaction('addQueryData', txn => { + return this.persistence.runTransaction('addQueryData', true, txn => { return this.cache.removeQueryData(txn, queryData); }); } getQueryData(query: Query): Promise { - return this.persistence.runTransaction('getQueryData', txn => { + return this.persistence.runTransaction('getQueryData', true, txn => { return this.cache.getQueryData(txn, query); }); } @@ -63,7 +63,7 @@ export class TestQueryCache { } addMatchingKeys(keys: DocumentKey[], targetId: TargetId): Promise { - return this.persistence.runTransaction('addMatchingKeys', txn => { + return this.persistence.runTransaction('addMatchingKeys', true, txn => { let set = documentKeySet(); for (const key of keys) { set = set.add(key); @@ -73,7 +73,7 @@ export class TestQueryCache { } removeMatchingKeys(keys: DocumentKey[], targetId: TargetId): Promise { - return this.persistence.runTransaction('removeMatchingKeys', txn => { + return this.persistence.runTransaction('removeMatchingKeys', true, txn => { let set = documentKeySet(); for (const key of keys) { set = set.add(key); @@ -84,7 +84,7 @@ export class TestQueryCache { getMatchingKeysForTargetId(targetId: TargetId): Promise { return this.persistence - .runTransaction('getMatchingKeysForTargetId', txn => { + .runTransaction('getMatchingKeysForTargetId', true, txn => { return this.cache.getMatchingKeysForTargetId(txn, targetId); }) .then(keySet => { @@ -97,6 +97,7 @@ export class TestQueryCache { removeMatchingKeysForTargetId(targetId: TargetId): Promise { return this.persistence.runTransaction( 'removeMatchingKeysForTargetId', + true, txn => { return this.cache.removeMatchingKeysForTargetId(txn, targetId); } @@ -104,7 +105,7 @@ export class TestQueryCache { } containsKey(key: DocumentKey): Promise { - return this.persistence.runTransaction('containsKey', txn => { + return this.persistence.runTransaction('containsKey', true, txn => { return this.cache.containsKey(txn, key); }); } @@ -112,6 +113,7 @@ export class TestQueryCache { setLastRemoteSnapshotVersion(version: SnapshotVersion) { return this.persistence.runTransaction( 'setLastRemoteSnapshotVersion', + true, txn => this.cache.setLastRemoteSnapshotVersion(txn, version) ); } diff --git a/packages/firestore/test/unit/local/test_remote_document_cache.ts b/packages/firestore/test/unit/local/test_remote_document_cache.ts index 4183c77c30a..9743af8cf7e 100644 --- a/packages/firestore/test/unit/local/test_remote_document_cache.ts +++ b/packages/firestore/test/unit/local/test_remote_document_cache.ts @@ -32,26 +32,30 @@ export class TestRemoteDocumentCache { ) {} addEntry(maybeDocument: MaybeDocument): Promise { - return this.persistence.runTransaction('addEntry', txn => { + return this.persistence.runTransaction('addEntry', true, txn => { return this.cache.addEntry(txn, maybeDocument); }); } removeEntry(documentKey: DocumentKey): Promise { - return this.persistence.runTransaction('removeEntry', txn => { + return this.persistence.runTransaction('removeEntry', true, txn => { return this.cache.removeEntry(txn, documentKey); }); } getEntry(documentKey: DocumentKey): Promise { - return this.persistence.runTransaction('getEntry', txn => { + return this.persistence.runTransaction('getEntry', true, txn => { return this.cache.getEntry(txn, documentKey); }); } getDocumentsMatchingQuery(query: Query): Promise { - return this.persistence.runTransaction('getDocumentsMatchingQuery', txn => { - return this.cache.getDocumentsMatchingQuery(txn, query); - }); + return this.persistence.runTransaction( + 'getDocumentsMatchingQuery', + true, + txn => { + return this.cache.getDocumentsMatchingQuery(txn, query); + } + ); } } diff --git a/packages/firestore/test/unit/local/test_remote_document_change_buffer.ts b/packages/firestore/test/unit/local/test_remote_document_change_buffer.ts index b2ef7918f19..38105d8b275 100644 --- a/packages/firestore/test/unit/local/test_remote_document_change_buffer.ts +++ b/packages/firestore/test/unit/local/test_remote_document_change_buffer.ts @@ -35,13 +35,13 @@ export class TestRemoteDocumentChangeBuffer { } getEntry(documentKey: DocumentKey): Promise { - return this.persistence.runTransaction('getEntry', txn => { + return this.persistence.runTransaction('getEntry', true, txn => { return this.buffer.getEntry(txn, documentKey); }); } apply(): Promise { - return this.persistence.runTransaction('apply', txn => { + return this.persistence.runTransaction('apply', true, txn => { return this.buffer.apply(txn); }); } diff --git a/packages/firestore/test/unit/specs/describe_spec.ts b/packages/firestore/test/unit/specs/describe_spec.ts index f59b302b818..667bf9c61b9 100644 --- a/packages/firestore/test/unit/specs/describe_spec.ts +++ b/packages/firestore/test/unit/specs/describe_spec.ts @@ -26,6 +26,8 @@ import { SpecStep } from './spec_test_runner'; const EXCLUSIVE_TAG = 'exclusive'; // Persistence-related tests. const PERSISTENCE_TAG = 'persistence'; +// Multi-client related tests. Requires persistence. +const MULTI_CLIENT_TAG = 'multi-client'; // Explicit per-platform disable flags. const NO_WEB_TAG = 'no-web'; const NO_ANDROID_TAG = 'no-android'; @@ -33,11 +35,18 @@ const NO_IOS_TAG = 'no-ios'; const KNOWN_TAGS = [ EXCLUSIVE_TAG, PERSISTENCE_TAG, + MULTI_CLIENT_TAG, NO_WEB_TAG, NO_ANDROID_TAG, NO_IOS_TAG ]; +// Filters out multi-client tests if persistence is not enabled. +const MULTI_CLIENT_TEST_FILTER = ( + tags: string[], + persistenceEnabled: boolean +) => tags.indexOf(MULTI_CLIENT_TAG) !== -1 && persistenceEnabled; + const WEB_SPEC_TEST_FILTER = (tags: string[]) => tags.indexOf(NO_WEB_TAG) === -1; @@ -69,6 +78,20 @@ export function setSpecJSONHandler(writer: (json: string) => void) { writeJSONFile = writer; } +/** Gets the test runner based on the specified tags. */ +function getTestRunner(tags, persistenceEnabled): Function { + if ( + !MULTI_CLIENT_TEST_FILTER(tags, persistenceEnabled) || + !WEB_SPEC_TEST_FILTER(tags) + ) { + return it.skip; + } else if (tags.indexOf(EXCLUSIVE_TAG) >= 0) { + return it.only; + } else { + return it; + } +} + /** * Like it(), but for spec tests. * @param name A name to give the test. @@ -98,14 +121,7 @@ export function specTest( : [false]; for (const usePersistence of persistenceModes) { const spec = builder(); - let runner: Function; - if (tags.indexOf(EXCLUSIVE_TAG) >= 0) { - runner = it.only; - } else if (!WEB_SPEC_TEST_FILTER(tags)) { - runner = it.skip; - } else { - runner = it; - } + const runner = getTestRunner(tags, usePersistence); const mode = usePersistence ? '(Persistence)' : '(Memory)'; const fullName = `${mode} ${name}`; runner(fullName, () => { diff --git a/packages/firestore/test/unit/specs/persistence_spec.test.ts b/packages/firestore/test/unit/specs/persistence_spec.test.ts index 081af702f5e..dc49f304cc7 100644 --- a/packages/firestore/test/unit/specs/persistence_spec.test.ts +++ b/packages/firestore/test/unit/specs/persistence_spec.test.ts @@ -19,7 +19,7 @@ import { Query } from '../../../src/core/query'; import { doc, path } from '../../util/helpers'; import { describeSpec, specTest } from './describe_spec'; -import { spec } from './spec_builder'; +import { client, spec } from './spec_builder'; describeSpec('Persistence:', ['persistence'], () => { specTest('Local mutations are persisted and re-sent', [], () => { @@ -182,4 +182,50 @@ describeSpec('Persistence:', ['persistence'], () => { }) ); }); + + specTest('Single tab acquires primary lease', ['multi-client'], () => { + // This test simulates primary state handoff between two background tabs. + // With all instances are in the background, the first active tab acquires + // ownership. + return client(0) + .becomeHidden() + .expectPrimaryState(true) + .client(1) + .becomeHidden() + .expectPrimaryState(false) + .client(0) + .shutdown() + .expectPrimaryState(false) + .client(1) + .restart() + .expectPrimaryState(true); + }); + + specTest('Foreground tab acquires primary lease', ['multi-client'], () => { + // This test verifies that in a multi-client scenario, a foreground tab + // takes precedence when a new primary client is elected. + return ( + client(0) + .becomeHidden() + .expectPrimaryState(true) + .client(1) + .becomeHidden() + .expectPrimaryState(false) + .client(2) + .becomeVisible() + .expectPrimaryState(false) + .client(0) + // Shutdown the client that is currently holding the primary lease. + .shutdown() + .expectPrimaryState(false) + .client(1) + // Restart client 1. This client is in the background and doesn't grab + // the primary lease as client 2 is in the foreground. + .restart() + .expectPrimaryState(false) + .client(2) + .restart() + .expectPrimaryState(true) + ); + }); }); diff --git a/packages/firestore/test/unit/specs/spec_builder.ts b/packages/firestore/test/unit/specs/spec_builder.ts index c59434b0f4e..e4f32f6dfe1 100644 --- a/packages/firestore/test/unit/specs/spec_builder.ts +++ b/packages/firestore/test/unit/specs/spec_builder.ts @@ -50,11 +50,12 @@ import { * duplicate tests in every client. */ export class SpecBuilder { - private config: SpecConfig = { useGarbageCollection: true }; - private steps: SpecStep[] = []; + protected config: SpecConfig = { useGarbageCollection: true, numClients: 1 }; // currentStep is built up (in particular, expectations can be added to it) // until nextStep() is called to append it to steps. - private currentStep: SpecStep | null = null; + protected currentStep: SpecStep | null = null; + + private steps: SpecStep[] = []; private queryMapping: { [query: string]: TargetId } = {}; private limboMapping: { [key: string]: TargetId } = {}; @@ -194,6 +195,22 @@ export class SpecBuilder { return this; } + becomeHidden(): SpecBuilder { + this.nextStep(); + this.currentStep = { + applyClientState: { visibility: 'hidden' } + }; + return this; + } + + becomeVisible(): SpecBuilder { + this.nextStep(); + this.currentStep = { + applyClientState: { visibility: 'visible' } + }; + return this; + } + changeUser(uid: string | null): SpecBuilder { this.nextStep(); this.currentStep = { changeUser: uid }; @@ -229,15 +246,33 @@ export class SpecBuilder { limboDocs: [] } }; + // Reset our mappings / target ids since all existing listens will be + // forgotten. + this.resetInMemoryState(); + return this; + } + shutdown(): SpecBuilder { + this.nextStep(); + this.currentStep = { + shutdown: true, + stateExpect: { + activeTargets: {}, + limboDocs: [] + } + }; // Reset our mappings / target ids since all existing listens will be - // forgotten + // forgotten. + this.resetInMemoryState(); + return this; + } + + private resetInMemoryState(): void { this.queryMapping = {}; this.limboMapping = {}; this.activeTargets = {}; this.queryIdGenerator = TargetIdGenerator.forLocalStore(); this.limboIdGenerator = TargetIdGenerator.forSyncEngine(); - return this; } /** Overrides the currently expected set of active targets. */ @@ -569,6 +604,14 @@ export class SpecBuilder { return this; } + expectPrimaryState(isPrimary: boolean): SpecBuilder { + this.assertStep('Expectations requires previous step'); + const currentStep = this.currentStep!; + currentStep.stateExpect = currentStep.stateExpect || {}; + currentStep.stateExpect.isPrimary = isPrimary; + return this; + } + private static queryToSpec(query: Query): SpecQuery { // TODO(dimond): full query support const spec: SpecQuery = { path: query.path.canonicalString() }; @@ -616,7 +659,7 @@ export class SpecBuilder { return key.path.canonicalString(); } - private nextStep(): void { + protected nextStep(): void { if (this.currentStep !== null) { this.steps.push(this.currentStep); this.currentStep = null; @@ -646,6 +689,243 @@ export class SpecBuilder { } } -export function spec() { +/** + * SpecBuilder that supports serialized interactions between different clients. + * + * Use `client(clientIndex)` to switch between clients. + */ +export class MultiClientSpecBuilder extends SpecBuilder { + private activeClientIndex = -1; + + client(clientIndex: number): MultiClientSpecBuilder { + // Since `currentStep` is fully self-contained and does not rely on previous + // state, we don't need to use a different SpecBuilder instance for each + // client. + this.nextStep(); + this.activeClientIndex = clientIndex; + this.config.numClients = Math.max( + this.config.numClients, + this.activeClientIndex + 1 + ); + return this; + } + + protected nextStep() { + if (this.currentStep !== null) { + this.currentStep.clientIndex = this.activeClientIndex; + } + super.nextStep(); + } + + withGCEnabled(gcEnabled: boolean): MultiClientSpecBuilder { + super.withGCEnabled(gcEnabled); + return this; + } + + userListens(query: Query, resumeToken?: string): MultiClientSpecBuilder { + super.userListens(query, resumeToken); + return this; + } + + restoreListen(query: Query, resumeToken: string): MultiClientSpecBuilder { + super.restoreListen(query, resumeToken); + return this; + } + + userUnlistens(query: Query): MultiClientSpecBuilder { + super.userUnlistens(query); + return this; + } + + userSets(key: string, value: JsonObject): MultiClientSpecBuilder { + super.userSets(key, value); + return this; + } + + userPatches(key: string, value: JsonObject): MultiClientSpecBuilder { + super.userPatches(key, value); + return this; + } + + userDeletes(key: string): MultiClientSpecBuilder { + super.userDeletes(key); + return this; + } + + becomeHidden(): MultiClientSpecBuilder { + super.becomeHidden(); + return this; + } + + becomeVisible(): MultiClientSpecBuilder { + super.becomeVisible(); + return this; + } + + changeUser(uid: string | null): MultiClientSpecBuilder { + super.changeUser(uid); + return this; + } + + disableNetwork(): MultiClientSpecBuilder { + super.disableNetwork(); + return this; + } + + enableNetwork(): MultiClientSpecBuilder { + super.enableNetwork(); + return this; + } + + restart(): MultiClientSpecBuilder { + super.restart(); + return this; + } + + expectActiveTargets(...targets): MultiClientSpecBuilder { + super.expectActiveTargets(...targets); + return this; + } + + expectLimboDocs(...keys): MultiClientSpecBuilder { + super.expectLimboDocs(...keys); + return this; + } + + ackLimbo( + version: TestSnapshotVersion, + doc: Document | NoDocument + ): MultiClientSpecBuilder { + super.ackLimbo(version, doc); + return this; + } + + watchRemovesLimboTarget(doc: Document | NoDocument): MultiClientSpecBuilder { + super.watchRemovesLimboTarget(doc); + return this; + } + + writeAcks( + version: TestSnapshotVersion, + options?: { expectUserCallback: boolean } + ): MultiClientSpecBuilder { + super.writeAcks(version, options); + return this; + } + + failWrite( + err: RpcError, + options?: { expectUserCallback: boolean } + ): MultiClientSpecBuilder { + super.failWrite(err, options); + return this; + } + + watchAcks(query: Query): MultiClientSpecBuilder { + super.watchAcks(query); + return this; + } + + watchCurrents(query: Query, resumeToken: string): MultiClientSpecBuilder { + super.watchCurrents(query, resumeToken); + return this; + } + + watchRemoves(query: Query, cause?: RpcError): MultiClientSpecBuilder { + super.watchRemoves(query, cause); + return this; + } + + watchSends( + targets: { affects?: Query[]; removed?: Query[] }, + ...docs + ): MultiClientSpecBuilder { + super.watchSends(targets, ...docs); + return this; + } + + watchRemovesDoc(key: DocumentKey, ...targets): MultiClientSpecBuilder { + super.watchRemovesDoc(key, ...targets); + return this; + } + + watchFilters(queries: Query[], ...docs): MultiClientSpecBuilder { + super.watchFilters(queries, ...docs); + return this; + } + + watchResets(...queries): MultiClientSpecBuilder { + super.watchResets(...queries); + return this; + } + + watchSnapshots(version: TestSnapshotVersion): MultiClientSpecBuilder { + super.watchSnapshots(version); + return this; + } + + watchAcksFull( + query: Query, + version: TestSnapshotVersion, + ...docs + ): MultiClientSpecBuilder { + super.watchAcksFull(query, version, ...docs); + return this; + } + + watchStreamCloses(error: Code): MultiClientSpecBuilder { + super.watchStreamCloses(error); + return this; + } + + expectEvents( + query: Query, + events: { + fromCache?: boolean; + hasPendingWrites?: boolean; + added?: Document[]; + modified?: Document[]; + removed?: Document[]; + metadata?: Document[]; + errorCode?: Code; + } + ): MultiClientSpecBuilder { + super.expectEvents(query, events); + return this; + } + + expectWatchStreamRequestCount(num: number): MultiClientSpecBuilder { + super.expectWatchStreamRequestCount(num); + return this; + } + + expectNumOutstandingWrites(num: number): MultiClientSpecBuilder { + super.expectNumOutstandingWrites(num); + return this; + } + + expectPrimaryState(isPrimary: boolean): MultiClientSpecBuilder { + super.expectPrimaryState(isPrimary); + return this; + } + + expectWriteStreamRequestCount(num: number): MultiClientSpecBuilder { + super.expectWriteStreamRequestCount(num); + return this; + } + + shutdown(): MultiClientSpecBuilder { + super.shutdown(); + return this; + } +} + +/** Starts a new single-client SpecTest. */ +export function spec(): SpecBuilder { return new SpecBuilder(); } + +/** Starts a new multi-client SpecTest. */ +export function client(num: number): MultiClientSpecBuilder { + return new MultiClientSpecBuilder().client(num); +} diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index 67767313259..09de5d4c140 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -50,7 +50,11 @@ import { DocumentOptions } from '../../../src/model/document'; import { DocumentKey } from '../../../src/model/document_key'; import { JsonObject } from '../../../src/model/field_value'; import { Mutation } from '../../../src/model/mutation'; -import { emptyByteString } from '../../../src/platform/platform'; +import { + emptyByteString, + Platform, + PlatformSupport +} from '../../../src/platform/platform'; import { Connection, Stream } from '../../../src/remote/connection'; import { Datastore } from '../../../src/remote/datastore'; import { ExistenceFilter } from '../../../src/remote/existence_filter'; @@ -342,7 +346,11 @@ abstract class TestRunner { private serializer: JsonProtoSerializer; - constructor(private readonly name: string, config: SpecConfig) { + constructor( + private readonly name: string, + protected readonly platform: MockPlatform, + config: SpecConfig + ) { this.databaseInfo = new DatabaseInfo( new DatabaseId('project'), 'persistenceKey', @@ -398,6 +406,7 @@ abstract class TestRunner { this.user ); + this.persistence.setPrimaryStateListener(this.syncEngine); // Setup wiring between sync engine and remote store this.remoteStore.syncEngine = this.syncEngine; @@ -413,7 +422,6 @@ abstract class TestRunner { protected abstract getPersistence( serializer: JsonProtoSerializer ): Persistence; - protected abstract destroyPersistence(): Promise; async start(): Promise { this.connection.reset(); @@ -425,18 +433,15 @@ abstract class TestRunner { async shutdown(): Promise { await this.remoteStore.shutdown(); await this.persistence.shutdown(); - await this.destroyPersistence(); } - run(steps: SpecStep[]): Promise { - console.log('Running spec: ' + this.name); - return sequence(steps, async step => { - await this.doStep(step); - await this.queue.drain(/* executeDelayedTasks */ false); - this.validateStepExpectations(step.expect!); - this.validateStateExpectations(step.stateExpect!); - this.eventList = []; - }); + /** Runs a single SpecStep on this runner. */ + async run(step: SpecStep): Promise { + await this.doStep(step); + await this.queue.drain(/* executeDelayedTasks */ false); + this.validateStepExpectations(step.expect!); + this.validateStateExpectations(step.stateExpect!); + this.eventList = []; } private doStep(step: SpecStep): Promise { @@ -475,6 +480,11 @@ abstract class TestRunner { } else if ('restart' in step) { assert(step.restart!, 'Restart cannot be false'); return this.doRestart(); + } else if ('shutdown' in step) { + assert(step.shutdown!, 'Shutdown cannot be false'); + return this.doShutdown(); + } else if ('applyClientState' in step) { + return this.doApplyClientState(step.applyClientState!); } else if ('changeUser' in step) { return this.doChangeUser(step.changeUser!); } else { @@ -778,6 +788,11 @@ abstract class TestRunner { await this.remoteStore.enableNetwork(); } + private async doShutdown(): Promise { + await this.remoteStore.shutdown(); + await this.persistence.shutdown(); + } + private async doRestart(): Promise { // Reinitialize everything, except the persistence. // No local store to shutdown. @@ -791,6 +806,15 @@ abstract class TestRunner { await this.localStore.start(); await this.remoteStore.start(); }); + + await this.persistence.tryBecomePrimary(); + } + + private doApplyClientState(state: SpecClientState): Promise { + if (state.visibility) { + this.platform.fireVisibilityEvent(state.visibility!); + } + return Promise.resolve(); } private doChangeUser(user: string | null): Promise { @@ -842,6 +866,9 @@ abstract class TestRunner { if ('activeTargets' in expectation) { this.expectedActiveTargets = expectation.activeTargets!; } + if ('isPrimary' in expectation) { + expect(this.syncEngine.isPrimaryClient).to.eq(expectation.isPrimary!); + } } // Always validate that the expected limbo docs match the actual limbo docs @@ -999,12 +1026,86 @@ class MemoryTestRunner extends TestRunner { return new MemoryPersistence(); } - protected destroyPersistence(): Promise { + static destroyPersistence(): Promise { // Nothing to do. return Promise.resolve(); } } +/** + * Implementation of `Platform` that allows mocking of `visibilitychange` + * events. + * */ +class MockPlatform implements Platform { + private visibilityState: VisibilityState = 'unloaded'; + private visibilityListener: EventListener | null = null; + private mockDocument: Document; + + constructor(private readonly basePlatform: Platform) { + this.initMockDocument(); + } + + initMockDocument() { + this.mockDocument = { + visibilityState: this.visibilityState, + addEventListener: (type: string, listener: EventListener) => { + assert( + type === 'visibilitychange', + "MockPlatform only supports events of type 'visibilitychange'" + ); + this.visibilityListener = listener; + }, + removeEventListener: (type: string, listener: EventListener) => { + if (listener === this.visibilityListener) { + this.visibilityListener = null; + } + } + } as any; // tslint:disable-line:no-any Not implementing the entire document interface + } + + fireVisibilityEvent(visibility: VisibilityState) { + this.visibilityState = visibility; + if (this.visibilityListener) { + this.visibilityListener(new Event('visibilitychange')); + } + } + + get window(): Window | null { + return this.basePlatform.window; + } + + get document(): Document { + return this.mockDocument; + } + + get base64Available(): boolean { + return this.basePlatform.base64Available; + } + + get emptyByteString(): ProtoByteString { + return this.basePlatform.emptyByteString; + } + + loadConnection(databaseInfo: DatabaseInfo): Promise { + return this.basePlatform.loadConnection(databaseInfo); + } + + newSerializer(databaseId: DatabaseId): JsonProtoSerializer { + return this.basePlatform.newSerializer(databaseId); + } + + formatJSON(value: AnyJs): string { + return this.basePlatform.formatJSON(value); + } + + atob(encoded: string): string { + return this.basePlatform.atob(encoded); + } + + btoa(raw: string): string { + return this.basePlatform.btoa(raw); + } +} /** * Runs the specs using IndexedDbPersistence, the creator must ensure that it is * enabled for the platform. @@ -1015,11 +1116,12 @@ class IndexedDbTestRunner extends TestRunner { protected getPersistence(serializer: JsonProtoSerializer): Persistence { return new IndexedDbPersistence( IndexedDbTestRunner.TEST_DB_NAME, + this.platform, serializer ); } - protected destroyPersistence(): Promise { + static destroyPersistence(): Promise { return SimpleDb.delete( IndexedDbTestRunner.TEST_DB_NAME + IndexedDbPersistence.MAIN_DATABASE ); @@ -1037,17 +1139,31 @@ export async function runSpec( config: SpecConfig, steps: SpecStep[] ): Promise { - let runner: TestRunner; - if (usePersistence) { - runner = new IndexedDbTestRunner(name, config); - } else { - runner = new MemoryTestRunner(name, config); + console.log('Running spec: ' + name); + const platform = new MockPlatform(PlatformSupport.getPlatform()); + let runners: TestRunner[] = []; + for (let i = 0; i < config.numClients; ++i) { + if (usePersistence) { + runners.push(new IndexedDbTestRunner(name, platform, config)); + } else { + runners.push(new MemoryTestRunner(name, platform, config)); + } + await runners[i].start(); } - await runner.start(); try { - await runner.run(steps); + let stepcnt = 0; + await sequence(steps, async step => { + return runners[step.clientIndex || 0].run(step); + }); } finally { - await runner.shutdown(); + for (const runner of runners) { + await runner.shutdown(); + } + if (usePersistence) { + await IndexedDbTestRunner.destroyPersistence(); + } else { + await MemoryTestRunner.destroyPersistence(); + } } } @@ -1055,6 +1171,9 @@ export async function runSpec( export interface SpecConfig { /** A boolean to enable / disable GC. */ useGarbageCollection: boolean; + + /** The number of active clients for this test run. */ + numClients: number; } /** @@ -1062,6 +1181,8 @@ export interface SpecConfig { * set and optionally expected events in the `expect` field. */ export interface SpecStep { + /** The index of the current client for multi-client spec tests. */ + clientIndex?: number; /** Listen to a new query (must be unique) */ userListen?: SpecUserListen; /** Unlisten from a query (must be listened to) */ @@ -1101,6 +1222,9 @@ export interface SpecStep { /** Enable or disable RemoteStore's network connection. */ enableNetwork?: boolean; + /** Changes the metadata state of a client instance. */ + applyClientState?: SpecClientState; + /** Change to a new active user (specified by uid or null for anonymous). */ changeUser?: string | null; @@ -1111,6 +1235,9 @@ export interface SpecStep { */ restart?: boolean; + /** Shut down the client and close it network connection. */ + shutdown?: boolean; + /** * Optional list of expected events. * If not provided, the test will fail if the step causes events to be raised. @@ -1190,6 +1317,11 @@ export interface SpecWatchEntity { removedTargets?: TargetId[]; } +export type SpecClientState = { + /** The visibility state of the browser tab running the client. */ + visibility?: VisibilityState; +}; + /** * [[, ...], , ...] * Note that the last parameter is really of type ...string (spread operator) @@ -1251,6 +1383,10 @@ export interface StateExpectation { watchStreamRequestCount?: number; /** Current documents in limbo. Verified in each step until overwritten. */ limboDocs?: string[]; + /** + * Whether the instance holds the primary lease. Used in multi-client tests. + */ + isPrimary?: boolean; /** * Current expected active targets. Verified in each step until overwritten. */ From 0d14759f322de47b0552275df0710abe4c6c314d Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Mon, 12 Feb 2018 16:57:49 -0800 Subject: [PATCH 02/18] Updating TODO message in LocalStore. --- packages/firestore/src/local/local_store.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/src/local/local_store.ts b/packages/firestore/src/local/local_store.ts index 8385bf8be63..214bf186c6f 100644 --- a/packages/firestore/src/local/local_store.ts +++ b/packages/firestore/src/local/local_store.ts @@ -401,7 +401,7 @@ export class LocalStore { getLastStreamToken(): Promise { return this.persistence.runTransaction( 'Get last stream token', - false, // todo: this does require owner lease + false, // TODO(multitab): This requires the owner lease txn => { return this.mutationQueue.getLastStreamToken(txn); } From 366f830bb98271f049efc0060e63b1cb1e48e99d Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Mon, 12 Feb 2018 16:59:40 -0800 Subject: [PATCH 03/18] Making Spec Tests consistent. --- .../test/unit/specs/persistence_spec.test.ts | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/packages/firestore/test/unit/specs/persistence_spec.test.ts b/packages/firestore/test/unit/specs/persistence_spec.test.ts index dc49f304cc7..5f4fdd97cc3 100644 --- a/packages/firestore/test/unit/specs/persistence_spec.test.ts +++ b/packages/firestore/test/unit/specs/persistence_spec.test.ts @@ -187,18 +187,20 @@ describeSpec('Persistence:', ['persistence'], () => { // This test simulates primary state handoff between two background tabs. // With all instances are in the background, the first active tab acquires // ownership. - return client(0) - .becomeHidden() - .expectPrimaryState(true) - .client(1) - .becomeHidden() - .expectPrimaryState(false) - .client(0) - .shutdown() - .expectPrimaryState(false) - .client(1) - .restart() - .expectPrimaryState(true); + return ( + client(0) + .becomeHidden() + .expectPrimaryState(true) + .client(1) + .becomeHidden() + .expectPrimaryState(false) + .client(0) + .shutdown() + .expectPrimaryState(false) + .client(1) + .restart() + .expectPrimaryState(true) + ); }); specTest('Foreground tab acquires primary lease', ['multi-client'], () => { From 75b08c8d9096dc5ff6683074529964570a4cd5e8 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Mon, 12 Feb 2018 17:00:02 -0800 Subject: [PATCH 04/18] [AUTOMATED]: Prettier Code Styling --- .../test/unit/specs/persistence_spec.test.ts | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/packages/firestore/test/unit/specs/persistence_spec.test.ts b/packages/firestore/test/unit/specs/persistence_spec.test.ts index 5f4fdd97cc3..dc49f304cc7 100644 --- a/packages/firestore/test/unit/specs/persistence_spec.test.ts +++ b/packages/firestore/test/unit/specs/persistence_spec.test.ts @@ -187,20 +187,18 @@ describeSpec('Persistence:', ['persistence'], () => { // This test simulates primary state handoff between two background tabs. // With all instances are in the background, the first active tab acquires // ownership. - return ( - client(0) - .becomeHidden() - .expectPrimaryState(true) - .client(1) - .becomeHidden() - .expectPrimaryState(false) - .client(0) - .shutdown() - .expectPrimaryState(false) - .client(1) - .restart() - .expectPrimaryState(true) - ); + return client(0) + .becomeHidden() + .expectPrimaryState(true) + .client(1) + .becomeHidden() + .expectPrimaryState(false) + .client(0) + .shutdown() + .expectPrimaryState(false) + .client(1) + .restart() + .expectPrimaryState(true); }); specTest('Foreground tab acquires primary lease', ['multi-client'], () => { From 64b0eb54826ce31e296d25ff61211960b8e94c0a Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Mon, 12 Feb 2018 18:30:06 -0800 Subject: [PATCH 05/18] Setting visibilityState on client init --- .../src/local/indexeddb_persistence.ts | 39 +++++----- .../test/unit/specs/spec_test_runner.ts | 76 ++++++++++--------- 2 files changed, 63 insertions(+), 52 deletions(-) diff --git a/packages/firestore/src/local/indexeddb_persistence.ts b/packages/firestore/src/local/indexeddb_persistence.ts index d09c59532a9..1b8a6e6ef78 100644 --- a/packages/firestore/src/local/indexeddb_persistence.ts +++ b/packages/firestore/src/local/indexeddb_persistence.ts @@ -109,8 +109,8 @@ export class IndexedDbPersistence implements Persistence { */ static MAIN_DATABASE = 'main'; - private readonly document: Document | null; - private readonly window: Window | null; + private readonly document: Document; + private readonly window: Window; private simpleDb: SimpleDb; private started: boolean; @@ -134,7 +134,7 @@ export class IndexedDbPersistence implements Persistence { private serializer: LocalSerializer; /** Our 'visibilitychange` listener if registered. */ - private documentVisibilityHandler: ((e: Event) => void) | null; + private documentVisibilityHandler: ((e?: Event) => void) | null; /** Callback for primary state notifications. */ private primaryStateListener = IndexedDbPersistence.EMPTY_PRIMARY_STATE_LISTENER; @@ -160,29 +160,32 @@ export class IndexedDbPersistence implements Persistence { return Promise.reject(this.persistenceError); } - if (this.document) { - this.documentVisibilityHandler = () => { - const inForeground = document.visibilityState === 'visible'; - if (inForeground !== this.inForeground) { - this.inForeground = inForeground; - this.refreshClientState(); - } - }; - - this.document.addEventListener( - 'visibilitychange', - this.documentVisibilityHandler - ); - } - assert(!this.started, 'IndexedDbPersistence double-started!'); this.started = true; + assert( + this.window !== null && this.document !== null, + "Expected 'window' and 'document' to be defined" + ); + + this.documentVisibilityHandler = () => { + const inForeground = this.document.visibilityState === 'visible'; + if (inForeground !== this.inForeground) { + this.inForeground = inForeground; + this.refreshClientState(); + } + }; + return SimpleDb.openOrCreate(this.dbName, SCHEMA_VERSION, createOrUpgradeDb) .then(db => { this.simpleDb = db; }) .then(() => { + this.document.addEventListener( + 'visibilitychange', + this.documentVisibilityHandler + ); + this.inForeground = this.document.visibilityState === 'visible'; this.refreshClientState(); this.scheduleClientStateRefresh(); this.attachWindowUnloadHook(); diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index 09de5d4c140..b35ee649144 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -348,7 +348,7 @@ abstract class TestRunner { constructor( private readonly name: string, - protected readonly platform: MockPlatform, + protected readonly platform: TestPlatform, config: SpecConfig ) { this.databaseInfo = new DatabaseInfo( @@ -812,7 +812,7 @@ abstract class TestRunner { private doApplyClientState(state: SpecClientState): Promise { if (state.visibility) { - this.platform.fireVisibilityEvent(state.visibility!); + this.platform.raiseVisibilityEvent(state.visibility!); } return Promise.resolve(); } @@ -1033,49 +1033,53 @@ class MemoryTestRunner extends TestRunner { } /** - * Implementation of `Platform` that allows mocking of `visibilitychange` - * events. - * */ -class MockPlatform implements Platform { - private visibilityState: VisibilityState = 'unloaded'; - private visibilityListener: EventListener | null = null; - private mockDocument: Document; - - constructor(private readonly basePlatform: Platform) { - this.initMockDocument(); - } - - initMockDocument() { - this.mockDocument = { - visibilityState: this.visibilityState, - addEventListener: (type: string, listener: EventListener) => { - assert( - type === 'visibilitychange', - "MockPlatform only supports events of type 'visibilitychange'" - ); - this.visibilityListener = listener; - }, - removeEventListener: (type: string, listener: EventListener) => { - if (listener === this.visibilityListener) { - this.visibilityListener = null; - } - } - } as any; // tslint:disable-line:no-any Not implementing the entire document interface + * `Document` mock that implements the `visibilitychange` API used by Firestore. + */ +class MockDocument { + private _visibilityState: VisibilityState = 'unloaded'; + private visibilityListener: EventListener | null; + + get visibilityState(): VisibilityState { + return this._visibilityState; } - fireVisibilityEvent(visibility: VisibilityState) { - this.visibilityState = visibility; + addEventListener(type: string, listener: EventListener) { + assert( + type === 'visibilitychange', + "MockDocument only supports events of type 'visibilitychange'" + ); + this.visibilityListener = listener; + } + + removeEventListener(type: string, listener: EventListener) { + if (listener === this.visibilityListener) { + this.visibilityListener = null; + } + } + + raiseVisibilityEvent(visibility: VisibilityState) { + this._visibilityState = visibility; if (this.visibilityListener) { this.visibilityListener(new Event('visibilitychange')); } } +} + +/** + * Implementation of `Platform` that allows mocking of `document` and `window`. + */ +class TestPlatform implements Platform { + private mockDocument = new MockDocument(); + + constructor(private readonly basePlatform: Platform) {} get window(): Window | null { return this.basePlatform.window; } get document(): Document { - return this.mockDocument; + // tslint:disable-next-line:no-any MockDocument doesn't support full Document interface. + return (this.mockDocument as any) as Document; } get base64Available(): boolean { @@ -1086,6 +1090,10 @@ class MockPlatform implements Platform { return this.basePlatform.emptyByteString; } + raiseVisibilityEvent(visibility: VisibilityState) { + this.mockDocument.raiseVisibilityEvent(visibility); + } + loadConnection(databaseInfo: DatabaseInfo): Promise { return this.basePlatform.loadConnection(databaseInfo); } @@ -1140,7 +1148,7 @@ export async function runSpec( steps: SpecStep[] ): Promise { console.log('Running spec: ' + name); - const platform = new MockPlatform(PlatformSupport.getPlatform()); + const platform = new TestPlatform(PlatformSupport.getPlatform()); let runners: TestRunner[] = []; for (let i = 0; i < config.numClients; ++i) { if (usePersistence) { From 38d23dbf3f6a74771301e600484c544446bcdfdf Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 15 Feb 2018 14:35:35 -0800 Subject: [PATCH 06/18] Addressing feedback --- .../firestore/src/core/firestore_client.ts | 9 +- packages/firestore/src/core/sync_engine.ts | 10 +- .../src/local/indexeddb_persistence.ts | 432 ++++++++++-------- .../firestore/src/local/indexeddb_schema.ts | 14 +- .../firestore/src/local/memory_persistence.ts | 5 +- packages/firestore/src/local/persistence.ts | 33 +- .../src/local/shared_client_state.ts | 5 +- packages/firestore/src/platform/platform.ts | 2 + packages/firestore/src/util/async_queue.ts | 3 +- .../unit/local/persistence_test_helpers.ts | 12 +- .../test/unit/specs/describe_spec.ts | 21 +- .../test/unit/specs/persistence_spec.test.ts | 12 +- .../firestore/test/unit/specs/spec_builder.ts | 13 + .../test/unit/specs/spec_test_runner.ts | 33 +- 14 files changed, 337 insertions(+), 267 deletions(-) diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index b9f2ca9346c..171088151f6 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -244,6 +244,7 @@ export class FirestoreClient { this.persistence = new IndexedDbPersistence( storagePrefix, this.platform, + this.asyncQueue, serializer ); return this.persistence.start(); @@ -256,7 +257,7 @@ export class FirestoreClient { */ private startMemoryPersistence(): Promise { this.garbageCollector = new EagerGarbageCollector(); - this.persistence = new MemoryPersistence(); + this.persistence = new MemoryPersistence(this.asyncQueue); return this.persistence.start(); } @@ -303,7 +304,6 @@ export class FirestoreClient { // Setup wiring between sync engine and remote store this.remoteStore.syncEngine = this.syncEngine; - this.persistence.setPrimaryStateListener(this.syncEngine); this.eventMgr = new EventManager(this.syncEngine); @@ -314,6 +314,11 @@ export class FirestoreClient { }) .then(() => { return this.remoteStore.start(); + }) + .then(() => { + this.persistence.setPrimaryStateListener(isPrimary => + this.syncEngine.applyPrimaryState(isPrimary) + ); }); } diff --git a/packages/firestore/src/core/sync_engine.ts b/packages/firestore/src/core/sync_engine.ts index 3f6cb626137..0b43e6f79f9 100644 --- a/packages/firestore/src/core/sync_engine.ts +++ b/packages/firestore/src/core/sync_engine.ts @@ -51,7 +51,6 @@ import { ViewDocumentChanges } from './view'; import { ViewSnapshot } from './view_snapshot'; -import { PrimaryStateListener } from '../local/persistence'; const LOG_TAG = 'SyncEngine'; @@ -103,7 +102,7 @@ class QueryView { * The SyncEngine’s methods should only ever be called by methods running in the * global async queue. */ -export class SyncEngine implements RemoteSyncer, PrimaryStateListener { +export class SyncEngine implements RemoteSyncer { private viewHandler: ViewHandler | null = null; private errorHandler: ErrorHandler | null = null; @@ -130,6 +129,7 @@ export class SyncEngine implements RemoteSyncer, PrimaryStateListener { private currentUser: User ) {} + // Only used for testing. get isPrimaryClient() { return this.isPrimary; } @@ -623,8 +623,8 @@ export class SyncEngine implements RemoteSyncer, PrimaryStateListener { }); } - applyPrimaryState(primaryClient: boolean): void { - // TODO(multitab): Apply the primary state internally on the AsyncQueue. - this.isPrimary = primaryClient; + applyPrimaryState(isPrimary: boolean): Promise { + this.isPrimary = isPrimary; + return Promise.resolve(); } } diff --git a/packages/firestore/src/local/indexeddb_persistence.ts b/packages/firestore/src/local/indexeddb_persistence.ts index 1b8a6e6ef78..57f631a7050 100644 --- a/packages/firestore/src/local/indexeddb_persistence.ts +++ b/packages/firestore/src/local/indexeddb_persistence.ts @@ -46,6 +46,8 @@ import { QueryCache } from './query_cache'; import { RemoteDocumentCache } from './remote_document_cache'; import { SimpleDb, SimpleDbStore, SimpleDbTransaction } from './simple_db'; import { Platform } from '../platform/platform'; +import { AsyncQueue, TimerId } from '../util/async_queue'; +import { ClientKey } from './shared_client_state'; const LOG_TAG = 'IndexedDbPersistence'; @@ -54,16 +56,22 @@ const LOG_TAG = 'IndexedDbPersistence'; * Client state and primary leases that are older than 5 seconds are ignored. */ const CLIENT_STATE_MAX_AGE_MS = 5000; - -/** Refresh interval for the client state. Currently set to four seconds. */ -const CLIENT_STATE_REFRESH_INTERVAL_MS = 4000; - +/** + * Maximum refresh interval for the primary lease. Used for extending a + * currently owned lease. + */ +const PRIMARY_LEASE_MAX_REFRESH_INTERVAL_MS = 4000; +/** + * Minimum interval for attemots to acquire the primary lease. Used when + * synchronizing client lease refreshes across all clients. + */ +const PRIMARY_LEASE_MIN_REFRESH_INTERVAL_MS = 4000; /** LocalStorage location to indicate a zombied primary key (see class comment). */ const ZOMBIE_PRIMARY_LOCALSTORAGE_SUFFIX = 'zombiedOwnerId'; -/** Error when the primary lease is required but not available. */ +/** User-facing error when the primary lease is required but not available. */ const PRIMARY_LEASE_LOST_ERROR_MSG = - 'The current tab is not the required state to perform the current ' + - 'operation. It might be necessary to refresh the browser tab.'; + 'The current tab is not in the required state to perform this operation. ' + + 'It might be necessary to refresh the browser tab.'; const UNSUPPORTED_PLATFORM_ERROR_MSG = 'This platform is either missing' + ' IndexedDB or is known to have an incomplete implementation. Offline' + @@ -97,12 +105,10 @@ const UNSUPPORTED_PLATFORM_ERROR_MSG = * the owner writes its ownerId to a "zombiedOwnerId" entry in LocalStorage * which acts as an indicator that another tab should go ahead and take the * owner lease immediately regardless of the current lease timestamp. + * + * TODO(multitab): Update this comment with multi-tab changes. */ export class IndexedDbPersistence implements Persistence { - private static EMPTY_PRIMARY_STATE_LISTENER: PrimaryStateListener = { - applyPrimaryState: () => {} - }; - /** * The name of the main (and currently only) IndexedDB database. this name is * appended to the prefix provided to the IndexedDbPersistence constructor. @@ -117,7 +123,7 @@ export class IndexedDbPersistence implements Persistence { private isPrimary = false; private dbName: string; private localStoragePrefix: string; - private clientKey: string = this.generateClientKey(); + private readonly clientKey = this.generateClientKey(); /** * Set to an Error object if we encounter an unrecoverable error. All further @@ -126,22 +132,23 @@ export class IndexedDbPersistence implements Persistence { private persistenceError: Error | null; /** The setInterval() handle tied to refreshing the owner lease. */ // tslint:disable-next-line:no-any setTimeout() type differs on browser / node - private clientStateRefreshHandler: any; + private clientStateRefreshHandle: any; /** Our window.unload handler, if registered. */ private windowUnloadHandler: (() => void) | null; private inForeground = false; private serializer: LocalSerializer; - /** Our 'visibilitychange` listener if registered. */ + /** Our 'visibilitychange' listener if registered. */ private documentVisibilityHandler: ((e?: Event) => void) | null; /** Callback for primary state notifications. */ - private primaryStateListener = IndexedDbPersistence.EMPTY_PRIMARY_STATE_LISTENER; + primaryStateListener: PrimaryStateListener = _ => Promise.resolve(); constructor( prefix: string, platform: Platform, + private readonly queue: AsyncQueue, serializer: JsonProtoSerializer ) { this.dbName = prefix + IndexedDbPersistence.MAIN_DATABASE; @@ -168,65 +175,128 @@ export class IndexedDbPersistence implements Persistence { "Expected 'window' and 'document' to be defined" ); - this.documentVisibilityHandler = () => { - const inForeground = this.document.visibilityState === 'visible'; - if (inForeground !== this.inForeground) { - this.inForeground = inForeground; - this.refreshClientState(); - } - }; - return SimpleDb.openOrCreate(this.dbName, SCHEMA_VERSION, createOrUpgradeDb) .then(db => { this.simpleDb = db; }) .then(() => { - this.document.addEventListener( - 'visibilitychange', - this.documentVisibilityHandler - ); - this.inForeground = this.document.visibilityState === 'visible'; - this.refreshClientState(); - this.scheduleClientStateRefresh(); + this.attachVisibilityHandler(); this.attachWindowUnloadHook(); + return this.schedulePrimaryLeaseRefreshes(0); }); } setPrimaryStateListener(primaryStateListener: PrimaryStateListener) { this.primaryStateListener = primaryStateListener; - primaryStateListener.applyPrimaryState(this.isPrimary); + primaryStateListener(this.isPrimary); } - tryBecomePrimary(): Promise { - return this.refreshClientState(); + /** + * Schedules a recurring timer to refresh or acquire the owner lease. + */ + private schedulePrimaryLeaseRefreshes(initialDelayMs: number): Promise { + return this.queue.enqueueAfterDelay( + TimerId.ClientStateRefresh, + initialDelayMs, + () => { + return this.tryBecomePrimary().then(currentPrimary => { + let nextDelayMs; + + if (this.isLocalClient(currentPrimary)) { + nextDelayMs = PRIMARY_LEASE_MAX_REFRESH_INTERVAL_MS; + } else if (currentPrimary !== null) { + nextDelayMs = Math.max( + PRIMARY_LEASE_MIN_REFRESH_INTERVAL_MS, + Date.now() - + (currentPrimary.leaseTimestampMs + CLIENT_STATE_MAX_AGE_MS) + + 1 + ); + } else { + nextDelayMs = PRIMARY_LEASE_MIN_REFRESH_INTERVAL_MS; + } + + this.schedulePrimaryLeaseRefreshes(nextDelayMs); + }); + } + ); } /** - * Updates the state of the client in IndexedDb. This operation may acquire - * the primary lease. - * - * @return Whether the client holds the primary lease. + * Attempts to obtain the primary lease for the local client. If successful, + * returns a `DbOwner` with the local client id. Otherwise, returns the + * current leaseholder or 'null' (if the primary lease should be obtained + * by another client running in the foreground). */ - private refreshClientState(): Promise { + private tryBecomePrimary(): Promise { return this.simpleDb - .runTransaction( - 'readwrite', - [DbOwner.store, DbClientMetadata.store], - txn => { - const metadataStore = clientMetadataStore(txn); - metadataStore.put( - new DbClientMetadata(this.clientKey, Date.now(), this.inForeground) - ); - return this.tryAcquirePrimaryLease(txn); - } - ) - .then(isPrimary => { - if (isPrimary !== this.isPrimary) { + .runTransaction('readwrite', ALL_STORES, txn => { + const metadataStore = clientMetadataStore(txn); + metadataStore.put( + new DbClientMetadata(this.clientKey, Date.now(), this.inForeground) + ); + return this.tryElectPrimaryCandidate(txn).next(electedPrimaryCandidate => { + if (this.isLocalClient(electedPrimaryCandidate)) { + return this.acquirePrimaryLease(txn); + } + return electedPrimaryCandidate; + }); + }) + .then(suggestedPrimaryClient => { + const isPrimary = this.isLocalClient(suggestedPrimaryClient); + if (this.isPrimary !== isPrimary) { this.isPrimary = isPrimary; - this.primaryStateListener.applyPrimaryState(this.isPrimary); + this.primaryStateListener(this.isPrimary); } - }) - .then(() => this.isPrimary); + return suggestedPrimaryClient; + }); + } + + /** Checks whether the `primary` is the local client. */ + private isLocalClient(primary:DbOwner|null): boolean { + return primary ? primary.ownerId === this.clientKey : false; + } + + /** + * Evaluate the state of all active instances and determine whether the local + * client can obtain the primary lease. Returns the leaseholder to use for + * pending operations, but does not actually acquire the lease. May return + * 'null' if there is no active leaseholder and another foreground client + * should become leaseholder instead. + * + * NOTE: To determine if the current leaseholder is zombied, this method reads + * from LocalStorage which could be mildly expensive. + */ + private tryElectPrimaryCandidate(txn): PersistencePromise { + const store = ownerStore(txn); + return store.get('owner').next(currentPrimary => { + if ( + currentPrimary !== null && + !this.isWithinMaxAge(currentPrimary.leaseTimestampMs) + ) { + currentPrimary = null; // primary lease has expired. + } else if ( + currentPrimary !== null && + currentPrimary.ownerId === this.getZombiedClientId() + ) { + currentPrimary = null; // primary's tab closed. + } + + if (currentPrimary) { + return currentPrimary; + } + + return this.canBecomePrimary(txn).next(canBecomePrimary => { + if (canBecomePrimary) { + log.debug( + LOG_TAG, + 'No valid primary. Acquiring primary lease.' + ); + return new DbOwner(this.clientKey, Date.now()); + } else { + return null; + } + }); + }); } shutdown(): Promise { @@ -234,6 +304,7 @@ export class IndexedDbPersistence implements Persistence { return Promise.resolve(); } this.started = false; + this.detachVisibilityHandler(); this.detachWindowUnloadHook(); this.stopClientStateRefreshes(); return this.releasePrimaryLease().then(() => { @@ -255,7 +326,7 @@ export class IndexedDbPersistence implements Persistence { runTransaction( action: string, - requireOwnerLease: boolean, + requirePrimaryLease: boolean, transactionOperation: ( transaction: PersistenceTransaction ) => PersistencePromise @@ -269,16 +340,43 @@ export class IndexedDbPersistence implements Persistence { // Do all transactions as readwrite against all object stores, since we // are the only reader/writer. return this.simpleDb.runTransaction('readwrite', ALL_STORES, txn => { - if (requireOwnerLease) { - return this.ensurePrimaryLease(txn).next(() => - transactionOperation(txn) - ); + if (requirePrimaryLease) { + // While we merely verify that we can obtain the lease at first, + // IndexedDb's transaction guarantees that we will be able to obtain + // the lease before we commit. We do this to not immediately lose our + // lease after long-running operations. + return this.tryElectPrimaryCandidate(txn) + .next(suggestedPrimary => { + if (!this.isLocalClient(suggestedPrimary)) { + // TODO(multitab): Handle this gracefully and transition back to + // secondary state. + throw new FirestoreError( + Code.FAILED_PRECONDITION, + PRIMARY_LEASE_LOST_ERROR_MSG + ); + } + return transactionOperation(txn); + }) + .next(result => { + return this.acquirePrimaryLease(txn).next(() => result); + }); } else { return transactionOperation(txn); } }); } + /** + * Obtains or extends the new primary lease for the current client. This + * method does not verify that the client is eligible for this lease. + */ + private acquirePrimaryLease(txn): PersistencePromise { + const newPrimary = new DbOwner(this.clientKey, Date.now()); + return ownerStore(txn) + .put('owner', newPrimary) + .next(() => newPrimary); + } + static isAvailable(): boolean { return SimpleDb.isAvailable(); } @@ -303,47 +401,11 @@ export class IndexedDbPersistence implements Persistence { return 'firestore/' + databaseInfo.persistenceKey + '/' + database + '/'; } - /** - * Tries to acquire and extend the primary lease. Returns whether the client - * holds the primary lease. - */ - private tryAcquirePrimaryLease( - txn: SimpleDbTransaction - ): PersistencePromise { - const ownerStore = txn.store(DbOwner.store); - return ownerStore.get('owner').next(dbOwner => { - const currentOwner = this.extractCurrentOwner(dbOwner); - if (currentOwner === null) { - return this.canBecomePrimary(txn).next(canBecomePrimary => { - if (canBecomePrimary) { - const newDbOwner = new DbOwner(this.clientKey, Date.now()); - log.debug( - LOG_TAG, - 'No valid primary. Acquiring primary lease. Current primary client:', - dbOwner, - 'New owner:', - newDbOwner - ); - return ownerStore.put('owner', newDbOwner).next(() => true); - } else { - return PersistencePromise.resolve(false); - } - }); - } else if (currentOwner === this.clientKey) { - // Refresh the primary lease - const newDbOwner = new DbOwner(this.clientKey, Date.now()); - return ownerStore.put('owner', newDbOwner).next(() => true); - } else { - return PersistencePromise.resolve(false); - } - }); - } - /** * Checks if this client is eligible for a primary lease based on its - * visibility state and the visibility state of all active instances. A - * client can obtain the primary lease if it is either in the foreground - * or if it and all other clients are in the background. + * visibility state and the visibility state of all active clients. A client + * can obtain the primary lease if it is either in the foreground or if it + * and all other clients are in the background. */ private canBecomePrimary( txn: SimpleDbTransaction @@ -356,60 +418,42 @@ export class IndexedDbPersistence implements Persistence { return clientMetadataStore(txn) .iterate((key, value, control) => { - if (this.clientKey !== value.clientKey) { - if ( - this.isRecentlyUpdated(value.updateTimeMs) && - value.inForeground - ) { + if (this.clientKey !== value.clientId) { + if (this.isWithinMaxAge(value.updateTimeMs) && value.inForeground) { canBecomePrimary = false; control.done(); } } }) - .next(() => canBecomePrimary); + .next(() => { + return canBecomePrimary; + }); } /** Checks the primary lease and removes it if we are the current primary. */ private releasePrimaryLease(): Promise { - // NOTE: Don't use this.runTransaction, since it acquires a lock on all - // object stores. - return this.simpleDb.runTransaction('readwrite', [DbOwner.store], txn => { - const store = txn.store(DbOwner.store); - return store.get('owner').next(dbOwner => { - if (dbOwner !== null && dbOwner.ownerId === this.clientKey) { - log.debug(LOG_TAG, 'Releasing owner lease.'); - this.primaryStateListener.applyPrimaryState(false); - return store.delete('owner'); - } else { - return PersistencePromise.resolve(); + return this.simpleDb + .runTransaction('readwrite', [DbOwner.store], txn => { + const store = txn.store(DbOwner.store); + return store.get('owner').next(dbOwner => { + if (dbOwner !== null && dbOwner.ownerId === this.clientKey) { + log.debug(LOG_TAG, 'Releasing primary lease.'); + return store.delete('owner'); + } else { + return PersistencePromise.resolve(); + } + }); + }) + .then(() => { + if (this.isPrimary) { + this.isPrimary = false; + return this.primaryStateListener(false); } }); - }); } - /** - * Checks the owner lease and returns a rejected promise if we are not the - * current owner. This should be included in every transaction to guard - * against losing the owner lease. - */ - private ensurePrimaryLease( - txn: SimpleDbTransaction - ): PersistencePromise { - return this.tryAcquirePrimaryLease(txn).next(owner => { - if (!owner) { - this.persistenceError = new FirestoreError( - Code.FAILED_PRECONDITION, - PRIMARY_LEASE_LOST_ERROR_MSG - ); - return PersistencePromise.reject(this.persistenceError); - } else { - return PersistencePromise.resolve(); - } - }); - } - - /** Verifies that that `updateTimeMs` is within CLIENT_STATE_MAX_AGE_MS. */ - private isRecentlyUpdated(updateTimeMs: number): boolean { + /** Verifies that `updateTimeMs` is within CLIENT_STATE_MAX_AGE_MS. */ + private isWithinMaxAge(updateTimeMs: number): boolean { const now = Date.now(); const minAcceptable = now - CLIENT_STATE_MAX_AGE_MS; const maxAcceptable = now; @@ -423,45 +467,39 @@ export class IndexedDbPersistence implements Persistence { return true; } - /** - * Returns true if the provided owner exists, has a recent timestamp, and - * isn't zombied. - * - * NOTE: To determine if the owner is zombied, this method reads from - * LocalStorage which could be mildly expensive. - */ - private extractCurrentOwner(dbOwner: DbOwner): string | null { - if (dbOwner === null) { - return null; // no owner. - } else if (!this.isRecentlyUpdated(dbOwner.leaseTimestampMs)) { - return null; // primary lease has expired. - } else if (dbOwner.ownerId === this.getZombiedOwnerId()) { - return null; // owner's tab closed. - } else { - return dbOwner.ownerId; + private stopClientStateRefreshes(): void { + if (this.clientStateRefreshHandle) { + clearInterval(this.clientStateRefreshHandle); + this.clientStateRefreshHandle = null; } } - /** - * Schedules a recurring timer to update the owner lease timestamp to prevent - * other tabs from taking the lease. - */ - private scheduleClientStateRefresh(): void { - // NOTE: This doesn't need to be scheduled on the async queue and doing so - // would increase the chances of us not refreshing on time if the queue is - // backed up for some reason. - - // TODO(mutlitab): To transition stale clients faster, we should synchronize - // the execution of this callback with primary client's lease expiry. - this.clientStateRefreshHandler = setInterval(() => { - this.refreshClientState(); - }, CLIENT_STATE_REFRESH_INTERVAL_MS); + private attachVisibilityHandler(): void { + this.documentVisibilityHandler = () => { + this.queue.enqueue(() => { + const inForeground = this.document.visibilityState === 'visible'; + if (inForeground !== this.inForeground) { + this.inForeground = inForeground; + return this.tryBecomePrimary(); + } else { + return Promise.resolve(); + } + }); + }; + + this.document.addEventListener( + 'visibilitychange', + this.documentVisibilityHandler + ); } - private stopClientStateRefreshes(): void { - if (this.clientStateRefreshHandler) { - clearInterval(this.clientStateRefreshHandler); - this.clientStateRefreshHandler = null; + private detachVisibilityHandler(): void { + if (this.documentVisibilityHandler) { + this.document.removeEventListener( + 'visibilitychange', + this.documentVisibilityHandler + ); + this.documentVisibilityHandler = null; } } @@ -476,13 +514,17 @@ export class IndexedDbPersistence implements Persistence { */ private attachWindowUnloadHook(): void { this.windowUnloadHandler = () => { + // Note: In theory, this should be scheduled on the AsyncQueue since it + // accesses internal state. We execute this code directly during shutdown + // to make sure it gets a chance to run. if (this.isPrimary) { - this.setZombiePrimaryKey(this.clientKey); + this.setZombiedClientId(this.clientKey); } - - // Attempt graceful shutdown (including releasing our owner lease), but - // there's no guarantee it will complete. - this.shutdown(); + this.queue.enqueue(() => { + // Attempt graceful shutdown (including releasing our owner lease), but + // there's no guarantee it will complete. + return this.shutdown(); + }); }; this.window.addEventListener('unload', this.windowUnloadHandler); } @@ -492,13 +534,6 @@ export class IndexedDbPersistence implements Persistence { this.window.removeEventListener('unload', this.windowUnloadHandler); this.windowUnloadHandler = null; } - if (this.documentVisibilityHandler) { - this.document.removeEventListener( - 'visibilitychange', - this.documentVisibilityHandler - ); - this.documentVisibilityHandler = null; - } } /** @@ -506,10 +541,10 @@ export class IndexedDbPersistence implements Persistence { * zombied due to their tab closing) from LocalStorage, or null if no such * record exists. */ - private getZombiedOwnerId(): string | null { + private getZombiedClientId(): string | null { try { const zombiedOwnerId = window.localStorage.getItem( - this.zombiedPrimaryLocalStorageKey() + this.zombiedClientLocalStorageKey() ); log.debug(LOG_TAG, 'Zombied ownerID from LocalStorage:', zombiedOwnerId); return zombiedOwnerId; @@ -521,16 +556,16 @@ export class IndexedDbPersistence implements Persistence { } /** - * Records a zombied primary key (a client that had its tab closed) in - * LocalStorage or, if passed null, deletes any recorded zombied owner. + * Records a zombied primary client (a primary client that had its tab closed) + * in LocalStorage or, if passed null, deletes any recorded zombied owner. */ - private setZombiePrimaryKey(zombieOwnerId: string | null) { + private setZombiedClientId(zombieOwnerId: string | null) { try { if (zombieOwnerId === null) { - window.localStorage.removeItem(this.zombiedPrimaryLocalStorageKey()); + window.localStorage.removeItem(this.zombiedClientLocalStorageKey()); } else { window.localStorage.setItem( - this.zombiedPrimaryLocalStorageKey(), + this.zombiedClientLocalStorageKey(), zombieOwnerId ); } @@ -540,16 +575,25 @@ export class IndexedDbPersistence implements Persistence { } } - private zombiedPrimaryLocalStorageKey(): string { + private zombiedClientLocalStorageKey(): string { return this.localStoragePrefix + ZOMBIE_PRIMARY_LOCALSTORAGE_SUFFIX; } - private generateClientKey(): string { + private generateClientKey(): ClientKey { // For convenience, just use an AutoId. return AutoId.newId(); } } +/** + * Helper to get a typed SimpleDbStore for the owner object store. + */ +function ownerStore( + txn: SimpleDbTransaction +): SimpleDbStore { + return txn.store(DbOwner.store); +} + /** * Helper to get a typed SimpleDbStore for the client metadata object store. */ diff --git a/packages/firestore/src/local/indexeddb_schema.ts b/packages/firestore/src/local/indexeddb_schema.ts index 5982338a4df..03105635e66 100644 --- a/packages/firestore/src/local/indexeddb_schema.ts +++ b/packages/firestore/src/local/indexeddb_schema.ts @@ -82,6 +82,8 @@ export type DbOwnerKey = 'owner'; * should regularly write an updated timestamp to prevent other tabs from * "stealing" ownership of the db. */ +// TODO(multitab): Rename this class to reflect the primary/secondary naming +// in the rest of the client. export class DbOwner { /** Name of the IndexedDb object store. */ static store = 'owner'; @@ -544,12 +546,12 @@ export class DbClientMetadata { /** Name of the IndexedDb object store. */ static store = 'clientMetadata'; - /** Keys are automatically assigned via the clientKey properties. */ - static key = ['clientKey']; + /** Keys are automatically assigned via the clientId properties. */ + static keyPath = ['clientId']; constructor( - /** The auto-generated client key assigned at client startup. */ - public clientKey: string, + /** The auto-generated client id assigned at client startup. */ + public clientId: string, /** The last time this state was updated. */ public updateTimeMs: number, /** Whether this client is running in a foreground tab. */ @@ -557,12 +559,12 @@ export class DbClientMetadata { ) {} } -/** Object keys in the 'clientMetadata' store are clientKey strings. */ +/** Object keys in the 'clientMetadata' store are clientId strings. */ export type DbClientMetadataKey = string; function createClientMetadataStore(db: IDBDatabase): void { db.createObjectStore(DbClientMetadata.store, { - keyPath: DbClientMetadata.key as KeyPath + keyPath: DbClientMetadata.keyPath as KeyPath }); } diff --git a/packages/firestore/src/local/memory_persistence.ts b/packages/firestore/src/local/memory_persistence.ts index 020cada4000..90b484cf669 100644 --- a/packages/firestore/src/local/memory_persistence.ts +++ b/packages/firestore/src/local/memory_persistence.ts @@ -30,6 +30,7 @@ import { import { PersistencePromise } from './persistence_promise'; import { QueryCache } from './query_cache'; import { RemoteDocumentCache } from './remote_document_cache'; +import { AsyncQueue } from '../util/async_queue'; const LOG_TAG = 'MemoryPersistence'; @@ -51,6 +52,8 @@ export class MemoryPersistence implements Persistence { private started = false; + constructor(private readonly queue: AsyncQueue) {} + start(): Promise { assert(!this.started, 'MemoryPersistence double-started!'); this.started = true; @@ -72,7 +75,7 @@ export class MemoryPersistence implements Persistence { setPrimaryStateListener(primaryStateListener: PrimaryStateListener) { // All clients using memory persistence act as primary. - primaryStateListener.applyPrimaryState(true); + this.queue.enqueue(() => primaryStateListener(true)); } getMutationQueue(user: User): MutationQueue { diff --git a/packages/firestore/src/local/persistence.ts b/packages/firestore/src/local/persistence.ts index d9624c9d013..f1c3ae16664 100644 --- a/packages/firestore/src/local/persistence.ts +++ b/packages/firestore/src/local/persistence.ts @@ -32,24 +32,16 @@ import { RemoteDocumentCache } from './remote_document_cache'; export interface PersistenceTransaction {} /** - * Interface that defines a callback for primary state notifications. This - * callback can be registered with the persistence layer to get notified - * when we transition from primary to secondary state and vice versa. + * Callback type for primary state notifications. This callback can be + * registered with the persistence layer to get notified when we transition from + * primary to secondary state and vice versa. * * Note: Instances can only toggle between Primary and Secondary state if - * IndexedDB persistence is enabled and multiple instances are active. If this + * IndexedDB persistence is enabled and multiple clients are active. If this * listener is registered with MemoryPersistence, the callback will be called * exactly once marking the current instance as Primary. */ -export interface PrimaryStateListener { - /** - * Transitions this instance between Primary and Secondary state. This - * callback can be called after the fact (for example when an instance loses - * its primary lease unexpectedly) and all necessary state transitions should - * be applied synchronously. - */ - applyPrimaryState(primaryClient: boolean): void; -} +export type PrimaryStateListener = (isPrimary: boolean) => Promise; /** * Persistence is the lowest-level shared interface to persistent storage in @@ -105,17 +97,6 @@ export interface Persistence { */ setPrimaryStateListener(primaryStateListener: PrimaryStateListener); - /** - * Execute the primary release check immediately (outside of its regular - * scheduled interval). Returns whether the instance is primary. - * - * TODO(multiab): Consider running this on the AsyncQueue and replacing with - * `runDelayedOperationsEarly`. - * - * Used for testing. - */ - tryBecomePrimary(): Promise; - /** * Returns a MutationQueue representing the persisted mutations for the * given user. @@ -160,7 +141,9 @@ export interface Persistence { * @param action A description of the action performed by this transaction, * used for logging. * @param requirePrimaryLease Whether this transaction can only be executed - * by the primary client only. + * by the primary client. If the primary lease cannot be acquired, the + * transactionOperation will not be run, and the returned promise will be + * rejected with a FAILED_PRECONDITION error. * @param transactionOperation The operation to run inside a transaction. * @return A promise that is resolved once the transaction completes. */ diff --git a/packages/firestore/src/local/shared_client_state.ts b/packages/firestore/src/local/shared_client_state.ts index e7fa2599280..bd9fb0b576c 100644 --- a/packages/firestore/src/local/shared_client_state.ts +++ b/packages/firestore/src/local/shared_client_state.ts @@ -25,6 +25,8 @@ import * as objUtils from '../util/obj'; const LOG_TAG = 'SharedClientState'; +// TODO(multitab): Change prefix of keys to "firestore_" to match IndexedDb. + // The format of the LocalStorage key that stores the client state is: // fs_clients__ const CLIENT_STATE_KEY_PREFIX = 'fs_clients'; @@ -32,6 +34,7 @@ const CLIENT_STATE_KEY_PREFIX = 'fs_clients'; /** * A randomly-generated key assigned to each Firestore instance at startup. */ +// TODO(multitab): Rename to ClientId. export type ClientKey = string; /** @@ -241,7 +244,7 @@ export class LocalClientState implements ClientState { /** * Converts this entry into a JSON-encoded format we can use for LocalStorage. - * Does not encode `clientKey` as it is part of the key in LocalStorage. + * Does not encode `clientId` as it is part of the key in LocalStorage. */ toLocalStorageJSON(): string { const data: ClientStateSchema = { diff --git a/packages/firestore/src/platform/platform.ts b/packages/firestore/src/platform/platform.ts index 39b12ff5feb..2dda1d34a54 100644 --- a/packages/firestore/src/platform/platform.ts +++ b/packages/firestore/src/platform/platform.ts @@ -27,6 +27,8 @@ import { AnyJs } from '../util/misc'; * * An implementation of this must be provided at compile time for the platform. */ +// TODO: Consider only exposing the APIs of 'document' and 'window' that we +// use in our client. export interface Platform { loadConnection(databaseInfo: DatabaseInfo): Promise; newSerializer(databaseId: DatabaseId): JsonProtoSerializer; diff --git a/packages/firestore/src/util/async_queue.ts b/packages/firestore/src/util/async_queue.ts index 453e720c373..adbb549aefa 100644 --- a/packages/firestore/src/util/async_queue.ts +++ b/packages/firestore/src/util/async_queue.ts @@ -32,7 +32,8 @@ export enum TimerId { ListenStreamIdle, ListenStreamConnection, WriteStreamIdle, - WriteStreamConnection + WriteStreamConnection, + ClientStateRefresh } /** diff --git a/packages/firestore/test/unit/local/persistence_test_helpers.ts b/packages/firestore/test/unit/local/persistence_test_helpers.ts index c6eb3bcca60..e910830a7b4 100644 --- a/packages/firestore/test/unit/local/persistence_test_helpers.ts +++ b/packages/firestore/test/unit/local/persistence_test_helpers.ts @@ -26,6 +26,7 @@ import { } from '../../../src/local/shared_client_state'; import { BatchId, TargetId } from '../../../src/core/types'; import { BrowserPlatform } from '../../../src/platform_browser/browser_platform'; +import { AsyncQueue } from '../../../src/util/async_queue'; /** The persistence prefix used for testing in IndexedBD and LocalStorage. */ export const TEST_PERSISTENCE_PREFIX = 'PersistenceTestHelpers'; @@ -50,14 +51,21 @@ export async function testIndexedDbPersistence(): Promise< useProto3Json: true }); const platform = new BrowserPlatform(); - const persistence = new IndexedDbPersistence(prefix, platform, serializer); + const queue = new AsyncQueue(); + const persistence = new IndexedDbPersistence( + prefix, + platform, + queue, + serializer + ); await persistence.start(); return persistence; } /** Creates and starts a MemoryPersistence instance for testing. */ export async function testMemoryPersistence(): Promise { - const persistence = new MemoryPersistence(); + const queue = new AsyncQueue(); + const persistence = new MemoryPersistence(queue); await persistence.start(); return persistence; } diff --git a/packages/firestore/test/unit/specs/describe_spec.ts b/packages/firestore/test/unit/specs/describe_spec.ts index 667bf9c61b9..fb86a18e9dd 100644 --- a/packages/firestore/test/unit/specs/describe_spec.ts +++ b/packages/firestore/test/unit/specs/describe_spec.ts @@ -26,7 +26,7 @@ import { SpecStep } from './spec_test_runner'; const EXCLUSIVE_TAG = 'exclusive'; // Persistence-related tests. const PERSISTENCE_TAG = 'persistence'; -// Multi-client related tests. Requires persistence. +// Multi-client related tests (which imply persistence). const MULTI_CLIENT_TAG = 'multi-client'; // Explicit per-platform disable flags. const NO_WEB_TAG = 'no-web'; @@ -41,14 +41,12 @@ const KNOWN_TAGS = [ NO_IOS_TAG ]; -// Filters out multi-client tests if persistence is not enabled. -const MULTI_CLIENT_TEST_FILTER = ( - tags: string[], - persistenceEnabled: boolean -) => tags.indexOf(MULTI_CLIENT_TAG) !== -1 && persistenceEnabled; - -const WEB_SPEC_TEST_FILTER = (tags: string[]) => - tags.indexOf(NO_WEB_TAG) === -1; +const WEB_SPEC_TEST_FILTER = (tags: string[], persistenceEnabled: boolean) => { + return ( + tags.indexOf(NO_WEB_TAG) === -1 && + (tags.indexOf(MULTI_CLIENT_TAG) === -1 || persistenceEnabled) + ); +}; // The format of one describeSpec written to a JSON file. interface SpecOutputFormat { @@ -80,10 +78,7 @@ export function setSpecJSONHandler(writer: (json: string) => void) { /** Gets the test runner based on the specified tags. */ function getTestRunner(tags, persistenceEnabled): Function { - if ( - !MULTI_CLIENT_TEST_FILTER(tags, persistenceEnabled) || - !WEB_SPEC_TEST_FILTER(tags) - ) { + if (!WEB_SPEC_TEST_FILTER(tags, persistenceEnabled)) { return it.skip; } else if (tags.indexOf(EXCLUSIVE_TAG) >= 0) { return it.only; diff --git a/packages/firestore/test/unit/specs/persistence_spec.test.ts b/packages/firestore/test/unit/specs/persistence_spec.test.ts index dc49f304cc7..a4eb4df7438 100644 --- a/packages/firestore/test/unit/specs/persistence_spec.test.ts +++ b/packages/firestore/test/unit/specs/persistence_spec.test.ts @@ -185,7 +185,7 @@ describeSpec('Persistence:', ['persistence'], () => { specTest('Single tab acquires primary lease', ['multi-client'], () => { // This test simulates primary state handoff between two background tabs. - // With all instances are in the background, the first active tab acquires + // With all instances in the background, the first active tab acquires // ownership. return client(0) .becomeHidden() @@ -197,7 +197,7 @@ describeSpec('Persistence:', ['persistence'], () => { .shutdown() .expectPrimaryState(false) .client(1) - .restart() + .tryAcquirePrimaryLease() .expectPrimaryState(true); }); @@ -219,12 +219,12 @@ describeSpec('Persistence:', ['persistence'], () => { .shutdown() .expectPrimaryState(false) .client(1) - // Restart client 1. This client is in the background and doesn't grab - // the primary lease as client 2 is in the foreground. - .restart() + // Client 1 is in the background and doesn't grab the primary lease as + // client 2 is in the foreground. + .tryAcquirePrimaryLease() .expectPrimaryState(false) .client(2) - .restart() + .tryAcquirePrimaryLease() .expectPrimaryState(true) ); }); diff --git a/packages/firestore/test/unit/specs/spec_builder.ts b/packages/firestore/test/unit/specs/spec_builder.ts index e4f32f6dfe1..34d962dd020 100644 --- a/packages/firestore/test/unit/specs/spec_builder.ts +++ b/packages/firestore/test/unit/specs/spec_builder.ts @@ -252,6 +252,14 @@ export class SpecBuilder { return this; } + tryAcquirePrimaryLease(): SpecBuilder { + this.nextStep(); + this.currentStep = { + acquirePrimaryLease: true + }; + return this; + } + shutdown(): SpecBuilder { this.nextStep(); this.currentStep = { @@ -782,6 +790,11 @@ export class MultiClientSpecBuilder extends SpecBuilder { return this; } + tryAcquirePrimaryLease(): MultiClientSpecBuilder { + super.tryAcquirePrimaryLease(); + return this; + } + expectActiveTargets(...targets): MultiClientSpecBuilder { super.expectActiveTargets(...targets); return this; diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index b1536106290..3fd98be3e99 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -320,10 +320,11 @@ interface OutstandingWrite { } abstract class TestRunner { + protected queue: AsyncQueue; + private connection: MockConnection; private eventManager: EventManager; private syncEngine: SyncEngine; - private queue: AsyncQueue; private eventList: QueryEvent[] = []; private outstandingWrites: OutstandingWrite[] = []; @@ -357,6 +358,7 @@ abstract class TestRunner { 'host', false ); + this.queue = new AsyncQueue(); this.serializer = new JsonProtoSerializer(this.databaseInfo.databaseId, { useProto3Json: true }); @@ -379,7 +381,6 @@ abstract class TestRunner { garbageCollector ); - this.queue = new AsyncQueue(); this.connection = new MockConnection(this.queue); this.datastore = new Datastore( this.queue, @@ -403,7 +404,6 @@ abstract class TestRunner { this.user ); - this.persistence.setPrimaryStateListener(this.syncEngine); // Setup wiring between sync engine and remote store this.remoteStore.syncEngine = this.syncEngine; @@ -425,6 +425,10 @@ abstract class TestRunner { await this.persistence.start(); await this.localStore.start(); await this.remoteStore.start(); + + this.persistence.setPrimaryStateListener(isPrimary => + this.syncEngine.applyPrimaryState(isPrimary) + ); } async shutdown(): Promise { @@ -474,11 +478,11 @@ abstract class TestRunner { return step.enableNetwork! ? this.doEnableNetwork() : this.doDisableNetwork(); + } else if ('acquirePrimaryLease' in step) { + return this.doAcquirePrimaryLease(); } else if ('restart' in step) { - assert(step.restart!, 'Restart cannot be false'); return this.doRestart(); } else if ('shutdown' in step) { - assert(step.shutdown!, 'Shutdown cannot be false'); return this.doShutdown(); } else if ('applyClientState' in step) { return this.doApplyClientState(step.applyClientState!); @@ -788,6 +792,12 @@ abstract class TestRunner { await this.remoteStore.enableNetwork(); } + private async doAcquirePrimaryLease(): Promise { + expect(this.queue.containsDelayedOperation(TimerId.ClientStateRefresh)).to + .be.true; + return this.queue.runDelayedOperationsEarly(TimerId.ClientStateRefresh); + } + private async doShutdown(): Promise { await this.remoteStore.shutdown(); await this.persistence.shutdown(); @@ -806,8 +816,6 @@ abstract class TestRunner { await this.localStore.start(); await this.remoteStore.start(); }); - - await this.persistence.tryBecomePrimary(); } private doApplyClientState(state: SpecClientState): Promise { @@ -1023,7 +1031,7 @@ abstract class TestRunner { class MemoryTestRunner extends TestRunner { protected getPersistence(serializer: JsonProtoSerializer): Persistence { - return new MemoryPersistence(); + return new MemoryPersistence(this.queue); } static destroyPersistence(): Promise { @@ -1125,6 +1133,7 @@ class IndexedDbTestRunner extends TestRunner { return new IndexedDbPersistence( IndexedDbTestRunner.TEST_DB_NAME, this.platform, + this.queue, serializer ); } @@ -1236,16 +1245,18 @@ export interface SpecStep { /** Change to a new active user (specified by uid or null for anonymous). */ changeUser?: string | null; + /** Attempt to acquire the primary lease. */ + acquirePrimaryLease?: true; + /** * Restarts the SyncEngine from scratch, except re-uses persistence and auth * components. This allows you to queue writes, get documents into cache, * etc. and then simulate an app restart. */ - restart?: boolean; + restart?: true; /** Shut down the client and close it network connection. */ - shutdown?: boolean; - + shutdown?: true; /** * Optional list of expected events. * If not provided, the test will fail if the step causes events to be raised. From 1f78076156c06d30d261230bd22da29ce7bede97 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 15 Feb 2018 17:52:54 -0800 Subject: [PATCH 07/18] [AUTOMATED]: Prettier Code Styling --- .../src/local/indexeddb_persistence.ts | 37 +++++++++---------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/packages/firestore/src/local/indexeddb_persistence.ts b/packages/firestore/src/local/indexeddb_persistence.ts index 57f631a7050..ee529d8c5f8 100644 --- a/packages/firestore/src/local/indexeddb_persistence.ts +++ b/packages/firestore/src/local/indexeddb_persistence.ts @@ -203,17 +203,17 @@ export class IndexedDbPersistence implements Persistence { let nextDelayMs; if (this.isLocalClient(currentPrimary)) { - nextDelayMs = PRIMARY_LEASE_MAX_REFRESH_INTERVAL_MS; + nextDelayMs = PRIMARY_LEASE_MAX_REFRESH_INTERVAL_MS; } else if (currentPrimary !== null) { - nextDelayMs = Math.max( - PRIMARY_LEASE_MIN_REFRESH_INTERVAL_MS, - Date.now() - - (currentPrimary.leaseTimestampMs + CLIENT_STATE_MAX_AGE_MS) + - 1 - ); + nextDelayMs = Math.max( + PRIMARY_LEASE_MIN_REFRESH_INTERVAL_MS, + Date.now() - + (currentPrimary.leaseTimestampMs + CLIENT_STATE_MAX_AGE_MS) + + 1 + ); } else { - nextDelayMs = PRIMARY_LEASE_MIN_REFRESH_INTERVAL_MS; - } + nextDelayMs = PRIMARY_LEASE_MIN_REFRESH_INTERVAL_MS; + } this.schedulePrimaryLeaseRefreshes(nextDelayMs); }); @@ -234,12 +234,14 @@ export class IndexedDbPersistence implements Persistence { metadataStore.put( new DbClientMetadata(this.clientKey, Date.now(), this.inForeground) ); - return this.tryElectPrimaryCandidate(txn).next(electedPrimaryCandidate => { - if (this.isLocalClient(electedPrimaryCandidate)) { - return this.acquirePrimaryLease(txn); + return this.tryElectPrimaryCandidate(txn).next( + electedPrimaryCandidate => { + if (this.isLocalClient(electedPrimaryCandidate)) { + return this.acquirePrimaryLease(txn); + } + return electedPrimaryCandidate; } - return electedPrimaryCandidate; - }); + ); }) .then(suggestedPrimaryClient => { const isPrimary = this.isLocalClient(suggestedPrimaryClient); @@ -252,7 +254,7 @@ export class IndexedDbPersistence implements Persistence { } /** Checks whether the `primary` is the local client. */ - private isLocalClient(primary:DbOwner|null): boolean { + private isLocalClient(primary: DbOwner | null): boolean { return primary ? primary.ownerId === this.clientKey : false; } @@ -287,10 +289,7 @@ export class IndexedDbPersistence implements Persistence { return this.canBecomePrimary(txn).next(canBecomePrimary => { if (canBecomePrimary) { - log.debug( - LOG_TAG, - 'No valid primary. Acquiring primary lease.' - ); + log.debug(LOG_TAG, 'No valid primary. Acquiring primary lease.'); return new DbOwner(this.clientKey, Date.now()); } else { return null; From 3116e70e4844369a634c99fd0f7d0824e62f2c13 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 15 Feb 2018 18:11:25 -0800 Subject: [PATCH 08/18] Cleanup --- .../src/local/indexeddb_persistence.ts | 35 ++++++++++--------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/packages/firestore/src/local/indexeddb_persistence.ts b/packages/firestore/src/local/indexeddb_persistence.ts index ee529d8c5f8..46a3dee4d59 100644 --- a/packages/firestore/src/local/indexeddb_persistence.ts +++ b/packages/firestore/src/local/indexeddb_persistence.ts @@ -63,7 +63,7 @@ const CLIENT_STATE_MAX_AGE_MS = 5000; const PRIMARY_LEASE_MAX_REFRESH_INTERVAL_MS = 4000; /** * Minimum interval for attemots to acquire the primary lease. Used when - * synchronizing client lease refreshes across all clients. + * synchronizing client lease refreshes across clients. */ const PRIMARY_LEASE_MIN_REFRESH_INTERVAL_MS = 4000; /** LocalStorage location to indicate a zombied primary key (see class comment). */ @@ -130,9 +130,9 @@ export class IndexedDbPersistence implements Persistence { * transactions will be failed with this error. */ private persistenceError: Error | null; - /** The setInterval() handle tied to refreshing the owner lease. */ + /** The setInterval() handle tied to refreshing the primary lease. */ // tslint:disable-next-line:no-any setTimeout() type differs on browser / node - private clientStateRefreshHandle: any; + private primaryLeaseRefreshHandle: any; /** Our window.unload handler, if registered. */ private windowUnloadHandler: (() => void) | null; private inForeground = false; @@ -143,7 +143,7 @@ export class IndexedDbPersistence implements Persistence { private documentVisibilityHandler: ((e?: Event) => void) | null; /** Callback for primary state notifications. */ - primaryStateListener: PrimaryStateListener = _ => Promise.resolve(); + private primaryStateListener: PrimaryStateListener = _ => Promise.resolve(); constructor( prefix: string, @@ -192,7 +192,7 @@ export class IndexedDbPersistence implements Persistence { } /** - * Schedules a recurring timer to refresh or acquire the owner lease. + * Schedules a recurring timer to refresh or acquire the primary lease. */ private schedulePrimaryLeaseRefreshes(initialDelayMs: number): Promise { return this.queue.enqueueAfterDelay( @@ -243,19 +243,19 @@ export class IndexedDbPersistence implements Persistence { } ); }) - .then(suggestedPrimaryClient => { - const isPrimary = this.isLocalClient(suggestedPrimaryClient); + .then(electedPrimaryCandidate => { + const isPrimary = this.isLocalClient(electedPrimaryCandidate); if (this.isPrimary !== isPrimary) { this.isPrimary = isPrimary; this.primaryStateListener(this.isPrimary); } - return suggestedPrimaryClient; + return electedPrimaryCandidate; }); } - /** Checks whether the `primary` is the local client. */ - private isLocalClient(primary: DbOwner | null): boolean { - return primary ? primary.ownerId === this.clientKey : false; + /** Checks whether `client` is the local client. */ + private isLocalClient(client: DbOwner | null): boolean { + return client ? client.ownerId === this.clientKey : false; } /** @@ -340,7 +340,8 @@ export class IndexedDbPersistence implements Persistence { // are the only reader/writer. return this.simpleDb.runTransaction('readwrite', ALL_STORES, txn => { if (requirePrimaryLease) { - // While we merely verify that we can obtain the lease at first, + assert(this.isPrimary, 'Operation requires the primary lease'); + // While we merely verify that we still retain the lease at first, // IndexedDb's transaction guarantees that we will be able to obtain // the lease before we commit. We do this to not immediately lose our // lease after long-running operations. @@ -434,8 +435,8 @@ export class IndexedDbPersistence implements Persistence { return this.simpleDb .runTransaction('readwrite', [DbOwner.store], txn => { const store = txn.store(DbOwner.store); - return store.get('owner').next(dbOwner => { - if (dbOwner !== null && dbOwner.ownerId === this.clientKey) { + return store.get('owner').next(primaryClient => { + if (this.isLocalClient(primaryClient)) { log.debug(LOG_TAG, 'Releasing primary lease.'); return store.delete('owner'); } else { @@ -467,9 +468,9 @@ export class IndexedDbPersistence implements Persistence { } private stopClientStateRefreshes(): void { - if (this.clientStateRefreshHandle) { - clearInterval(this.clientStateRefreshHandle); - this.clientStateRefreshHandle = null; + if (this.primaryLeaseRefreshHandle) { + clearInterval(this.primaryLeaseRefreshHandle); + this.primaryLeaseRefreshHandle = null; } } From ea07c8edb02ce686bc245fc22468bb16bc339287 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Tue, 27 Feb 2018 05:41:03 -0800 Subject: [PATCH 09/18] Addressing review feedback. --- .../firestore/src/core/firestore_client.ts | 2 + .../src/local/indexeddb_persistence.ts | 379 +++++++++--------- .../firestore/src/local/memory_persistence.ts | 5 - packages/firestore/src/util/async_queue.ts | 2 +- .../test/unit/specs/persistence_spec.test.ts | 2 - .../test/unit/specs/spec_test_runner.ts | 8 +- 6 files changed, 207 insertions(+), 191 deletions(-) diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index 171088151f6..fe65f132b89 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -316,6 +316,8 @@ export class FirestoreClient { return this.remoteStore.start(); }) .then(() => { + // NOTE: This will immediately call the listener, so we make sure to + // set it after localStore / remoteStore are started. this.persistence.setPrimaryStateListener(isPrimary => this.syncEngine.applyPrimaryState(isPrimary) ); diff --git a/packages/firestore/src/local/indexeddb_persistence.ts b/packages/firestore/src/local/indexeddb_persistence.ts index 46a3dee4d59..b3b0ef42896 100644 --- a/packages/firestore/src/local/indexeddb_persistence.ts +++ b/packages/firestore/src/local/indexeddb_persistence.ts @@ -52,22 +52,29 @@ import { ClientKey } from './shared_client_state'; const LOG_TAG = 'IndexedDbPersistence'; /** - * Oldest acceptable age in milliseconds for client states read from IndexedDB. - * Client state and primary leases that are older than 5 seconds are ignored. + * Oldest acceptable age in milliseconds for client metadata read from + * IndexedDB. Client metadata and primary leases that are older than 5 seconds + * are ignored. */ -const CLIENT_STATE_MAX_AGE_MS = 5000; +const CLIENT_METADATA_MAX_AGE_MS = 5000; /** - * Maximum refresh interval for the primary lease. Used for extending a - * currently owned lease. + * The maximum interval after which clients will update their metadata, + * including refreshing the primary lease if held or potentially trying to + * acquire it if not held. + * + * Primary clients may opportunistically refresh their metadata earlier + * if they're already performing an IndexedDB operation. Secondary clients + * may also temporarily use a shorter refresh interval to synchronize their + * state refreshes with the primary lease holder. */ -const PRIMARY_LEASE_MAX_REFRESH_INTERVAL_MS = 4000; +const CLIENT_METADATA_REFRESH_INTERVAL_MS = 4000; /** - * Minimum interval for attemots to acquire the primary lease. Used when - * synchronizing client lease refreshes across clients. + * Minimum interval for all state refreshes. Used when synchronizing the primary + * lease refresh with an existing lease that is about to expire. */ -const PRIMARY_LEASE_MIN_REFRESH_INTERVAL_MS = 4000; -/** LocalStorage location to indicate a zombied primary key (see class comment). */ -const ZOMBIE_PRIMARY_LOCALSTORAGE_SUFFIX = 'zombiedOwnerId'; +const MINIMUM_REFRESH_INTERVAL_MS = 1000; +/** LocalStorage location to indicate a zombied client id (see class comment). */ +const ZOMBIED_PRIMARY_LOCALSTORAGE_SUFFIX = 'zombiedClientId'; /** User-facing error when the primary lease is required but not available. */ const PRIMARY_LEASE_LOST_ERROR_MSG = 'The current tab is not in the required state to perform this operation. ' + @@ -102,7 +109,7 @@ const UNSUPPORTED_PLATFORM_ERROR_MSG = * a refreshed tab is able to immediately re-acquire the owner lease). * Unfortunately, IndexedDB cannot be reliably used in window.unload since it is * an asynchronous API. So in addition to attempting to give up the lease, - * the owner writes its ownerId to a "zombiedOwnerId" entry in LocalStorage + * the owner writes its ownerId to a "zombiedClientId" entry in LocalStorage * which acts as an indicator that another tab should go ahead and take the * owner lease immediately regardless of the current lease timestamp. * @@ -130,9 +137,6 @@ export class IndexedDbPersistence implements Persistence { * transactions will be failed with this error. */ private persistenceError: Error | null; - /** The setInterval() handle tied to refreshing the primary lease. */ - // tslint:disable-next-line:no-any setTimeout() type differs on browser / node - private primaryLeaseRefreshHandle: any; /** Our window.unload handler, if registered. */ private windowUnloadHandler: (() => void) | null; private inForeground = false; @@ -142,7 +146,7 @@ export class IndexedDbPersistence implements Persistence { /** Our 'visibilitychange' listener if registered. */ private documentVisibilityHandler: ((e?: Event) => void) | null; - /** Callback for primary state notifications. */ + /** A listener to notify on primary state changes. */ private primaryStateListener: PrimaryStateListener = _ => Promise.resolve(); constructor( @@ -182,7 +186,9 @@ export class IndexedDbPersistence implements Persistence { .then(() => { this.attachVisibilityHandler(); this.attachWindowUnloadHook(); - return this.schedulePrimaryLeaseRefreshes(0); + return this.updateClientMetadataAndTryBecomePrimary().then(leaseTtl => + this.scheduleClientMetadataAndPrimaryLeaseRefreshes(leaseTtl) + ); }); } @@ -192,65 +198,85 @@ export class IndexedDbPersistence implements Persistence { } /** - * Schedules a recurring timer to refresh or acquire the primary lease. + * Updates the client metadata in IndexedDb and attempts to either obtain or + * extend the primary lease for the local client. Asynchronously notifies the + * primary state listener if the client either newly obtained or released its + * primary lease. + * + * @return {Promise} A Promise that resolves with the interval in ms + * after which the metadata and primary lease needs to be refreshed. */ - private schedulePrimaryLeaseRefreshes(initialDelayMs: number): Promise { - return this.queue.enqueueAfterDelay( - TimerId.ClientStateRefresh, - initialDelayMs, - () => { - return this.tryBecomePrimary().then(currentPrimary => { - let nextDelayMs; - - if (this.isLocalClient(currentPrimary)) { - nextDelayMs = PRIMARY_LEASE_MAX_REFRESH_INTERVAL_MS; - } else if (currentPrimary !== null) { - nextDelayMs = Math.max( - PRIMARY_LEASE_MIN_REFRESH_INTERVAL_MS, - Date.now() - - (currentPrimary.leaseTimestampMs + CLIENT_STATE_MAX_AGE_MS) + - 1 - ); - } else { - nextDelayMs = PRIMARY_LEASE_MIN_REFRESH_INTERVAL_MS; - } + private updateClientMetadataAndTryBecomePrimary(): Promise { + return this.simpleDb.runTransaction('readwrite', ALL_STORES, txn => { + const metadataStore = clientMetadataStore(txn); + metadataStore.put( + new DbClientMetadata(this.clientKey, Date.now(), this.inForeground) + ); - this.schedulePrimaryLeaseRefreshes(nextDelayMs); - }); - } - ); - } + return this.getCurrentPrimary(txn).next(currentPrimary => { + return this.canActAsPrimary(currentPrimary, txn).next( + canActAsPrimary => { + if (canActAsPrimary !== this.isPrimary) { + this.isPrimary = canActAsPrimary; + log.debug( + LOG_TAG, + `Primary lease changed. Current client ${ + this.isPrimary ? 'acquired' : 'released' + } its primary lease.` + ); + this.queue.enqueue(() => + this.primaryStateListener(this.isPrimary) + ); + } - /** - * Attempts to obtain the primary lease for the local client. If successful, - * returns a `DbOwner` with the local client id. Otherwise, returns the - * current leaseholder or 'null' (if the primary lease should be obtained - * by another client running in the foreground). - */ - private tryBecomePrimary(): Promise { - return this.simpleDb - .runTransaction('readwrite', ALL_STORES, txn => { - const metadataStore = clientMetadataStore(txn); - metadataStore.put( - new DbClientMetadata(this.clientKey, Date.now(), this.inForeground) - ); - return this.tryElectPrimaryCandidate(txn).next( - electedPrimaryCandidate => { - if (this.isLocalClient(electedPrimaryCandidate)) { - return this.acquirePrimaryLease(txn); + if (currentPrimary != null) { + if (this.isPrimary) { + // If we are the primary lease holder, we refresh the client + // metadata and the primary lease after the default interval. + return this.acquireOrExtendPrimaryLease(txn).next( + () => CLIENT_METADATA_REFRESH_INTERVAL_MS + ); + } else { + // If another client currently holds the lease, we use a custom + // refresh interval and schedule a lease refresh immediately after + // the current lease is expected to expire. + const remainingLifetimeMs = + Date.now() - + (currentPrimary.leaseTimestampMs + + CLIENT_METADATA_MAX_AGE_MS); + return Math.min( + MINIMUM_REFRESH_INTERVAL_MS, + remainingLifetimeMs + 1 + ); + } + } else { + // If there is no current leaseholder, but we are not ourselves + // eligible to directly obtain the lease (based on our foreground + // state), we try again after the minimum refresh interval. + return MINIMUM_REFRESH_INTERVAL_MS; } - return electedPrimaryCandidate; } ); - }) - .then(electedPrimaryCandidate => { - const isPrimary = this.isLocalClient(electedPrimaryCandidate); - if (this.isPrimary !== isPrimary) { - this.isPrimary = isPrimary; - this.primaryStateListener(this.isPrimary); - } - return electedPrimaryCandidate; }); + }); + } + + /** + * Schedules a recurring timer to update the client metadata and to either + * extend or acquire the primary lease if the client is eligible. + * + * @param delayMs The delay to use for the first refresh. Subsequent refreshes + * may use a different delay based on the primary leaseholder's refresh + * interval. + */ + private scheduleClientMetadataAndPrimaryLeaseRefreshes( + delayMs: number + ): void { + this.queue.enqueueAfterDelay(TimerId.ClientMetadataRefresh, delayMs, () => { + return this.updateClientMetadataAndTryBecomePrimary().then(leaseTtl => + this.scheduleClientMetadataAndPrimaryLeaseRefreshes(leaseTtl) + ); + }); } /** Checks whether `client` is the local client. */ @@ -258,44 +284,75 @@ export class IndexedDbPersistence implements Persistence { return client ? client.ownerId === this.clientKey : false; } - /** - * Evaluate the state of all active instances and determine whether the local - * client can obtain the primary lease. Returns the leaseholder to use for - * pending operations, but does not actually acquire the lease. May return - * 'null' if there is no active leaseholder and another foreground client - * should become leaseholder instead. - * - * NOTE: To determine if the current leaseholder is zombied, this method reads - * from LocalStorage which could be mildly expensive. - */ - private tryElectPrimaryCandidate(txn): PersistencePromise { + private getCurrentPrimary( + txn: SimpleDbTransaction + ): PersistencePromise { const store = ownerStore(txn); return store.get('owner').next(currentPrimary => { if ( currentPrimary !== null && - !this.isWithinMaxAge(currentPrimary.leaseTimestampMs) + this.isWithinMaxAge(currentPrimary.leaseTimestampMs) && + currentPrimary.ownerId !== this.getZombiedClientId() ) { - currentPrimary = null; // primary lease has expired. - } else if ( - currentPrimary !== null && - currentPrimary.ownerId === this.getZombiedClientId() - ) { - currentPrimary = null; // primary's tab closed. - } - - if (currentPrimary) { return currentPrimary; + } else { + return null; } + }); + } - return this.canBecomePrimary(txn).next(canBecomePrimary => { - if (canBecomePrimary) { - log.debug(LOG_TAG, 'No valid primary. Acquiring primary lease.'); - return new DbOwner(this.clientKey, Date.now()); - } else { - return null; + /** + * Evaluate the state of all active instances and determine whether the local + * client is or can act as the holder of the primary lease. Returns whether + * the client is eligible for the lease, but does not actually acquire it. + * May return 'false' even if there is no active leaseholder when a foreground + * client should become leaseholder instead. + */ + private canActAsPrimary( + currentPrimary: DbOwner | null, + txn: SimpleDbTransaction + ): PersistencePromise { + if (currentPrimary !== null) { + return PersistencePromise.resolve(this.isLocalClient(currentPrimary)); + } + + // Check if this client is eligible for a primary lease based on its + // visibility state and the visibility state of all active clients. A + // client can obtain the primary lease if it is either in the foreground + // or if this client and all other clients are in the background. + return PersistencePromise.resolve(true) + .next(() => { + if (this.inForeground) { + return true; } + + let canActAsPrimary = true; + + return clientMetadataStore(txn) + .iterate((key, value, control) => { + if (this.clientKey !== value.clientId) { + if ( + this.isWithinMaxAge(value.updateTimeMs) && + value.inForeground + ) { + canActAsPrimary = false; + control.done(); + } + } + }) + .next(() => { + return canActAsPrimary; + }); + }) + .next(canActAsPrimary => { + log.debug( + LOG_TAG, + `Client ${ + canActAsPrimary ? 'is' : 'is not' + } eligible for a primary lease.` + ); + return canActAsPrimary; }); - }); } shutdown(): Promise { @@ -305,8 +362,7 @@ export class IndexedDbPersistence implements Persistence { this.started = false; this.detachVisibilityHandler(); this.detachWindowUnloadHook(); - this.stopClientStateRefreshes(); - return this.releasePrimaryLease().then(() => { + return this.releasePrimaryLeaseIfHeld().then(() => { this.simpleDb.close(); }); } @@ -340,14 +396,15 @@ export class IndexedDbPersistence implements Persistence { // are the only reader/writer. return this.simpleDb.runTransaction('readwrite', ALL_STORES, txn => { if (requirePrimaryLease) { - assert(this.isPrimary, 'Operation requires the primary lease'); - // While we merely verify that we still retain the lease at first, - // IndexedDb's transaction guarantees that we will be able to obtain - // the lease before we commit. We do this to not immediately lose our - // lease after long-running operations. - return this.tryElectPrimaryCandidate(txn) - .next(suggestedPrimary => { - if (!this.isLocalClient(suggestedPrimary)) { + // While we merely verify that we have (or can acquire) the lease + // immediately, we wait to extend the primary lease until after + // executing transactionOperation(). This ensures that even if the + // transactionOperation takes a long time, we'll use a recent + // leaseTimestampMs in the extended (or newly acquired) lease. + return this.getCurrentPrimary(txn) + .next(currentPrimary => this.canActAsPrimary(currentPrimary, txn)) + .next(isPrimary => { + if (!isPrimary) { // TODO(multitab): Handle this gracefully and transition back to // secondary state. throw new FirestoreError( @@ -358,7 +415,7 @@ export class IndexedDbPersistence implements Persistence { return transactionOperation(txn); }) .next(result => { - return this.acquirePrimaryLease(txn).next(() => result); + return this.acquireOrExtendPrimaryLease(txn).next(() => result); }); } else { return transactionOperation(txn); @@ -370,7 +427,7 @@ export class IndexedDbPersistence implements Persistence { * Obtains or extends the new primary lease for the current client. This * method does not verify that the client is eligible for this lease. */ - private acquirePrimaryLease(txn): PersistencePromise { + private acquireOrExtendPrimaryLease(txn): PersistencePromise { const newPrimary = new DbOwner(this.clientKey, Date.now()); return ownerStore(txn) .put('owner', newPrimary) @@ -401,89 +458,47 @@ export class IndexedDbPersistence implements Persistence { return 'firestore/' + databaseInfo.persistenceKey + '/' + database + '/'; } - /** - * Checks if this client is eligible for a primary lease based on its - * visibility state and the visibility state of all active clients. A client - * can obtain the primary lease if it is either in the foreground or if it - * and all other clients are in the background. - */ - private canBecomePrimary( - txn: SimpleDbTransaction - ): PersistencePromise { - if (this.inForeground) { - return PersistencePromise.resolve(true); - } - - let canBecomePrimary = true; - - return clientMetadataStore(txn) - .iterate((key, value, control) => { - if (this.clientKey !== value.clientId) { - if (this.isWithinMaxAge(value.updateTimeMs) && value.inForeground) { - canBecomePrimary = false; - control.done(); - } - } - }) - .next(() => { - return canBecomePrimary; - }); - } - /** Checks the primary lease and removes it if we are the current primary. */ - private releasePrimaryLease(): Promise { - return this.simpleDb - .runTransaction('readwrite', [DbOwner.store], txn => { - const store = txn.store(DbOwner.store); - return store.get('owner').next(primaryClient => { - if (this.isLocalClient(primaryClient)) { - log.debug(LOG_TAG, 'Releasing primary lease.'); - return store.delete('owner'); - } else { - return PersistencePromise.resolve(); - } - }); - }) - .then(() => { - if (this.isPrimary) { - this.isPrimary = false; - return this.primaryStateListener(false); + private releasePrimaryLeaseIfHeld(): Promise { + this.isPrimary = false; + + return this.simpleDb.runTransaction('readwrite', [DbOwner.store], txn => { + const store = txn.store(DbOwner.store); + return store.get('owner').next(primaryClient => { + if (this.isLocalClient(primaryClient)) { + log.debug(LOG_TAG, 'Releasing primary lease.'); + return store.delete('owner'); + } else { + return PersistencePromise.resolve(); } }); + }); } /** Verifies that `updateTimeMs` is within CLIENT_STATE_MAX_AGE_MS. */ private isWithinMaxAge(updateTimeMs: number): boolean { const now = Date.now(); - const minAcceptable = now - CLIENT_STATE_MAX_AGE_MS; + const minAcceptable = now - CLIENT_METADATA_MAX_AGE_MS; const maxAcceptable = now; if (updateTimeMs < minAcceptable) { return false; } else if (updateTimeMs > maxAcceptable) { - log.error('Detected an update time that is in the future.'); + log.error( + `Detected an update time that is in the future: ${updateTimeMs} > ${ + maxAcceptable + }` + ); return false; } return true; } - private stopClientStateRefreshes(): void { - if (this.primaryLeaseRefreshHandle) { - clearInterval(this.primaryLeaseRefreshHandle); - this.primaryLeaseRefreshHandle = null; - } - } - private attachVisibilityHandler(): void { this.documentVisibilityHandler = () => { this.queue.enqueue(() => { - const inForeground = this.document.visibilityState === 'visible'; - if (inForeground !== this.inForeground) { - this.inForeground = inForeground; - return this.tryBecomePrimary(); - } else { - return Promise.resolve(); - } + this.inForeground = this.document.visibilityState === 'visible'; + return Promise.resolve(); }); }; @@ -541,16 +556,20 @@ export class IndexedDbPersistence implements Persistence { * zombied due to their tab closing) from LocalStorage, or null if no such * record exists. */ - private getZombiedClientId(): string | null { + private getZombiedClientId(): ClientKey | null { try { - const zombiedOwnerId = window.localStorage.getItem( + const zombiedClientId = window.localStorage.getItem( this.zombiedClientLocalStorageKey() ); - log.debug(LOG_TAG, 'Zombied ownerID from LocalStorage:', zombiedOwnerId); - return zombiedOwnerId; + log.debug( + LOG_TAG, + 'Zombied clientId from LocalStorage:', + zombiedClientId + ); + return zombiedClientId; } catch (e) { // Gracefully handle if LocalStorage isn't available / working. - log.error('Failed to get zombie owner id.', e); + log.error('Failed to get zombie client id.', e); return null; } } @@ -559,14 +578,14 @@ export class IndexedDbPersistence implements Persistence { * Records a zombied primary client (a primary client that had its tab closed) * in LocalStorage or, if passed null, deletes any recorded zombied owner. */ - private setZombiedClientId(zombieOwnerId: string | null) { + private setZombiedClientId(zombiedClientId: ClientKey | null) { try { - if (zombieOwnerId === null) { + if (zombiedClientId === null) { window.localStorage.removeItem(this.zombiedClientLocalStorageKey()); } else { window.localStorage.setItem( this.zombiedClientLocalStorageKey(), - zombieOwnerId + zombiedClientId ); } } catch (e) { @@ -576,7 +595,7 @@ export class IndexedDbPersistence implements Persistence { } private zombiedClientLocalStorageKey(): string { - return this.localStoragePrefix + ZOMBIE_PRIMARY_LOCALSTORAGE_SUFFIX; + return this.localStoragePrefix + ZOMBIED_PRIMARY_LOCALSTORAGE_SUFFIX; } private generateClientKey(): ClientKey { diff --git a/packages/firestore/src/local/memory_persistence.ts b/packages/firestore/src/local/memory_persistence.ts index 90b484cf669..86e1b43452d 100644 --- a/packages/firestore/src/local/memory_persistence.ts +++ b/packages/firestore/src/local/memory_persistence.ts @@ -68,11 +68,6 @@ export class MemoryPersistence implements Persistence { return Promise.resolve(); } - tryBecomePrimary(): Promise { - // All clients using memory persistence act as primary. - return Promise.resolve(true); - } - setPrimaryStateListener(primaryStateListener: PrimaryStateListener) { // All clients using memory persistence act as primary. this.queue.enqueue(() => primaryStateListener(true)); diff --git a/packages/firestore/src/util/async_queue.ts b/packages/firestore/src/util/async_queue.ts index adbb549aefa..337f7651d31 100644 --- a/packages/firestore/src/util/async_queue.ts +++ b/packages/firestore/src/util/async_queue.ts @@ -33,7 +33,7 @@ export enum TimerId { ListenStreamConnection, WriteStreamIdle, WriteStreamConnection, - ClientStateRefresh + ClientMetadataRefresh } /** diff --git a/packages/firestore/test/unit/specs/persistence_spec.test.ts b/packages/firestore/test/unit/specs/persistence_spec.test.ts index a4eb4df7438..7ca5a626174 100644 --- a/packages/firestore/test/unit/specs/persistence_spec.test.ts +++ b/packages/firestore/test/unit/specs/persistence_spec.test.ts @@ -195,7 +195,6 @@ describeSpec('Persistence:', ['persistence'], () => { .expectPrimaryState(false) .client(0) .shutdown() - .expectPrimaryState(false) .client(1) .tryAcquirePrimaryLease() .expectPrimaryState(true); @@ -217,7 +216,6 @@ describeSpec('Persistence:', ['persistence'], () => { .client(0) // Shutdown the client that is currently holding the primary lease. .shutdown() - .expectPrimaryState(false) .client(1) // Client 1 is in the background and doesn't grab the primary lease as // client 2 is in the foreground. diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index 3fd98be3e99..1212be728be 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -793,9 +793,11 @@ abstract class TestRunner { } private async doAcquirePrimaryLease(): Promise { - expect(this.queue.containsDelayedOperation(TimerId.ClientStateRefresh)).to - .be.true; - return this.queue.runDelayedOperationsEarly(TimerId.ClientStateRefresh); + // We drain the queue after running the client metadata refresh task as teh + // refresh might schedule the primary state callback on the queue as well. + return this.queue + .runDelayedOperationsEarly(TimerId.ClientMetadataRefresh) + .then(() => this.queue.drain()); } private async doShutdown(): Promise { From 010729bba7fc8a99e4024de6d9192bf62e644d4a Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Tue, 27 Feb 2018 11:10:23 -0800 Subject: [PATCH 10/18] Removing the persistence Spec test --- packages/firestore/test/unit/specs/describe_spec.ts | 3 --- packages/firestore/test/unit/specs/persistence_spec.test.ts | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/firestore/test/unit/specs/describe_spec.ts b/packages/firestore/test/unit/specs/describe_spec.ts index fb86a18e9dd..d2345e8170e 100644 --- a/packages/firestore/test/unit/specs/describe_spec.ts +++ b/packages/firestore/test/unit/specs/describe_spec.ts @@ -24,8 +24,6 @@ import { SpecStep } from './spec_test_runner'; // Disables all other tests; useful for debugging. Multiple tests can have // this tag and they'll all be run (but all others won't). const EXCLUSIVE_TAG = 'exclusive'; -// Persistence-related tests. -const PERSISTENCE_TAG = 'persistence'; // Multi-client related tests (which imply persistence). const MULTI_CLIENT_TAG = 'multi-client'; // Explicit per-platform disable flags. @@ -34,7 +32,6 @@ const NO_ANDROID_TAG = 'no-android'; const NO_IOS_TAG = 'no-ios'; const KNOWN_TAGS = [ EXCLUSIVE_TAG, - PERSISTENCE_TAG, MULTI_CLIENT_TAG, NO_WEB_TAG, NO_ANDROID_TAG, diff --git a/packages/firestore/test/unit/specs/persistence_spec.test.ts b/packages/firestore/test/unit/specs/persistence_spec.test.ts index 7ca5a626174..1acfb725f79 100644 --- a/packages/firestore/test/unit/specs/persistence_spec.test.ts +++ b/packages/firestore/test/unit/specs/persistence_spec.test.ts @@ -21,7 +21,7 @@ import { doc, path } from '../../util/helpers'; import { describeSpec, specTest } from './describe_spec'; import { client, spec } from './spec_builder'; -describeSpec('Persistence:', ['persistence'], () => { +describeSpec('Persistence:', [], () => { specTest('Local mutations are persisted and re-sent', [], () => { return spec() .userSets('collection/key1', { foo: 'bar' }) From 806a86ae4c33af6fec5b1232e0bde7740e797117 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Tue, 27 Feb 2018 11:41:49 -0800 Subject: [PATCH 11/18] Cleanup for canActAsPrimary --- .../src/local/indexeddb_persistence.ts | 58 ++++++++----------- .../test/unit/specs/spec_test_runner.ts | 10 +++- 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/packages/firestore/src/local/indexeddb_persistence.ts b/packages/firestore/src/local/indexeddb_persistence.ts index b3b0ef42896..7920f408a4d 100644 --- a/packages/firestore/src/local/indexeddb_persistence.ts +++ b/packages/firestore/src/local/indexeddb_persistence.ts @@ -229,26 +229,23 @@ export class IndexedDbPersistence implements Persistence { ); } - if (currentPrimary != null) { - if (this.isPrimary) { - // If we are the primary lease holder, we refresh the client - // metadata and the primary lease after the default interval. - return this.acquireOrExtendPrimaryLease(txn).next( - () => CLIENT_METADATA_REFRESH_INTERVAL_MS - ); - } else { - // If another client currently holds the lease, we use a custom - // refresh interval and schedule a lease refresh immediately after - // the current lease is expected to expire. - const remainingLifetimeMs = - Date.now() - - (currentPrimary.leaseTimestampMs + - CLIENT_METADATA_MAX_AGE_MS); - return Math.min( - MINIMUM_REFRESH_INTERVAL_MS, - remainingLifetimeMs + 1 - ); - } + if (this.isPrimary) { + // If we are the primary lease holder, we refresh the client + // metadata and the primary lease after the default interval. + return this.acquireOrExtendPrimaryLease(txn).next( + () => CLIENT_METADATA_REFRESH_INTERVAL_MS + ); + } else if (currentPrimary != null) { + // If another client currently holds the lease, we use a custom + // refresh interval and schedule a lease refresh immediately after + // the current lease is expected to expire. + const remainingLifetimeMs = + Date.now() - + (currentPrimary.leaseTimestampMs + CLIENT_METADATA_MAX_AGE_MS); + return Math.min( + MINIMUM_REFRESH_INTERVAL_MS, + remainingLifetimeMs + 1 + ); } else { // If there is no current leaseholder, but we are not ourselves // eligible to directly obtain the lease (based on our foreground @@ -316,20 +313,17 @@ export class IndexedDbPersistence implements Persistence { return PersistencePromise.resolve(this.isLocalClient(currentPrimary)); } + let canActAsPrimary = this.inForeground; + // Check if this client is eligible for a primary lease based on its // visibility state and the visibility state of all active clients. A // client can obtain the primary lease if it is either in the foreground // or if this client and all other clients are in the background. - return PersistencePromise.resolve(true) + return PersistencePromise.resolve() .next(() => { - if (this.inForeground) { - return true; - } - - let canActAsPrimary = true; - - return clientMetadataStore(txn) - .iterate((key, value, control) => { + if (!canActAsPrimary) { + canActAsPrimary = true; + return clientMetadataStore(txn).iterate((key, value, control) => { if (this.clientKey !== value.clientId) { if ( this.isWithinMaxAge(value.updateTimeMs) && @@ -339,12 +333,10 @@ export class IndexedDbPersistence implements Persistence { control.done(); } } - }) - .next(() => { - return canActAsPrimary; }); + } }) - .next(canActAsPrimary => { + .next(() => { log.debug( LOG_TAG, `Client ${ diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index 1212be728be..a417358ddae 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -1169,11 +1169,19 @@ export async function runSpec( } await runners[i].start(); } + let lastStep = null; + let count = 0; try { - let stepcnt = 0; await sequence(steps, async step => { + ++count; + lastStep = step; return runners[step.clientIndex || 0].run(step); }); + } catch (err) { + console.warn( + `Spec test failed at step ${count}: ${JSON.stringify(lastStep)}` + ); + throw err; } finally { for (const runner of runners) { await runner.shutdown(); From 2e10f8a9933fc0f5a9d1952886ebfdd40157d7e8 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Tue, 27 Feb 2018 11:56:53 -0800 Subject: [PATCH 12/18] Comment Cleanup --- .../firestore/src/local/indexeddb_persistence.ts | 12 ++++++------ packages/firestore/src/local/indexeddb_schema.ts | 4 ++-- .../firestore/test/unit/specs/spec_test_runner.ts | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/firestore/src/local/indexeddb_persistence.ts b/packages/firestore/src/local/indexeddb_persistence.ts index 7920f408a4d..c85a26b4c6b 100644 --- a/packages/firestore/src/local/indexeddb_persistence.ts +++ b/packages/firestore/src/local/indexeddb_persistence.ts @@ -59,7 +59,7 @@ const LOG_TAG = 'IndexedDbPersistence'; const CLIENT_METADATA_MAX_AGE_MS = 5000; /** * The maximum interval after which clients will update their metadata, - * including refreshing the primary lease if held or potentially trying to + * including refreshing their primary lease if held or potentially trying to * acquire it if not held. * * Primary clients may opportunistically refresh their metadata earlier @@ -302,8 +302,8 @@ export class IndexedDbPersistence implements Persistence { * Evaluate the state of all active instances and determine whether the local * client is or can act as the holder of the primary lease. Returns whether * the client is eligible for the lease, but does not actually acquire it. - * May return 'false' even if there is no active leaseholder when a foreground - * client should become leaseholder instead. + * May return 'false' even if there is no active leaseholder if another + * (foreground) client should become leaseholder instead. */ private canActAsPrimary( currentPrimary: DbOwner | null, @@ -313,18 +313,18 @@ export class IndexedDbPersistence implements Persistence { return PersistencePromise.resolve(this.isLocalClient(currentPrimary)); } - let canActAsPrimary = this.inForeground; - // Check if this client is eligible for a primary lease based on its // visibility state and the visibility state of all active clients. A // client can obtain the primary lease if it is either in the foreground // or if this client and all other clients are in the background. + let canActAsPrimary = this.inForeground; + return PersistencePromise.resolve() .next(() => { if (!canActAsPrimary) { canActAsPrimary = true; return clientMetadataStore(txn).iterate((key, value, control) => { - if (this.clientKey !== value.clientId) { + if (this.clientKey !== value.clientKey) { if ( this.isWithinMaxAge(value.updateTimeMs) && value.inForeground diff --git a/packages/firestore/src/local/indexeddb_schema.ts b/packages/firestore/src/local/indexeddb_schema.ts index 03105635e66..1894d209af5 100644 --- a/packages/firestore/src/local/indexeddb_schema.ts +++ b/packages/firestore/src/local/indexeddb_schema.ts @@ -547,11 +547,11 @@ export class DbClientMetadata { static store = 'clientMetadata'; /** Keys are automatically assigned via the clientId properties. */ - static keyPath = ['clientId']; + static keyPath = ['clientKey']; constructor( /** The auto-generated client id assigned at client startup. */ - public clientId: string, + public clientKey: string, /** The last time this state was updated. */ public updateTimeMs: number, /** Whether this client is running in a foreground tab. */ diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index a417358ddae..24a9da280f9 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -793,8 +793,8 @@ abstract class TestRunner { } private async doAcquirePrimaryLease(): Promise { - // We drain the queue after running the client metadata refresh task as teh - // refresh might schedule the primary state callback on the queue as well. + // We drain the queue after running the client metadata refresh task as the + // refresh might schedule a primary state callback on the queue as well. return this.queue .runDelayedOperationsEarly(TimerId.ClientMetadataRefresh) .then(() => this.queue.drain()); From 8481ee5aafed5c509d3e41314d883a307055b64e Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 28 Feb 2018 10:07:13 -0800 Subject: [PATCH 13/18] Adding TODO to remove tryAcquirePrimaryLease --- packages/firestore/src/local/indexeddb_persistence.ts | 2 +- packages/firestore/test/unit/specs/spec_builder.ts | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/firestore/src/local/indexeddb_persistence.ts b/packages/firestore/src/local/indexeddb_persistence.ts index c85a26b4c6b..c65ef993e79 100644 --- a/packages/firestore/src/local/indexeddb_persistence.ts +++ b/packages/firestore/src/local/indexeddb_persistence.ts @@ -302,7 +302,7 @@ export class IndexedDbPersistence implements Persistence { * Evaluate the state of all active instances and determine whether the local * client is or can act as the holder of the primary lease. Returns whether * the client is eligible for the lease, but does not actually acquire it. - * May return 'false' even if there is no active leaseholder if another + * May return 'false' even if there is no active leaseholder and another * (foreground) client should become leaseholder instead. */ private canActAsPrimary( diff --git a/packages/firestore/test/unit/specs/spec_builder.ts b/packages/firestore/test/unit/specs/spec_builder.ts index 34d962dd020..e407e16dabe 100644 --- a/packages/firestore/test/unit/specs/spec_builder.ts +++ b/packages/firestore/test/unit/specs/spec_builder.ts @@ -252,6 +252,8 @@ export class SpecBuilder { return this; } + // TODO: Replace with .runTimer(TimerId.ClientStateRefresh) once #412 is + // merged. tryAcquirePrimaryLease(): SpecBuilder { this.nextStep(); this.currentStep = { From 972cb4cb95cb8bf8111061c21b90a26f9a68176c Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 28 Feb 2018 16:14:35 -0800 Subject: [PATCH 14/18] Addressing comments --- .../src/local/indexeddb_persistence.ts | 181 +++++++----------- 1 file changed, 65 insertions(+), 116 deletions(-) diff --git a/packages/firestore/src/local/indexeddb_persistence.ts b/packages/firestore/src/local/indexeddb_persistence.ts index c65ef993e79..7e4c2e1d92d 100644 --- a/packages/firestore/src/local/indexeddb_persistence.ts +++ b/packages/firestore/src/local/indexeddb_persistence.ts @@ -58,21 +58,14 @@ const LOG_TAG = 'IndexedDbPersistence'; */ const CLIENT_METADATA_MAX_AGE_MS = 5000; /** - * The maximum interval after which clients will update their metadata, - * including refreshing their primary lease if held or potentially trying to - * acquire it if not held. + * The interval at which clients will update their metadata, including + * refreshing their primary lease if held or potentially trying to acquire it if + * not held. * * Primary clients may opportunistically refresh their metadata earlier - * if they're already performing an IndexedDB operation. Secondary clients - * may also temporarily use a shorter refresh interval to synchronize their - * state refreshes with the primary lease holder. + * if they're already performing an IndexedDB operation. */ const CLIENT_METADATA_REFRESH_INTERVAL_MS = 4000; -/** - * Minimum interval for all state refreshes. Used when synchronizing the primary - * lease refresh with an existing lease that is about to expire. - */ -const MINIMUM_REFRESH_INTERVAL_MS = 1000; /** LocalStorage location to indicate a zombied client id (see class comment). */ const ZOMBIED_PRIMARY_LOCALSTORAGE_SUFFIX = 'zombiedClientId'; /** User-facing error when the primary lease is required but not available. */ @@ -186,8 +179,8 @@ export class IndexedDbPersistence implements Persistence { .then(() => { this.attachVisibilityHandler(); this.attachWindowUnloadHook(); - return this.updateClientMetadataAndTryBecomePrimary().then(leaseTtl => - this.scheduleClientMetadataAndPrimaryLeaseRefreshes(leaseTtl) + return this.updateClientMetadataAndTryBecomePrimary().then(() => + this.scheduleClientMetadataAndPrimaryLeaseRefreshes() ); }); } @@ -206,54 +199,22 @@ export class IndexedDbPersistence implements Persistence { * @return {Promise} A Promise that resolves with the interval in ms * after which the metadata and primary lease needs to be refreshed. */ - private updateClientMetadataAndTryBecomePrimary(): Promise { + private updateClientMetadataAndTryBecomePrimary(): Promise { return this.simpleDb.runTransaction('readwrite', ALL_STORES, txn => { const metadataStore = clientMetadataStore(txn); metadataStore.put( new DbClientMetadata(this.clientKey, Date.now(), this.inForeground) ); - return this.getCurrentPrimary(txn).next(currentPrimary => { - return this.canActAsPrimary(currentPrimary, txn).next( - canActAsPrimary => { - if (canActAsPrimary !== this.isPrimary) { - this.isPrimary = canActAsPrimary; - log.debug( - LOG_TAG, - `Primary lease changed. Current client ${ - this.isPrimary ? 'acquired' : 'released' - } its primary lease.` - ); - this.queue.enqueue(() => - this.primaryStateListener(this.isPrimary) - ); - } + return this.canActAsPrimary(txn).next(canActAsPrimary => { + if (canActAsPrimary !== this.isPrimary) { + this.isPrimary = canActAsPrimary; + this.queue.enqueue(() => this.primaryStateListener(this.isPrimary)); + } - if (this.isPrimary) { - // If we are the primary lease holder, we refresh the client - // metadata and the primary lease after the default interval. - return this.acquireOrExtendPrimaryLease(txn).next( - () => CLIENT_METADATA_REFRESH_INTERVAL_MS - ); - } else if (currentPrimary != null) { - // If another client currently holds the lease, we use a custom - // refresh interval and schedule a lease refresh immediately after - // the current lease is expected to expire. - const remainingLifetimeMs = - Date.now() - - (currentPrimary.leaseTimestampMs + CLIENT_METADATA_MAX_AGE_MS); - return Math.min( - MINIMUM_REFRESH_INTERVAL_MS, - remainingLifetimeMs + 1 - ); - } else { - // If there is no current leaseholder, but we are not ourselves - // eligible to directly obtain the lease (based on our foreground - // state), we try again after the minimum refresh interval. - return MINIMUM_REFRESH_INTERVAL_MS; - } - } - ); + if (this.isPrimary) { + return this.acquireOrExtendPrimaryLease(txn); + } }); }); } @@ -261,19 +222,17 @@ export class IndexedDbPersistence implements Persistence { /** * Schedules a recurring timer to update the client metadata and to either * extend or acquire the primary lease if the client is eligible. - * - * @param delayMs The delay to use for the first refresh. Subsequent refreshes - * may use a different delay based on the primary leaseholder's refresh - * interval. */ - private scheduleClientMetadataAndPrimaryLeaseRefreshes( - delayMs: number - ): void { - this.queue.enqueueAfterDelay(TimerId.ClientMetadataRefresh, delayMs, () => { - return this.updateClientMetadataAndTryBecomePrimary().then(leaseTtl => - this.scheduleClientMetadataAndPrimaryLeaseRefreshes(leaseTtl) - ); - }); + private scheduleClientMetadataAndPrimaryLeaseRefreshes(): void { + this.queue.enqueueAfterDelay( + TimerId.ClientMetadataRefresh, + CLIENT_METADATA_REFRESH_INTERVAL_MS, + () => { + return this.updateClientMetadataAndTryBecomePrimary().then(() => + this.scheduleClientMetadataAndPrimaryLeaseRefreshes() + ); + } + ); } /** Checks whether `client` is the local client. */ @@ -281,23 +240,6 @@ export class IndexedDbPersistence implements Persistence { return client ? client.ownerId === this.clientKey : false; } - private getCurrentPrimary( - txn: SimpleDbTransaction - ): PersistencePromise { - const store = ownerStore(txn); - return store.get('owner').next(currentPrimary => { - if ( - currentPrimary !== null && - this.isWithinMaxAge(currentPrimary.leaseTimestampMs) && - currentPrimary.ownerId !== this.getZombiedClientId() - ) { - return currentPrimary; - } else { - return null; - } - }); - } - /** * Evaluate the state of all active instances and determine whether the local * client is or can act as the holder of the primary lease. Returns whether @@ -306,24 +248,32 @@ export class IndexedDbPersistence implements Persistence { * (foreground) client should become leaseholder instead. */ private canActAsPrimary( - currentPrimary: DbOwner | null, txn: SimpleDbTransaction ): PersistencePromise { - if (currentPrimary !== null) { - return PersistencePromise.resolve(this.isLocalClient(currentPrimary)); - } + const store = ownerStore(txn); + return store + .get('owner') + .next(currentPrimary => { + const currentLeaseIsValid = + currentPrimary !== null && + this.isWithinMaxAge(currentPrimary.leaseTimestampMs) && + currentPrimary.ownerId !== this.getZombiedClientId(); + + if (currentLeaseIsValid) { + return this.isLocalClient(currentPrimary); + } + + // Check if this client is eligible for a primary lease based on its + // visibility state and the visibility state of all active clients. A + // client can obtain the primary lease if it is either in the foreground + // or if this client and all other clients are in the background. + if (this.inForeground) { + return true; + } - // Check if this client is eligible for a primary lease based on its - // visibility state and the visibility state of all active clients. A - // client can obtain the primary lease if it is either in the foreground - // or if this client and all other clients are in the background. - let canActAsPrimary = this.inForeground; - - return PersistencePromise.resolve() - .next(() => { - if (!canActAsPrimary) { - canActAsPrimary = true; - return clientMetadataStore(txn).iterate((key, value, control) => { + let canActAsPrimary = true; + return clientMetadataStore(txn) + .iterate((key, value, control) => { if (this.clientKey !== value.clientKey) { if ( this.isWithinMaxAge(value.updateTimeMs) && @@ -333,16 +283,18 @@ export class IndexedDbPersistence implements Persistence { control.done(); } } - }); - } + }) + .next(() => canActAsPrimary); }) - .next(() => { - log.debug( - LOG_TAG, - `Client ${ - canActAsPrimary ? 'is' : 'is not' - } eligible for a primary lease.` - ); + .next(canActAsPrimary => { + if (this.isPrimary !== canActAsPrimary) { + log.debug( + LOG_TAG, + `Client ${ + canActAsPrimary ? 'is' : 'is not' + } eligible for a primary lease.` + ); + } return canActAsPrimary; }); } @@ -393,10 +345,9 @@ export class IndexedDbPersistence implements Persistence { // executing transactionOperation(). This ensures that even if the // transactionOperation takes a long time, we'll use a recent // leaseTimestampMs in the extended (or newly acquired) lease. - return this.getCurrentPrimary(txn) - .next(currentPrimary => this.canActAsPrimary(currentPrimary, txn)) - .next(isPrimary => { - if (!isPrimary) { + return this.canActAsPrimary(txn) + .next(canActAsPrimary => { + if (!canActAsPrimary) { // TODO(multitab): Handle this gracefully and transition back to // secondary state. throw new FirestoreError( @@ -419,11 +370,9 @@ export class IndexedDbPersistence implements Persistence { * Obtains or extends the new primary lease for the current client. This * method does not verify that the client is eligible for this lease. */ - private acquireOrExtendPrimaryLease(txn): PersistencePromise { + private acquireOrExtendPrimaryLease(txn): PersistencePromise { const newPrimary = new DbOwner(this.clientKey, Date.now()); - return ownerStore(txn) - .put('owner', newPrimary) - .next(() => newPrimary); + return ownerStore(txn).put('owner', newPrimary); } static isAvailable(): boolean { @@ -490,7 +439,7 @@ export class IndexedDbPersistence implements Persistence { this.documentVisibilityHandler = () => { this.queue.enqueue(() => { this.inForeground = this.document.visibilityState === 'visible'; - return Promise.resolve(); + return this.updateClientMetadataAndTryBecomePrimary(); }); }; From 9111376b21cbd6d67f750b021d4614e984af09f4 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 28 Feb 2018 18:02:58 -0800 Subject: [PATCH 15/18] Adding assert to AsyncQueue --- packages/firestore/src/util/async_queue.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/firestore/src/util/async_queue.ts b/packages/firestore/src/util/async_queue.ts index 337f7651d31..12d2c3aa755 100644 --- a/packages/firestore/src/util/async_queue.ts +++ b/packages/firestore/src/util/async_queue.ts @@ -218,6 +218,9 @@ export class AsyncQueue { ): CancelablePromise { this.verifyNotFailed(); + assert(delayMs >= 0, + `Attempted to schedule an operation with a negative delay of ${delayMs}`); + // While not necessarily harmful, we currently don't expect to have multiple // ops with the same timer id in the queue, so defensively reject them. assert( From 042a8e53654649f079b1aeca9fd1f9ad458dfdaf Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 28 Feb 2018 18:03:21 -0800 Subject: [PATCH 16/18] [AUTOMATED]: Prettier Code Styling --- packages/firestore/src/util/async_queue.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/firestore/src/util/async_queue.ts b/packages/firestore/src/util/async_queue.ts index 12d2c3aa755..e4aaab59033 100644 --- a/packages/firestore/src/util/async_queue.ts +++ b/packages/firestore/src/util/async_queue.ts @@ -218,8 +218,10 @@ export class AsyncQueue { ): CancelablePromise { this.verifyNotFailed(); - assert(delayMs >= 0, - `Attempted to schedule an operation with a negative delay of ${delayMs}`); + assert( + delayMs >= 0, + `Attempted to schedule an operation with a negative delay of ${delayMs}` + ); // While not necessarily harmful, we currently don't expect to have multiple // ops with the same timer id in the queue, so defensively reject them. From 50a60511e8a320fdb52a62598e59ddcb52049d30 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 1 Mar 2018 09:56:55 -0800 Subject: [PATCH 17/18] Last round of feedback --- .../firestore/src/local/indexeddb_persistence.ts | 5 +---- .../firestore/test/unit/specs/spec_builder.ts | 6 ++++++ .../test/unit/specs/spec_test_runner.ts | 16 ++++++---------- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/packages/firestore/src/local/indexeddb_persistence.ts b/packages/firestore/src/local/indexeddb_persistence.ts index 7e4c2e1d92d..b6368ab96d3 100644 --- a/packages/firestore/src/local/indexeddb_persistence.ts +++ b/packages/firestore/src/local/indexeddb_persistence.ts @@ -195,9 +195,6 @@ export class IndexedDbPersistence implements Persistence { * extend the primary lease for the local client. Asynchronously notifies the * primary state listener if the client either newly obtained or released its * primary lease. - * - * @return {Promise} A Promise that resolves with the interval in ms - * after which the metadata and primary lease needs to be refreshed. */ private updateClientMetadataAndTryBecomePrimary(): Promise { return this.simpleDb.runTransaction('readwrite', ALL_STORES, txn => { @@ -241,7 +238,7 @@ export class IndexedDbPersistence implements Persistence { } /** - * Evaluate the state of all active instances and determine whether the local + * Evaluate the state of all active clients and determine whether the local * client is or can act as the holder of the primary lease. Returns whether * the client is eligible for the lease, but does not actually acquire it. * May return 'false' even if there is no active leaseholder and another diff --git a/packages/firestore/test/unit/specs/spec_builder.ts b/packages/firestore/test/unit/specs/spec_builder.ts index e407e16dabe..52e5726247e 100644 --- a/packages/firestore/test/unit/specs/spec_builder.ts +++ b/packages/firestore/test/unit/specs/spec_builder.ts @@ -195,6 +195,7 @@ export class SpecBuilder { return this; } + // PORTING NOTE: Only used by web multi-tab tests. becomeHidden(): SpecBuilder { this.nextStep(); this.currentStep = { @@ -203,6 +204,7 @@ export class SpecBuilder { return this; } + // PORTING NOTE: Only used by web multi-tab tests. becomeVisible(): SpecBuilder { this.nextStep(); this.currentStep = { @@ -254,6 +256,7 @@ export class SpecBuilder { // TODO: Replace with .runTimer(TimerId.ClientStateRefresh) once #412 is // merged. + // PORTING NOTE: Only used by web multi-tab tests. tryAcquirePrimaryLease(): SpecBuilder { this.nextStep(); this.currentStep = { @@ -704,7 +707,9 @@ export class SpecBuilder { * * Use `client(clientIndex)` to switch between clients. */ +// PORTING NOTE: Only used by web multi-tab tests. export class MultiClientSpecBuilder extends SpecBuilder { +// TODO(multitab): Consider merging this with SpecBuilder. private activeClientIndex = -1; client(clientIndex: number): MultiClientSpecBuilder { @@ -941,6 +946,7 @@ export function spec(): SpecBuilder { } /** Starts a new multi-client SpecTest. */ +// PORTING NOTE: Only used by web multi-tab tests. export function client(num: number): MultiClientSpecBuilder { return new MultiClientSpecBuilder().client(num); } diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index 24a9da280f9..01f1422daed 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -479,12 +479,14 @@ abstract class TestRunner { ? this.doEnableNetwork() : this.doDisableNetwork(); } else if ('acquirePrimaryLease' in step) { + // PORTING NOTE: Only used by web multi-tab tests. return this.doAcquirePrimaryLease(); } else if ('restart' in step) { return this.doRestart(); } else if ('shutdown' in step) { return this.doShutdown(); } else if ('applyClientState' in step) { + // PORTING NOTE: Only used by web multi-tab tests. return this.doApplyClientState(step.applyClientState!); } else if ('changeUser' in step) { return this.doChangeUser(step.changeUser!); @@ -1035,11 +1037,6 @@ class MemoryTestRunner extends TestRunner { protected getPersistence(serializer: JsonProtoSerializer): Persistence { return new MemoryPersistence(this.queue); } - - static destroyPersistence(): Promise { - // Nothing to do. - return Promise.resolve(); - } } /** @@ -1188,8 +1185,6 @@ export async function runSpec( } if (usePersistence) { await IndexedDbTestRunner.destroyPersistence(); - } else { - await MemoryTestRunner.destroyPersistence(); } } } @@ -1209,7 +1204,7 @@ export interface SpecConfig { */ export interface SpecStep { /** The index of the current client for multi-client spec tests. */ - clientIndex?: number; + clientIndex?: number; // PORTING NOTE: Only used by web multi-tab tests /** Listen to a new query (must be unique) */ userListen?: SpecUserListen; /** Unlisten from a query (must be listened to) */ @@ -1250,13 +1245,13 @@ export interface SpecStep { enableNetwork?: boolean; /** Changes the metadata state of a client instance. */ - applyClientState?: SpecClientState; + applyClientState?: SpecClientState; // PORTING NOTE: Only used by web multi-tab tests /** Change to a new active user (specified by uid or null for anonymous). */ changeUser?: string | null; /** Attempt to acquire the primary lease. */ - acquirePrimaryLease?: true; + acquirePrimaryLease?: true; // PORTING NOTE: Only used by web multi-tab tests /** * Restarts the SyncEngine from scratch, except re-uses persistence and auth @@ -1346,6 +1341,7 @@ export interface SpecWatchEntity { removedTargets?: TargetId[]; } +// PORTING NOTE: Only used by web multi-tab tests. export type SpecClientState = { /** The visibility state of the browser tab running the client. */ visibility?: VisibilityState; From 35dfb02f1f37652b38c76a9f2a5686b3c667ad32 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 1 Mar 2018 11:14:15 -0800 Subject: [PATCH 18/18] [AUTOMATED]: Prettier Code Styling --- packages/firestore/test/unit/specs/spec_builder.ts | 2 +- packages/firestore/test/unit/specs/spec_test_runner.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/firestore/test/unit/specs/spec_builder.ts b/packages/firestore/test/unit/specs/spec_builder.ts index 52e5726247e..58e3b865faf 100644 --- a/packages/firestore/test/unit/specs/spec_builder.ts +++ b/packages/firestore/test/unit/specs/spec_builder.ts @@ -709,7 +709,7 @@ export class SpecBuilder { */ // PORTING NOTE: Only used by web multi-tab tests. export class MultiClientSpecBuilder extends SpecBuilder { -// TODO(multitab): Consider merging this with SpecBuilder. + // TODO(multitab): Consider merging this with SpecBuilder. private activeClientIndex = -1; client(clientIndex: number): MultiClientSpecBuilder { diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index 01f1422daed..0a049913380 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -1204,7 +1204,7 @@ export interface SpecConfig { */ export interface SpecStep { /** The index of the current client for multi-client spec tests. */ - clientIndex?: number; // PORTING NOTE: Only used by web multi-tab tests + clientIndex?: number; // PORTING NOTE: Only used by web multi-tab tests /** Listen to a new query (must be unique) */ userListen?: SpecUserListen; /** Unlisten from a query (must be listened to) */ @@ -1245,13 +1245,13 @@ export interface SpecStep { enableNetwork?: boolean; /** Changes the metadata state of a client instance. */ - applyClientState?: SpecClientState; // PORTING NOTE: Only used by web multi-tab tests + applyClientState?: SpecClientState; // PORTING NOTE: Only used by web multi-tab tests /** Change to a new active user (specified by uid or null for anonymous). */ changeUser?: string | null; /** Attempt to acquire the primary lease. */ - acquirePrimaryLease?: true; // PORTING NOTE: Only used by web multi-tab tests + acquirePrimaryLease?: true; // PORTING NOTE: Only used by web multi-tab tests /** * Restarts the SyncEngine from scratch, except re-uses persistence and auth