Re: [IndexedDB] Atomic schema changes

On Fri, Jun 25, 2010 at 2:43 AM, Jeremy Orlow <jorlow@chromium.org> wrote:
>> > I'm OK with making createObjectStore/createIndex synchronous.  It would
>> > definitely make such code cleaner and I don't see a major downside, but
>> > at
>> > the same time I feel like this API is starting to get kind of ad-hoc and
>> > unintuitive to a new user.  Having the create and the remove functions
>> > completely different and in different places seems weird.
>> > So I agree with either A or leaving things as originally proposed by
>> > Jonas
>> > in "[IndexDB] Proposal for async API changes".
>>
>> Well, there's no reason not to make the same changes to
>> removeObjectStore/removeIndex. Though the more I think with this
>> design createObjectStore and removeObjectStore can stay on IDBDatabase
>> and simply throw if called outside the appropriate transaction
>> callback.
>>
>> Here is the revised proposal:
>>
>> interface IDBDatabase {
>>    ...
>>    IDBObjectStore createObjectStore (in DOMString name, in optional
>> DOMString keyPath, in optional boolean autoIncrement);
>>    void removeObjectStore (in DOMString storeName);
>>    ...
>> };
>>
>> interface IDBObjectStore {
>>    ...
>>    IDBIndex createIndex (in DOMString name, in DOMString keyPath, in
>> optional boolean unique);
>>    IDBRequest removeIndex (in DOMString indexName);
>>    ...
>> };
>>
>> Where createObjecStore/createIndex throws if a objectStore or index of
>> the given name already exists, if the keyPath has an invalid syntax,
>> or if the function is called when not inside a version-change
>> transaction callback. And removeObjectStore/removeIndex throws if the
>> objectStore or index doesn't exist or if the function is called when
>> not inside a version-change transaction callback.
>>
>> Throwing if not inside the right type of callback isn't a super clean
>> solution, but is really not that different from how add/put throws if
>> called when not inside a READ_WRITE transaction.
>
> Just to be clear, no async function (one that returns an IDBRequest) should
> _ever_ throw.  (They should only call onerror.)  I assume you didn't mean
> "add/put throws" literally?

Well, there's no reason we couldn't make add/put throw if called from
inside the wrong type of transaction. I guess I don't feel strongly
about it, but it seems to me that calling put from inside a READ_ONLY
transaction is a much bigger mistake than writing a duplicate key or
running out of disc space. Additionally it's a situation where we
synchronously know that there is a bug in the program.

>> > But, I feel pretty strongly that a setVersion/schema change transaction
>> > should not simply kill off anything else currently running.  The reason
>> > is
>> > that it's not hard for apps to recover from a connection failing, but it
>> > is
>> > hard to handle a transaction failing in an unpredictable way.
>> >  Especially
>> > static transactions (which should rarely fail committing since
>> > serialization
>> > can be guaranteed before the transaction starts).
>>
>> That might be a good idea for v1. I was planning on doing a separate
>> thread for setVersion, but maybe it's tied enough to the topic of
>> schema changes that it makes sense to bring up here.
>>
>> What I suggest is that when setVersion is called, we fire
>> 'versionchange' event on all other open IDBDatabase objects. This
>> event contains information of what the desired new version number is.
>> If no other IDBDatabase objects are open for the specific database, no
>> 'versionchange' events are fired. This allows pages using the old
>> schema version to automatically save any pending data (for example any
>> draft emails) and display UI to the user suggesting that the tab be
>> closed. If possible without dataloss, the tab could even reload itself
>> to automatically load an updated version of the page which uses the
>> new schema version.
>>
>> The 'versionchange' event would use an interface like
>>
>> interface IDBVersionEvent : IDBEvent {
>>  readonly attribute string version;
>> };
>
> First of all, what I was originally advocating (sorry for not being clear)
> is that we should kill the database connection but not until all active
> transactions are complete.  Though we should probably block new transactions
> from starting once setVersion is called.
> But I really like your versionchange event idea regardless.  I agree that
> letting the app sync any data that might be in memory (for example, a draft
> email) is important.  And the idea that the web app could refresh itself (or
> download new application code or something) seems pretty cool and useful.
>  I'm fine with it firing on all frames except the one that initiated (like
> storage events).  If we go with the "kill the connection once all active
> transactions are done and block new ones from starting", we'd want to start
> the blocking only after all versionchange events have finished.
> The main reason that I like the idea of not stating the version change until
> all active connections have closed is that not all apps will handle
> versionchange.  My original idea was that we should just break such web apps
> and let the user refresh, but now that you've pointed out the potential for
> data loss I'm not sure that's an option.  Savvy web apps can kill all
> existing database connections when they get the versionchange and thus avoid
> stalling things.
>
>> Additionally, if there are open IDBDatabase objects, we fire a
>> 'blocked' event at the IDBRequest object returned from the setVersion
>> call. This allows the page to display UI to the user asking the user
>> to close all other relevant tabs.
>>
>> Once all other IDBDatabase objects are closed, we create a transaction
>> and fire the normal 'success' event.
>>
>> While there are pending version change requests, no success events are
>> fired for calls to IDBFactory.open for the relevant database.
>>
>> We might want to support forcefully killing off open IDBDatabase
>> objects in the future, but I think that can wait until after v1.
>
> Really?  I can't see an app like gmail ever asking users to close tabs.  I
> bet they'd sooner run all the application logic in an iframe and navigate it
> away when doing a schema change.
> And I don't see many people correctly implementing a blocked event handler.
>  If anything, it should be an error code.
> It doesn't seem that hard to have an explicit way to tell the database
> explicitly "OK, I'm done".  Or, at very least, we could make it so that when
> there's an existing setVersion happening, all new connection requests stall.
>  That way all pages can reload themselves but they won't connect to the
> database until the upgrade is complete.
> But really...all of this is really hacky.  I'm starting to wonder if we
> should just kill the database connections on a setVersion as I originally
> tried to suggest.

I'm pretty concerned though that sites will need to take asynchronous
actions in order to save all required data. While gmail happily
automatically saves every few minutes, and presumably could
immediately do so upon a 'versionchange' event, I don't think all
editors are willing t. For example many editors ask the user if they
want to save the current changes when they are closed, in order to not
overwrite correct data.

Additionally, there is always the risk that developers will forget to
use a versionchange event handler to protect their data. I think a
good design principal is that if sites do the minimal amount of work,
that should default to safe behavior.

I do realize that not all applications are willing to do the "please
close all other tabs" UI thing. But for those we would provide enough
tools do something better. If we add a IDBDatabase.close() function
then applications that can easily emulate the "force-close open
connections" using that and the versionchange event. And they could
even use that to implement asynchronous data saving for when that is
required.

/ Jonas

Received on Saturday, 26 June 2010 09:10:11 UTC