Repository navigation
Proposal: Add explicit IDBTransaction.commit() (was: add "writeonly" mode) #234
Description
Forked off from #34 and previous mentioned on the old wiki, but formalizing it a bit:
- Introduce a new transaction mode, "writeonly"
- If a transaction's mode is "writeonly":
- any IDBObjectStore or IDBIndex operation other than
put()oradd()will throwNotSupportedError put()andadd()return null (return type changed toIDBRequest?)success/errorevents not fired (i.e. don't bubble up to the transaction/connection)- a failed operation (usually, conflicting
add()) still causes the transaction to abort complete/abortevents are still fired against the transaction
- any IDBObjectStore or IDBIndex operation other than
There is probably additional spec text required for clarity, but effectively since the transaction can no longer become active after the cleanup transactions steps run when the task ends, the transaction can attempt to commit immediately; it needs to wait until all of the put()s/add()s finish. The transaction lifetime steps are still a bit wishy-washy; I didn't completely revise those as part of the big 2.0 rework of the spec.
Activity
inexorabletash commentedon Apr 10, 2018
The big use case for this is performance:
inexorabletash commentedon Apr 11, 2018
@aliams, @wanderview, @beidson - any initial feedback (not official opinions) on this idea? A non-starter, or something that would be worth the Chrome team prototyping?
wanderview commentedon Apr 11, 2018
301 @asutherland
inexorabletash commentedon Apr 13, 2018
FYI, I sketched out another idea a few years ago - adding an explicit
commit()method to the transaction that would immediately and permanently deactivate it to prevent further requests from being filed. gist - this might be more or less appealing than a distinct mode. The down side is that success/error events are still fired against each request even if the transaction is not activated on their callbacks, and that's still unnecessary overhead in some cases.asutherland commentedon Apr 16, 2018
commit()as described at https://gist.github.com/inexorabletash/d55a6669a040e92e47c6 seems more appealing and versatile. Being unable to benefit from the optimization if you want your transaction to first assert something about the state of the database with a read limits its utility. And having a mode where events that would normally fire no longer fire and calls that would normally never throw suddenly throw doesn't help from a complexity perspective for an API many view as already overly complex.It would of course be useful to have an idea that a commit() will be coming before the requests start getting issued. Perhaps commit() could be paired with something like
beginBatch()that could hint to the implementation that it is allowed to buffer all subsequent request responses until the subsequentcommit()or an also possibleflush()is processed.At least in Gecko's implementation, this would allow us to greatly optimize against janking as long as no listeners were added to the requests. Obviously, if 100 requests are dispatched and each has a listener, dispatching all 100 in a single go just before the complete/abort happens is potentially much worse than the status quo. That said, beginBatch should still allow for implementation latitude so that the 100 can be spread out over multiple logical tasks/events to avoid dominating the event loop.
inexorabletash commentedon Apr 17, 2018
Thanks @asutherland ! I'll pursue
commit(). From our previous testing, it seems like it's a perf benefit even if the individual request events are fired, and I think it satisfies the use cases for bringing this up again (fire-and-forget at shutdown/suspend)beginBatch()seems like it could be layered on later.nolanlawson commentedon May 22, 2018
Given that the main benefit here is performance, I agree
commit()may be a more direct way of tackling this problem. Adding a special mode where you can only write and can't respond to individual constraint errors may be of limited value, although it is perfect for initial database setup scenarios such as #224.If the core performance issue is that creating too many callbacks in the form of multiple
put()s causes overhead, I also thinkputAll()(returning some kind of array of results/errors) may also be an interesting option here.inexorabletash commentedon Jun 18, 2018
Early draft PR added (#242) - any additional eyeballs on that would be welcome.
[-]Proposal: Add "writeonly" transaction mode[/-][+]Proposal: Add explicit IDBTransaction.commit() (was: add "writeonly" mode)[/+]asutherland commentedon Oct 6, 2018
Speculative Firefox/Gecko bug tracking implementing this is https://bugzilla.mozilla.org/show_bug.cgi?id=1497007 but we're not actually ready to implement at this time.
Add IDBTransaction commit() method (#242)
inexorabletash commentedon Jan 23, 2019
Merged PR the spec. I'm not assuming it's perfect, but let's iterate on the spec with additional bugs, so let's close this.
Thanks to everyone for the discussion!
Add IDBTransaction commit() method (#242)