Re: [IndexedDB] IDBRequest.abort on writing requests

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).

J