Skip to content

Ignore primary lease loss in maybeGarbageCollectMultiClientState #2585

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 4 commits into from
Jan 31, 2020
Merged

Ignore primary lease loss in maybeGarbageCollectMultiClientState #2585

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cf7d2b5
Ignore primary lease loss in maybeGarbageCollectMultiClientState
schmidt-sebastian Jan 31, 2020
22555df
[AUTOMATED]: Prettier Code Styling
schmidt-sebastian Jan 31, 2020
bd33328
Remove cyclic dependency
schmidt-sebastian Jan 31, 2020
57bc363
Update indexeddb_persistence.ts
schmidt-sebastian Jan 31, 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
8 changes: 6 additions & 2 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 @@ -72,7 +72,6 @@ import { ReferenceSet } from './reference_set';
import { ClientId } from './shared_client_state';
import { TargetData } from './target_data';
import { SimpleDb, SimpleDbStore, SimpleDbTransaction } from './simple_db';

const LOG_TAG = 'IndexedDbPersistence';

/**
Expand Down Expand Up @@ -489,7 +488,12 @@ export class IndexedDbPersistence implements Persistence {
).next(() => inactive);
});
}
);
).catch(() => {
// Ignore primary lease violations or any other type of error.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's worth adding why it's OK to ignore all the errors here.

Something along the line where you put in PR description: There is no special recovery logic needed as the next primary will run the periodic GC.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

// We don't use `ignoreIfPrimaryLeaseLoss` since we don't want to depend
// on LocalStore.
return [];
});

// Delete potential leftover entries that may continue to mark the
// inactive clients as zombied in LocalStorage.
Expand Down