RE: [IndexedDB] IDBRequest.abort on writing requests

>From my perspective cancelling is not something that happens that often, and when it happens it's probably ok to cancel the whole transaction. If we can spec abort() in the transaction object such that it try to cancel all pending operations and then rollback any work that has been done so far, then we probably don't need abort on individual operations (with the added value that it's uniform across read and write operations).

-pablo

From: public-webapps-request@w3.org [mailto:public-webapps-request@w3.org] On Behalf Of Jeremy Orlow
Sent: Wednesday, July 14, 2010 1:57 AM

On Wed, Jul 14, 2010 at 9:14 AM, Jonas Sicking <jonas@sicking.cc> wrote:
On Wed, Jul 14, 2010 at 1:02 AM, Jeremy Orlow <jorlow@chromium.org> wrote:
> On Wed, Jul 14, 2010 at 8:53 AM, Jonas Sicking <jonas@sicking.cc> wrote:
>>
>> On Tue, Jul 13, 2010 at 11:33 PM, Jeremy Orlow <jorlow@chromium.org>
>> wrote:
>> > On Wed, Jul 14, 2010 at 7:28 AM, Jonas Sicking <jonas@sicking.cc> wrote:
>> >>
>> >> On Tue, Jul 13, 2010 at 11:12 PM, Jeremy Orlow <jorlow@chromium.org>
>> >> wrote:
>> >> > On Tue, Jul 13, 2010 at 9:41 PM, Jonas Sicking <jonas@sicking.cc>
>> >> > wrote:
>> >> >>
>> >> >> On Tue, Jul 13, 2010 at 1:17 PM, Jeremy Orlow <jorlow@chromium.org>
>> >> >> wrote:
>> >> >> > On Tue, Jul 13, 2010 at 8:25 PM, Jonas Sicking <jonas@sicking.cc>
>> >> >> > wrote:
>> >> >> >>
>> >> >> >> Hi All,
>> >> >> >>
>> >> >> >> Sorry if this is something that I've brought up before. I know I
>> >> >> >> meant
>> >> >> >> to bring this up in the past, but I couldn't find any actual
>> >> >> >> emails.
>> >> >> >>
>> >> >> >> One thing that we discussed while implementing IndexedDB was what
>> >> >> >> to
>> >> >> >> do for IDBRequest.abort() or "writing" requests. For example on
>> >> >> >> the
>> >> >> >> request object returned from IDBObjectStore.remove() or
>> >> >> >> IDBCursor.update().
>> >> >> >>
>> >> >> >> Ideal would of course be if it would cancel the write operation,
>> >> >> >> however this isn't always possible. If the call to .abort() comes
>> >> >> >> after the write operation has already executed in the database,
>> >> >> >> but
>> >> >> >> before the 'success' event has had a chance to fire. What's worse
>> >> >> >> is
>> >> >> >> that other write operations might already have been performed on
>> >> >> >> top
>> >> >> >> of the aborted request. Consider for example the following code:
>> >> >> >>
>> >> >> >> req1 = myObjectStore.remove(12);
>> >> >> >> req2 = myObjectStore.add({ id: 12, name: "Benny Andersson" });
>> >> >> >> .... do other stuff ....
>> >> >> >> req1.abort();
>> >> >> >>
>> >> >> >> In this case, even if the database supported aborting a specific
>> >> >> >> operation, it's very hard to say what the correct thing to do
>> >> >> >> with
>> >> >> >> operations performed after it. As far as I know, databases
>> >> >> >> generally
>> >> >> >> don't support rolling back a given operation, only rolling back
>> >> >> >> to a
>> >> >> >> specific point, i.e. rolling back a given operation and all
>> >> >> >> operations
>> >> >> >> performed after it.
>> >> >> >>
>> >> >> >> We could say that abort() signals some sort of error if the
>> >> >> >> operation
>> >> >> >> has already been performed in the database, however that makes
>> >> >> >> abort()
>> >> >> >> very racy.
>> >> >> >>
>> >> >> >> Instead we concluded that the best thing to do was to specify
>> >> >> >> that
>> >> >> >> IDBRequest.abort() should throw if called on a modifying request.
>> >> >> >> If
>> >> >> >> this sounds good I'll make this change to the spec.
>> >> >> >
>> >> >> > I'd be fine with that.
>> >> >> > Or we could remove abort all together.  I can't really think of
>> >> >> > what
>> >> >> > types
>> >> >> > of operations you'd really want to abort until (at least) we have
>> >> >> > some
>> >> >> > sort
>> >> >> > of join language or other mechanism to do really expensive
>> >> >> > read-only
>> >> >> > calls.
>> >> >>
>> >> >> I think there are expensive-ish read-only calls. Indexes are
>> >> >> effectively a join mechanism since you'll hit one b-tree to do the
>> >> >> index lookup, and then a second b-tree to look up the full object in
>> >> >> the objectStore.
>> >> >
>> >> > But each individual call (the scope of canceling an IDBRequest) is
>> >> > pretty
>> >> > short.
>> >> >
>> >> >>
>> >> >> I don't really feel strongly either way. I think abort() isn't too
>> >> >> hard to implement, but also doesn't provide a ton of value. At least
>> >> >> not, like you say, until we add expensive calls like getAll or
>> >> >> multi-step joins.
>> >> >
>> >> > I agree that when we look at adding such calls we may want to add an
>> >> > abort
>> >> > on just IDBRequest, but until then I don't think it's a very useful
>> >> > feature.
>> >> >  And being easy to add is not a good reason to lock ourselves into
>> >> > a particular design in the future.  I think we should remove it until
>> >> > there's a good reason for it to exist.
>> >> >
>> >> >>
>> >> >> > Or we could take abort off IDBRequest and instead put a rollback
>> >> >> > on
>> >> >> > transactions (and not do the modify limitation).
>> >> >>
>> >> >> I definitely think we should have IDBTransaction.abort() no matter
>> >> >> what. And that should allow rolling back write operations.
>> >> >
>> >> > Agreed.  In which case it seems as though being able to abort
>> >> > individual
>> >> > operations isn't that important...especially given what we just
>> >> > talked
>> >> > about
>> >> > above.
>> >> > So can we just get rid of abort() on IDBRequest?
>> >>
>> >> I don't feel strongly either way. We'll probably keep them in the
>> >> mozilla implementation since we have experimental
>> >> objectStore.getAll(key) and index.getAllObjects(key) implementations,
>> >> which both probably count as long-running.
>> >
>> > Sounds good.  I'll file a bug to remove them from the spec then (and put
>> > a
>> > note that if we do have getAll* we should re-add it).
>>
>> Actually, I thought of another situation where we currently could have
>> long-running read requests.
>>
>> One implementation strategy for transactions is to not wait for all
>> objectStores to become available before starting to execute requests.
>> Instead you start locking objectStores in some implementation defined,
>> but consistent, order and as soon as you've locked the objectStore at
>> which the next pending request is placed, you perform that request. As
>> long as the order in which the implementation locks the objectStores
>> is the same for all transactions, there is no risk of deadlocks.
>>
>> Consider for example the following code:
>>
>> trans = db.transaction(["foo", "bar"]);
>> trans.objectStore("bar").get(12).onsuccess = function(e) {
>>  trans.objectStore("foo").get(e.result.parentId).onsuccess = ...;
>> }
>>
>> If the implementation decided to lock objectStores in alphabetical
>> order, it could perform the .get() request as soon as it had locked
>> the "bar" objectStore.
>>
>> Note that all of this is possible without any changes to the spec. It
>> still fulfills all requirements set out by the spec, such as never
>> resulting in deadlocks, and performing requests in the order they were
>> made. It's essentially completely transparent to the page, it just
>> results in that separate transactions can run more concurrently.
>
> Maybe we should add something to the spec then.  Doing it this way seems
> silly.
Huh? Why? It can significantly improve performance at no added effort
to the author. That doesn't seem silly at all.

First of all, I'm highly skeptical that it can "significantly improve performance".

But beyond that, I guess there are pros and cons.  The pro you mentioned.  The con is that any other transactions that might want to lock the object store exclusively need to wait until the other transaction finishes.  Which might be some time if it's hard to get a lock on bar.  My intuition is that locking aggressively like you suggest would be a net loss to concurrency which is why I made the comment, but I guess I should have better explained my reasoning.  Either way, I don't think we'll be doing this in WebKit.
 
>>
>> In such an implementation, a simple objectStore.get() request can take
>> quite a while to execute as it might depend on another transaction
>> completely finishing. So in such an implementation, pages would
>> benefit from aggressively aborting requests they no longer care about.
>
> You can still kill the transaction though.
Only if you are fine with rolling back any writes done earlier during
the transaction. And only if you don't need to do any other requests
in the transaction.

This is pretty deep in the land of hypothetical at this point.  I'm positive this will affect someone somewhere, but that doesn't mean it should be a v1 feature.
 
> Anyway, it seems like the only use case for this stuff is an app detecting
> something is taking a while and canceling it.  Honestly, I don't see this
> being too common of a use case.  I mean, the app would need to set timeouts
> and keep track of what's been running for a while and then abort it.  While
> that's not too far fetched, assuming an app will do this in a way that
> doesn't break the rest of the app or mess up transactions does.  All the
> cases I can think of where you'd want to abort a read, it seems like
> aborting the whole transaction isn't that much worse.
> Unless someone can come up with some solid use cases for aborting a single
> read-only IDBRequest and not a whole transaction, I feel strongly we should
> remove it.
Ok.

/ Jonas

Received on Thursday, 15 July 2010 00:33:21 UTC