Re: [IndexedDB] setVersion is cra... is not good

On Wed, Jul 20, 2011 at 1:33 PM, Israel Hilerio <israelh@microsoft.com> wrote:
> On Monday, July 18, 2011 10:50 AM, Jonas Sicking wrote:
>> On Mon, Jul 18, 2011 at 10:20 AM, Israel Hilerio <israelh@microsoft.com>
>> wrote:
>> > On Sunday, July 17, 2011 1:46 PM, Jonas Sicking wrote:
>> >> On Fri, Jul 15, 2011 at 5:55 PM, Israel Hilerio <israelh@microsoft.com>
>> wrote:
>> >> > Jonas,
>> >> >
>> >> > We like the concepts and principles you are proposing.  It provides
>> >> > a more
>> >> cohesive mechanism for enforcing/supporting upgrades and it leverages
>> >> the existing concepts like "onversionchange" and "onblock" handlers
>> >> which we are familiar with.
>> >> >
>> >> > Assuming we end up using: "IDBOpenDBRequest open (in DOMString
>> >> name, in DOMString version);"
>> >> > I would like to verify the side effects:
>> >>
>> >> Is there a reason you're preferring to go with the version as a
>> >> DOMString rather than as a integer as in my second proposal. I think
>> >> using a string leaves a serious problem in that people could end up
>> >> accidentally downgrading a database which could lead to things like
>> >> dataloss. Though more likely is simply bad performance as when the
>> >> downgrade happens, all other windows would if written properly close
>> their connection to the database.
>> >>
>> >> > 1. The new open API will eliminate the need for the setVersion API.
>> >>
>> >> Yes.
>> >>
>> >> > 2. The new IDBOpenDBRequest will eliminate the need for the
>> >> IDBVersionChangeRequest interface.
>> >>
>> >> Currently IDBVersionChangeRequest is used both for setVersion and for
>> >> deleteDatabase. We certainly could use IDBOpenDBRequest for both, but
>> >> onupgrade would never fire when deleteDatabase was called.
>> >>
>> >> Another alternative would be to do something like
>> >>
>> >> interface IDBBlockableRequest : IDBRequest {  attribute Function
>> >> onblocked; }; interface IDBOpenDBRequest : IDBBlockableRequest {
>> >> attribute Function onupgrade; };
>> >>
>> >> Where IDBBlockableRequest is what would be returned from
>> >> deleteDatabase. But that might be a bit over engineered.
>> >>
>> >> I'm fine with just having IDBOpenDBRequest if that is what people prefer.
>> >>
>> >> > 3. Any time the version string in the open API doesn't match the
>> >> > current
>> >> version string in the DB, the onupgrade handler will be called.  We
>> >> probably should use a different name since this will apply to upgrade
>> >> and downgrade (e.g. onversionchange).
>> >>
>> >> This is another reason i'm suggesting using an integer for version
>> >> instead and not having to deal with downgrading.
>> >>
>> >> > 4. After the onupgrade handler is called, the onsuccess handler
>> >> > will be
>> >> automatically called.
>> >>
>> >> Yes. If the transaction created when calling the onupgrade handler is
>> >> successfully committed then the onsuccess handler is called. If,
>> >> however the transaction is aborted. The rules for when a transaction
>> >> is aborted follows the same rules as we have for setversion-transactions
>> today.
>> >>
>> >> > 5. If the onerror handler is called (for the open API IDBRequest),
>> >> > the
>> >> database will be in a closed state.  Devs will have to call open
>> >> again with a different version to re-open the db.
>> >>
>> >> Yes.
>> >>
>> >> > 6. These changes won't affect IDBFactory.deleteDatabase or
>> >> IDBDatabase.close.
>> >>
>> >> Agreed. With possible exception for what type of request to return
>> >> for IDBFactory.deleteDatabase. But the actual functionality remains
>> unchanged.
>> >>
>> >> > 7. The changes will affect the following method and interfaces only:
>> >> IDBFactory.open, IDBDatabase.setVersion, and
>> IDBVersionChangeRequest.
>> >>
>> >> Yes.
>> >>
>> >> > 8. Developers won't be able to update their schemas without
>> >> > changing
>> >> their version string.
>> >>
>> >> Yes.
>> >>
>> >> > Our concern is the destabilization this will introduce in the
>> >> > current spec and
>> >> the amount of time this could take to bake.  If we can scope these
>> >> changes to be minimal and clearly define the areas of the spec that
>> >> are affect that would ease our worries.  What areas of the spec do
>> >> you expect will be affected by this change?  Our goal would be to
>> >> minimize the side effects related to this change. How long before we
>> >> can bake the concept enough to feel comfortable about its possible side
>> effects?
>> >> >
>> >> > From our conversation with developers, they seem to be waiting for
>> >> > the
>> >> IndexedDB spec to be locked before they start using the APIs.  It
>> >> would be awesome if the first version of the spec was locked soon so
>> >> developers can start relying on our APIs to start using the
>> >> functionality. Do you feel this will be the last major change to the spec
>> before we can start to lock it down?
>> >>
>> >> I agree with all of the above. Would love to see agreement on this as
>> >> soon as possible. We'd likely roll out the new API in firefox
>> >> nightlies in a matter of weeks after there is agreement, and in
>> >> released versions as soon as our release schedule allows us (12 weeks or
>> so).
>> >>
>> >> / Jonas
>> >
>> > The reason I picked the string versioning approach was based in
>> conversation I had with Pablo. He mentioned that using a number instead of
>> string is a way of having the system impose policy on how databases are
>> versioned.  One advantage of using strings is that they provide a mechanism
>> to version using tags.  For example, my version could be a comma-separated
>> list of tags that indicate which app features are present in the schema.  In
>> addition, it could be used to help decide compatibility rules (i.e. backwards
>> and forwards compatibility).  The approach seemed more flexible.
>>
>> I definitely agree it's more flexible, but it's also more complex to use since
>> the page has to make sure not to accidentally downgrade the version.
>>
>> Would you have remembered to put the "am I downgrading" check in the
>> example I drafted in the initial email?
>>
>> There's also the problem of that if someone accidentally attempts to open a
>> database with an old version, that will likely result in any other page using
>> the database receiving a "versionchange" event which likely will disable the
>> page until the user reloads it (as generally "versionchange" happens at an
>> upgrade, at which point the page
>> *should* drop all database connections).
>>
>> So using a string also affects the user experience somewhat.
>>
>> > I'm okay keeping IDBVersionChangeRequest for deleteDatabase and
>> IDBOpenDBRequest for the new open method.  This seems cleaner.
>>
>> The more I think about it, the more I like the simplicity of using
>> IDBOpenDBRequest for both, that way we have fewer request interfaces.
>> And looking at the HTML5 DOM it has plenty of onX attributes that will never
>> fire.
>>
>> But I'm totally fine with with going either way!
>>
>> > It would be great to hear Jeremy's opinion on this.
>>
>> Yes!
>>
>> / Jonas
>
> We're okay going with the numeric versioning approach you proposed.

Yay! Awesome!

> However, we have a couple of questions about the versioning scheme:
> * What is the granularity of the numbering scheme (i.e. float, int, unsigned shorts, etc.)?

Either float or long long would work for me. (shorts are obviously a
bad idea since there's a real risk of running out of numbers).

For some reason I had convinced myself that integers were better, but
I can't remember my reasoning any more. Floats seems to provide more
flexibility.

> * Do we need to introduce a new error code (e.g. VESION_ERROR) to catch version problems?

Yes.

> * What happens when the db is at version 1.3 and someone specifies version 1.0?  Are we going to throw an error event or an exception?

Yes, it would make the .open call fail.

For the async API we couldn't throw an exception since it requires IO
to figure out what the current version number is. So failing means
firing an error event. I.e. in your example we'd fire an error event
on the request returned from .open with code set to the new
VERSION_ERROR.

I didn't draft the sync API in my original proposal, but I think it
would look something like this:

interface IDBFactorySync {
  IDBDatabaseSync open(in DOMString name, in float version, in
IDBTransactionCallback callback, in optional unsigned long timeout);
  void deleteDatabase (in DOMString name); // unchanged
}

If opening the database fails IDBFactorySync.open throws an exception,
just like now. If opening succeeds, but the version argument is
smaller than the stored database, it throws an exception with the new
VERSION_ERROR code. If opening succeeds and the version argument is
the same as the stored database, the database is simply returned.

If opening the database succeeds and the version argument is greater
than the stored version, things work a lot like setVersion does today.
I.e. we'd create a new transaction and call the callback argument and
pass it the newly created transaction. If the callback returns
successfully the transaction is committed and a IDBDatabaseSync object
is returned from the open function. If the callback throws, the
database is closed and the exception is propagated out from the .open
function.

> Also, how long and extensive do you expect the spec changes to be?

I should be able to do them during the first half of next week.

/ Jonas

Received on Wednesday, 20 July 2011 21:50:23 UTC