Skip to content

Ignore IndexedDB failure during lease refresh #3020

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 14, 2020
Merged

Ignore IndexedDB failure during lease refresh #3020

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3073f60
Ignore IndexedDB failure during lease refresh
schmidt-sebastian May 5, 2020
d87dc1e
Rewrite everything
schmidt-sebastian May 6, 2020
8d08444
Review
schmidt-sebastian May 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 72 additions & 50 deletions packages/firestore/src/local/indexeddb_persistence.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ const MAX_PRIMARY_ELIGIBLE_AGE_MS = 5000;
const CLIENT_METADATA_REFRESH_INTERVAL_MS = 4000;
/** User-facing error when the primary lease is required but not available. */
const PRIMARY_LEASE_EXCLUSIVE_ERROR_MSG =
'Another tab has exclusive access to the persistence layer. ' +
'Failed to obtain exclusive access to the persistence layer. ' +
'To allow shared access, make sure to invoke ' +
'`enablePersistence()` with `synchronizeTabs:true` in all tabs.';
const UNSUPPORTED_PLATFORM_ERROR_MSG =
Expand Down Expand Up @@ -191,11 +191,12 @@ export class IndexedDbPersistence implements Persistence {
private readonly document: Document | null;
private readonly window: Window;

// Technically these types should be `| undefined` because they are
// Technically `simpleDb` should be `| undefined` because it is
// initialized asynchronously by start(), but that would be more misleading
// than useful.
private simpleDb!: SimpleDb;
private listenSequence!: ListenSequence;

private listenSequence: ListenSequence | null = null;

private _started = false;
private isPrimary = false;
Expand Down Expand Up @@ -287,7 +288,15 @@ export class IndexedDbPersistence implements Persistence {
// having the persistence lock), so it's the first thing we should do.
return this.updateClientMetadataAndTryBecomePrimary();
})
.then(() => {
.then(e => {
if (!this.isPrimary && !this.allowTabSynchronization) {
// Fail `start()` if `synchronizeTabs` is disabled and we cannot
// obtain the primary lease.
throw new FirestoreError(
Code.FAILED_PRECONDITION,
PRIMARY_LEASE_EXCLUSIVE_ERROR_MSG
);
}
this.attachVisibilityHandler();
this.attachWindowUnloadHook();

Expand Down Expand Up @@ -375,8 +384,10 @@ export class IndexedDbPersistence implements Persistence {
* primary lease.
*/
private updateClientMetadataAndTryBecomePrimary(): Promise<void> {
return this.simpleDb
.runTransaction('readwrite', ALL_STORES, txn => {
return this.runTransaction(
'updateClientMetadataAndTryBecomePrimary',
'readwrite',
txn => {
const metadataStore = clientMetadataStore(txn);
return metadataStore
.put(
Expand Down Expand Up @@ -409,10 +420,18 @@ export class IndexedDbPersistence implements Persistence {
return /* canActAsPrimary= */ false;
}
});
})
}
)
.catch(e => {
if (!this.allowTabSynchronization) {
throw e;
if (e.name === 'IndexedDbTransactionError') {
logDebug(LOG_TAG, 'Failed to extend owner lease: ', e);
// Proceed with the existing state. Any subsequent access to
// IndexedDB will verify the lease.
return this.isPrimary;
} else {
throw e;
}
}

logDebug(
Expand All @@ -433,7 +452,7 @@ export class IndexedDbPersistence implements Persistence {
}

private verifyPrimaryLease(
txn: SimpleDbTransaction
txn: PersistenceTransaction
): PersistencePromise<boolean> {
const store = primaryClientStore(txn);
return store.get(DbPrimaryClient.key).next(primaryClient => {
Expand All @@ -442,7 +461,7 @@ export class IndexedDbPersistence implements Persistence {
}

private removeClientMetadata(
txn: SimpleDbTransaction
txn: PersistenceTransaction
): PersistencePromise<void> {
const metadataStore = clientMetadataStore(txn);
return metadataStore.delete(this.clientId);
Expand Down Expand Up @@ -536,7 +555,7 @@ export class IndexedDbPersistence implements Persistence {
* (foreground) client should become leaseholder instead.
*/
private canActAsPrimary(
txn: SimpleDbTransaction
txn: PersistenceTransaction
): PersistencePromise<boolean> {
const store = primaryClientStore(txn);
return store
Expand Down Expand Up @@ -645,15 +664,13 @@ export class IndexedDbPersistence implements Persistence {
}
this.detachVisibilityHandler();
this.detachWindowUnloadHook();
await this.simpleDb.runTransaction(
'readwrite',
[DbPrimaryClient.store, DbClientMetadata.store],
txn => {
return this.releasePrimaryLeaseIfHeld(txn).next(() =>
this.removeClientMetadata(txn)
);
}
);
await this.runTransaction('shutdown', 'readwrite', txn => {
return this.releasePrimaryLeaseIfHeld(txn).next(() =>
this.removeClientMetadata(txn)
);
}).catch(e => {
logDebug(LOG_TAG, 'Proceeding with shutdown despite failure: ', e);
});
this.simpleDb.close();

// Remove the entry marking the client as zombied from LocalStorage since
Expand Down Expand Up @@ -684,19 +701,15 @@ export class IndexedDbPersistence implements Persistence {
* PORTING NOTE: This is only used for Web multi-tab.
*/
getActiveClients(): Promise<ClientId[]> {
return this.simpleDb.runTransaction(
'readonly',
[DbClientMetadata.store],
txn => {
return clientMetadataStore(txn)
.loadAll()
.next(clients =>
this.filterActiveClients(clients, MAX_CLIENT_AGE_MS).map(
clientMetadata => clientMetadata.clientId
)
);
}
);
return this.runTransaction('getActiveClients', 'readonly', txn => {
return clientMetadataStore(txn)
.loadAll()
.next(clients =>
this.filterActiveClients(clients, MAX_CLIENT_AGE_MS).map(
clientMetadata => clientMetadata.clientId
)
);
});
}

static async clearPersistence(persistenceKey: string): Promise<void> {
Expand Down Expand Up @@ -767,7 +780,9 @@ export class IndexedDbPersistence implements Persistence {
.runTransaction(simpleDbMode, ALL_STORES, simpleDbTxn => {
persistenceTransaction = new IndexedDbTransaction(
simpleDbTxn,
this.listenSequence.next()
this.listenSequence
? this.listenSequence.next()
: ListenSequence.INVALID
);

if (mode === 'readwrite-primary') {
Expand All @@ -776,12 +791,12 @@ 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.verifyPrimaryLease(simpleDbTxn)
return this.verifyPrimaryLease(persistenceTransaction)
.next(holdsPrimaryLease => {
if (holdsPrimaryLease) {
return /* holdsPrimaryLease= */ true;
}
return this.canActAsPrimary(simpleDbTxn);
return this.canActAsPrimary(persistenceTransaction);
})
.next(holdsPrimaryLease => {
if (!holdsPrimaryLease) {
Expand All @@ -800,14 +815,14 @@ export class IndexedDbPersistence implements Persistence {
return transactionOperation(persistenceTransaction);
})
.next(result => {
return this.acquireOrExtendPrimaryLease(simpleDbTxn).next(
() => result
);
return this.acquireOrExtendPrimaryLease(
persistenceTransaction
).next(() => result);
});
} else {
return this.verifyAllowTabSynchronization(simpleDbTxn).next(() =>
transactionOperation(persistenceTransaction)
);
return this.verifyAllowTabSynchronization(
persistenceTransaction
).next(() => transactionOperation(persistenceTransaction));
}
})
.then(result => {
Expand All @@ -823,7 +838,7 @@ export class IndexedDbPersistence implements Persistence {
// TODO(b/114226234): Remove this check when `synchronizeTabs` can no longer
// be turned off.
private verifyAllowTabSynchronization(
txn: SimpleDbTransaction
txn: PersistenceTransaction
): PersistencePromise<void> {
const store = primaryClientStore(txn);
return store.get(DbPrimaryClient.key).next(currentPrimary => {
Expand All @@ -836,7 +851,10 @@ export class IndexedDbPersistence implements Persistence {
!this.isClientZombied(currentPrimary.ownerId);

if (currentLeaseIsValid && !this.isLocalClient(currentPrimary)) {
if (!currentPrimary!.allowTabSynchronization) {
if (
!this.allowTabSynchronization ||
!currentPrimary!.allowTabSynchronization
) {
throw new FirestoreError(
Code.FAILED_PRECONDITION,
PRIMARY_LEASE_EXCLUSIVE_ERROR_MSG
Expand All @@ -851,7 +869,7 @@ export class IndexedDbPersistence implements Persistence {
* method does not verify that the client is eligible for this lease.
*/
private acquireOrExtendPrimaryLease(
txn: SimpleDbTransaction
txn: PersistenceTransaction
): PersistencePromise<void> {
const newPrimary = new DbPrimaryClient(
this.clientId,
Expand Down Expand Up @@ -887,7 +905,7 @@ export class IndexedDbPersistence implements Persistence {

/** Checks the primary lease and removes it if we are the current primary. */
private releasePrimaryLeaseIfHeld(
txn: SimpleDbTransaction
txn: PersistenceTransaction
): PersistencePromise<void> {
const store = primaryClientStore(txn);
return store.get(DbPrimaryClient.key).next(primaryClient => {
Expand Down Expand Up @@ -1052,18 +1070,22 @@ export class IndexedDbPersistence implements Persistence {
* Helper to get a typed SimpleDbStore for the primary client object store.
*/
function primaryClientStore(
txn: SimpleDbTransaction
txn: PersistenceTransaction
): SimpleDbStore<DbPrimaryClientKey, DbPrimaryClient> {
return txn.store<DbPrimaryClientKey, DbPrimaryClient>(DbPrimaryClient.store);
return IndexedDbPersistence.getStore<DbPrimaryClientKey, DbPrimaryClient>(
txn,
DbPrimaryClient.store
);
}

/**
* Helper to get a typed SimpleDbStore for the client metadata object store.
*/
function clientMetadataStore(
txn: SimpleDbTransaction
txn: PersistenceTransaction
): SimpleDbStore<DbClientMetadataKey, DbClientMetadata> {
return txn.store<DbClientMetadataKey, DbClientMetadata>(
return IndexedDbPersistence.getStore<DbClientMetadataKey, DbClientMetadata>(
txn,
DbClientMetadata.store
);
}
Expand Down
Loading