diff --git a/packages/firestore/src/core/sync_engine.ts b/packages/firestore/src/core/sync_engine.ts index 0204f403e50..1c8a0aee75f 100644 --- a/packages/firestore/src/core/sync_engine.ts +++ b/packages/firestore/src/core/sync_engine.ts @@ -1110,14 +1110,10 @@ export class MultiTabSyncEngine extends SyncEngine const queries = this.queriesByTarget.get(targetId); if (queries && queries.length !== 0) { - // For queries that have a local View, we need to update their state - // in LocalStore (as the resume token and the snapshot version + // For queries that have a local View, we fetch their current state + // from LocalStore (as the resume token and the snapshot version // might have changed) and reconcile their views with the persisted // state (the list of syncedDocuments may have gotten out of sync). - await this.localStore.releaseTarget( - targetId, - /*keepPersistedTargetData=*/ true - ); targetData = await this.localStore.allocateTarget( queries[0].toTarget() ); @@ -1136,7 +1132,7 @@ export class MultiTabSyncEngine extends SyncEngine } else { debugAssert( transitionToPrimary, - 'A secondary tab should never have an active target without an active query.' + 'A secondary tab should never have an active view without an active target.' ); // For queries that never executed on this client, we need to // allocate the target in LocalStore and initialize a new View. diff --git a/packages/firestore/src/local/indexeddb_persistence.ts b/packages/firestore/src/local/indexeddb_persistence.ts index 7733706c3ac..a0207e7e1c2 100644 --- a/packages/firestore/src/local/indexeddb_persistence.ts +++ b/packages/firestore/src/local/indexeddb_persistence.ts @@ -404,7 +404,7 @@ export class IndexedDbPersistence implements Persistence { return this.verifyPrimaryLease(txn).next(success => { if (!success) { this.isPrimary = false; - this.queue.enqueueAndForget(() => + this.queue.enqueueRetryable(() => this.primaryStateListener(false) ); } @@ -444,7 +444,7 @@ export class IndexedDbPersistence implements Persistence { }) .then(isPrimary => { if (this.isPrimary !== isPrimary) { - this.queue.enqueueAndForget(() => + this.queue.enqueueRetryable(() => this.primaryStateListener(isPrimary) ); } @@ -805,7 +805,7 @@ export class IndexedDbPersistence implements Persistence { `Failed to obtain primary lease for action '${action}'.` ); this.isPrimary = false; - this.queue.enqueueAndForget(() => + this.queue.enqueueRetryable(() => this.primaryStateListener(false) ); throw new FirestoreError( diff --git a/packages/firestore/src/local/local_store.ts b/packages/firestore/src/local/local_store.ts index 361cbd48b7f..d09f48089b2 100644 --- a/packages/firestore/src/local/local_store.ts +++ b/packages/firestore/src/local/local_store.ts @@ -831,7 +831,17 @@ export class LocalStore { }); }) .then(targetData => { - if (this.targetDataByTarget.get(targetData.targetId) === null) { + // If Multi-Tab is enabled, the existing target data may be newer than + // the in-memory data + const cachedTargetData = this.targetDataByTarget.get( + targetData.targetId + ); + if ( + cachedTargetData === null || + targetData.snapshotVersion.compareTo( + cachedTargetData.snapshotVersion + ) > 0 + ) { this.targetDataByTarget = this.targetDataByTarget.insert( targetData.targetId, targetData diff --git a/packages/firestore/test/unit/specs/listen_spec.test.ts b/packages/firestore/test/unit/specs/listen_spec.test.ts index 5845e4e6f54..a34d14298a6 100644 --- a/packages/firestore/test/unit/specs/listen_spec.test.ts +++ b/packages/firestore/test/unit/specs/listen_spec.test.ts @@ -17,7 +17,7 @@ import { Query } from '../../../src/core/query'; import { Code } from '../../../src/util/error'; -import { deletedDoc, doc, filter, path, orderBy } from '../../util/helpers'; +import { deletedDoc, doc, filter, orderBy, path } from '../../util/helpers'; import { TimerId } from '../../../src/util/async_queue'; import { describeSpec, specTest } from './describe_spec'; diff --git a/packages/firestore/test/unit/specs/recovery_spec.test.ts b/packages/firestore/test/unit/specs/recovery_spec.test.ts index a302036c593..26a8208bbf3 100644 --- a/packages/firestore/test/unit/specs/recovery_spec.test.ts +++ b/packages/firestore/test/unit/specs/recovery_spec.test.ts @@ -88,7 +88,7 @@ describeSpec('Persistence Recovery', ['no-ios', 'no-android'], () => { ); specTest( - 'Query raises events in secondary client (with recovery)', + 'Query raises events in secondary client (with recovery)', ['multi-client'], () => { const query = Query.atPath(path('collection')); @@ -144,6 +144,84 @@ describeSpec('Persistence Recovery', ['no-ios', 'no-android'], () => { } ); + specTest( + 'Query with active view recovers after primary tab failover (with recovery)', + ['multi-client'], + () => { + const query = Query.atPath(path('collection')); + const docA = doc('collection/a', 1000, { key: 'a' }); + const docB = doc('collection/b', 2000, { key: 'b' }); + + return ( + client(0) + .expectPrimaryState(true) + .client(1) + // Register a query in the secondary client + .userListens(query) + .client(0) + .expectListen(query) + .watchAcksFull(query, 1000, docA) + // Shutdown the primary client to release its lease + .shutdown() + .client(1) + .expectEvents(query, { added: [docA] }) + // Run the lease refresh to attempt taking over the primary lease. The + // first lease refresh fails with a simulated transaction failure. + .failDatabaseTransactions('Allocate target') + .runTimer(TimerId.ClientMetadataRefresh) + .expectPrimaryState(false) + .recoverDatabase() + .runTimer(TimerId.AsyncQueueRetry) + .expectPrimaryState(true) + .expectListen(query, 'resume-token-1000') + .watchAcksFull(query, 2000, docB) + .expectEvents(query, { added: [docB] }) + ); + } + ); + + specTest( + 'Query without active view recovers after primary tab failover (with recovery)', + ['multi-client'], + () => { + const query = Query.atPath(path('collection')); + const docA = doc('collection/a', 1000, { key: 'a' }); + const docB = doc('collection/b', 2000, { key: 'b' }); + + return ( + client(0) + .expectPrimaryState(true) + // Initialize a second client that doesn't have any active targets + .client(1) + .client(2) + // Register a query in the third client + .userListens(query) + .client(0) + .expectListen(query) + .watchAcksFull(query, 1000, docA) + .client(2) + .expectEvents(query, { added: [docA] }) + .client(0) + // Shutdown the primary client to release its lease + .shutdown() + .client(1) + // Run the lease refresh in the second client, which does not yet have + // an active view for the third client's query. The lease refresh fails + // at first, but then recovers and initializes the view. + .failDatabaseTransactions('Allocate target') + .runTimer(TimerId.ClientMetadataRefresh) + .expectPrimaryState(false) + .recoverDatabase() + .runTimer(TimerId.AsyncQueueRetry) + .expectPrimaryState(true) + .expectListen(query, 'resume-token-1000') + .watchAcksFull(query, 2000, docB) + .client(2) + .expectEvents(query, { added: [docB] }) + ); + } + ); + specTest( 'Ignores intermittent lease refresh failures (with recovery)', ['multi-client'],