Skip to content
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

Perform the whole sync (pull + push) within a single transaction #16

Open
n1k0 opened this issue Jun 17, 2015 · 6 comments

Comments

@n1k0
Copy link
Contributor

commented Jun 17, 2015

So we can abort the whole import process on error, avoiding leaving the db in an inconsistent state.

@n1k0 n1k0 modified the milestone: 1.0.0 Jul 15, 2015

@michielbdejong

This comment has been minimized.

Copy link
Contributor

commented Aug 12, 2015

Not sure if this is relevant here, but just wanted to mention how important batching together writes to IndexedDB is, for performance. This is also a topic in localForage, we are trying to do automatic batching of writes there. The basic principle of that is:

  • you add or update one record -> it starts updating IndexedDB
  • you add or update more records -> it queues them up until IndexedDB is idle
  • the first transaction completes -> it creates a new batch transaction from everything that was added to the queue in the meantime
@n1k0

This comment has been minimized.

Copy link
Contributor Author

commented Sep 7, 2015

After a quick discussion with @ametaireau, here's the current state of our thinking on this:

  • Having transactions would provide a neat way to cancel local persistence operations;
  • It would also improve performances on large batches of persistence operations;
  • localStorage adapter would need a specific manually implemented solution to cover all the use cases;
  • We should heavily challenge if we actually need this feature: for now, nobody has explicitly expressed a need for it;
  • We should also discuss this with @leplatrem which I remember was having opinions on the matter.

Thoughts?

@almet almet added the question label Sep 7, 2015

@almet almet changed the title Perform batch local updates within a single transaction Perform batch local updates within a single transaction? Sep 7, 2015

@n1k0 n1k0 added this to the 2.0.0 milestone Sep 15, 2015

n1k0 added a commit that referenced this issue Jan 27, 2016
Fixes #177: Add support for transactions at the adapter level.
This patch was paired by @leplatrem and @n1k0.

Also fixes #172, fixes #93, refs #16.
n1k0 added a commit that referenced this issue Jan 28, 2016
Fixes #177: Add support for transactions at the adapter level.
This patch was paired by @leplatrem and @n1k0.

Also fixes #172, fixes #93, refs #16.
@n1k0

This comment has been minimized.

Copy link
Contributor Author

commented Jan 28, 2016

#303 has now landed and is a huge step towards this goal.

@leplatrem leplatrem changed the title Perform batch local updates within a single transaction? Perform the whole sync (pull + push) within a single transaction Feb 2, 2016

@leplatrem leplatrem added enhancement and removed question labels Feb 2, 2016

@leplatrem leplatrem removed this from the 2.0.0 milestone Feb 2, 2016

@leplatrem

This comment has been minimized.

Copy link
Contributor

commented Jul 5, 2017

When https://bugzilla.mozilla.org/show_bug.cgi?id=1193394 will land in Firefox, it will be possible to have IndexedDB transactions across promises in Chrome+Firefox.

@leplatrem

This comment has been minimized.

Copy link
Contributor

commented Mar 7, 2018

That's it! It landed!

Firefox 60 supports promises within IndexedDB transactions \o/

There is a lot of things that could be simplified, quite a huge amount of work.

@michielbdejong

This comment has been minimized.

Copy link
Contributor

commented Mar 7, 2018

Wow! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.