Skip to content

Mark mostly readonly calls as idempotent #2252

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.

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 10 commits into from
Oct 10, 2019

Conversation

schmidt-sebastian
Copy link
Contributor

This PR marks the simple callsites of IndexedDbPersistence.runTransaction() as idempotent.

Prerequisite was to make 'readonly-primary' and friends available via the public Persistence interface.

@@ -273,7 +273,16 @@ export class SimpleDb {
objectStores
);
try {
// TODO(schmidt-sebastian): Remove this code/comment or find a way to
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I left this here for now to make my development process easier. This actually caught an issue during development, when I marked getNewDocumentChanges as idempotent. It is not yet since it modifies internal state that is used during execution.

@schmidt-sebastian schmidt-sebastian changed the title Mark (most) readonly calls as idempotent Mark mostly readonly calls as idempotent Oct 9, 2019
@schmidt-sebastian schmidt-sebastian changed the base branch from mrschmidt/retry to mrschmidt/idempotent October 10, 2019 18:45
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

@wilhuff wilhuff assigned schmidt-sebastian and unassigned wilhuff Oct 10, 2019
@schmidt-sebastian schmidt-sebastian merged commit fd66c1a into mrschmidt/idempotent Oct 10, 2019
schmidt-sebastian added a commit that referenced this pull request Oct 10, 2019
@schmidt-sebastian schmidt-sebastian deleted the mrschmidt/moresimplecalls branch October 10, 2019 22:54
hsubox76 pushed a commit that referenced this pull request Oct 15, 2019
* Add transaction retries (#2250)

* Marking SimpleDb calls as idempotent (#2251)

* Mark mostly readonly calls as idempotent (#2252)

* Fix test failure (#2256)

* Make handleUserChange idempotent (#2257)

* Temporarily disable CountingQueryEngine tests (#2258)

*  Improve test hack (#2259)

* Improve test hack

* Comment in test hack

* Make getNewDocumentChanges() idempotent (#2255)

* Add onCommitted listeners for transactions (#2265)

* Fix build

* Fix Lint

* Make applyRemoteEvent idempotent (#2263)

* Make notifyLocalViewChanges idempotent (#2268)

* Make releaseQuery idempotent (#2266)

* Mark acknowledgeBatch and rejectBatch idempotent (#2269)

* idempotent `allocateQuery` and `notifyLocalViewChanges` (#2264)

* Mark collectGarbage idempotent (#2267)

* Idempotency: Address TODOs, add Changelog (#2270)
@firebase firebase locked and limited conversation to collaborators Nov 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants