Skip to content

Don't fail client if lease refresh fails #2307

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

Jump to bottom
Merged
schmidt-sebastian merged 3 commits into from
Oct 30, 2019
Merged

Don't fail client if lease refresh fails #2307

schmidt-sebastian merged 3 commits into from
Oct 30, 2019

Conversation

@schmidt-sebastian
Copy link
Contributor

@schmidt-sebastian schmidt-sebastian commented Oct 30, 2019 •
edited
Loading

This PR:

  • Makes the lease refresh idempotent by moving the primaryStateListener callback outside of the transaction function. Note that it call still get called with primaryStateListener(false), as calling that repeatedly should not have any side effects.
  • Changes transactions to obtain the IndexedDB lease even if they don't already have it.

Addresses #2232

wilhuff
wilhuff approved these changes Oct 30, 2019
View reviewed changes
Copy link
Contributor

@wilhuff wilhuff left a comment

Choose a reason for hiding this comment

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

LGTM

packages/firestore/test/unit/local/indexeddb_persistence.test.ts Outdated

await persistence.runTransaction(
'regain lease',
'readwrite-primary',
Copy link
Contributor

@wilhuff wilhuff Oct 30, 2019

Choose a reason for hiding this comment

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

readwrite-primary-idempotent?

Copy link
Contributor Author

@schmidt-sebastian schmidt-sebastian Oct 30, 2019

Choose a reason for hiding this comment

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

Done.

@wilhuff wilhuff assigned schmidt-sebastian and unassigned wilhuff Oct 30, 2019
@schmidt-sebastian schmidt-sebastian merged commit 68101ee into master Oct 30, 2019
@schmidt-sebastian schmidt-sebastian deleted the mrschmidt/leaserefresh branch October 30, 2019 22:55
@hsubox76 hsubox76 added this to the 7.2.3 milestone Oct 30, 2019
@schmidt-sebastian schmidt-sebastian mentioned this pull request Oct 31, 2019
Closed
@firebase firebase locked and limited conversation to collaborators Nov 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@wilhuff wilhuff wilhuff approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@schmidt-sebastian schmidt-sebastian

Labels

api: firestore

Projects

None yet

Milestone

7.2.3

Development

Successfully merging this pull request may close these issues.

3 participants

@schmidt-sebastian @wilhuff @hsubox76