RE: [IndexedDB] New version API checked in

On Tuesday, September 27, 2011 5:40 PM, Jonas Sicking wrote:
> On Tue, Sep 27, 2011 at 2:41 PM, Israel Hilerio <israelh@microsoft.com>
> wrote:
> > On Wednesday, September 21, 2011 7:11 PM, Jonas Sicking wrote:
> >> On Mon, Sep 12, 2011 at 1:56 PM, Israel Hilerio
> >> <israelh@microsoft.com>
> >> wrote:
> >> > On Sunday, September 04, 2011 3:33 AM, Jonas Sicking wrote:
> >> >> Hi Everyone,
> >> >>
> >> >> I finally got around to updating the IndexedDB spec to the new
> >> >> version
> >> API!
> >> >> Definitely a non-trivial change, so I'd love for people to have a
> >> >> look at it to see if I messed anything up.
> >> >>
> >> >> I decided to go with the name "upgradeneeded" for the event fired
> >> >> when a version upgrade is needed. I'm not terribly happy with the
> >> >> name, but it does feel descriptive.
> >> >>
> >> >> I also went with integers (long long) for the version number. The
> >> >> reason was that I wanted to avoid people passing strings like "1.10"
> >> >> to the API since JS will automatically and silently convert that
> >> >> to the number 1.1. This could lead to confusion since people might
> >> >> think that "1.10" is a higher version than "1.9".
> >> >>
> >> >>
> >> >> There were a few issues that came up during editing, mostly
> >> >> related to edge cases that doesn't matter terribly much:
> >> >>
> >> >> * What to do if multiple different versions are opened at the same
> time.
> >> >> Consider the following scenario. A database with name "mydb" has
> >> >> version 3. The following two calls happen almost at the same time:
> >> >>
> >> >> req1 = indexedDB.open("mydb", 4);
> >> >> and
> >> >> req2 = indexedDB.open("mydb", 5);
> >> >>
> >> >> It's clear that we should here fire a "upgradeneeded" event on
> >> >> req2 and let it run a VERSION_CHANGE transaction to upgrade the
> >> >> database to
> >> version 5.
> >> >> There are however two possible things we could do to req1.
> >> >>
> >> >> A) Immediately fail by firing an "error" event on req1.
> >> >> B) Wait for req2 to attempt to upgrade the database version to 5.
> >> >> Only once that succeeds fail req1 by firing an "error" event at it.
> >> >> If req2 failed to upgrade the database (due to a aborted
> >> >> transaction), then fire a "upgradeneeded" transaction on req1.
> >> >>
> >> >> This seems like a really rare edge case and I don't think it
> >> >> matters much what we do. I chose to go with option B since it
> >> >> results in the least amount of errors and it doesn't seem
> >> >> particularly important to optimize for failing open calls quickly in this
> rare situation.
> >> >>
> >> >> I don't think it matters much what we choose here. I think it's
> >> >> very unlikely to matter in any real-world scenarios. I might even
> >> >> be fine with letting implementations choose to go with either solution
> here.
> >> >>
> >> >>
> >> >> * What to do if indexedDB.open is called while a VERSION_CHANGE
> >> >> transaction is pending, but the new call is for a higher version.
> >> >> Consider the following scenario:
> >> >>
> >> >> 1. A database with name "mydb" and version 1 is currently open in tab
> 1.
> >> >> 2. Someone calls indexedDB.open("mydb", 2) in tab 2.
> >> >> 3. The indexedDB implementation fires a "versionchange" event on
> >> >> the open connection in tab 1 and waits for it to close. The
> >> >> newVersion property of the event is set to 2.
> >> >> 4. Someone calls indexedDB.open("mydb", 3) in tab 3.
> >> >>
> >> >> At this point there are at least two options:
> >> >> A) Simply let the call in step 4 wait for the call in step 2 to
> >> >> finish. Only after it has finished will we fire new events to
> >> >> attempt an upgrade to version 3
> >> >> B) Stall the upgrade two version 2 and instead start attempting an
> >> >> upgrade to version 3. I.e. fire a new "versionchange" event on the
> >> >> open connection in tab 1 (now with newVersion set to 3), and once
> >> >> that connection is closed, fire a "upgradeneeded" event and start
> >> >> a
> >> VERSION_CHANGE transaction in tab 3.
> >> >>
> >> >> Option A basically makes us behave as if the call in step 4
> >> >> happened after the VERSION_CHANGE transaction for the call in step
> >> >> 2 had started. Option B almost makes us behave as if the calls in
> >> >> step 2 and step 4 had happened at the same time (with the
> >> >> exception that two "versionchange" events are fired).
> >> >>
> >> >> As with the previous issue I don't think it matters much what we
> >> >> choose here. I think it's very unlikely to matter in any
> >> >> real-world scenarios. I might even be fine with letting
> >> >> implementations choose to go with either solution here.
> >> >>
> >> >>
> >> >> * What to do if db.close() is called during the VERSION_CHANGE
> >> >> transaction Calling db.close() during a VERSION_CHANGE transaction
> >> >> somewhat similar to calling transaction.abort(). At least in the
> >> >> sense that in neither case does it make sense for
> >> >> IDBFactorySync.open/IDBFactory.open to complete successfully. I.e.
> >> >> it would seem strange to let IDBFactorySync.open return a closed
> >> >> database, or to fire a "success" event on the request returned by
> >> IDBFactory.open and then deliver a closed database.
> >> >>
> >> >> We could make db.close() throw an exception in this case, but that
> >> >> seems like a odd behavior for db.close() compared to how it
> >> >> usually interacts with running transactions (i.e. it usually lets them
> finish).
> >> >>
> >> >> I'm instead leaning towards letting the VERSION_CHANGE transaction
> >> >> continue running, but make IDBFactorySync.open throw an exception
> >> >> and fire an "error" event on the request returned from
> IDBFactory.open.
> >> >>
> >> >> In fact, after thinking about this some more I checked in a change
> >> >> to the spec to make it define that behavior. The main problem was
> >> >> that we don't have a really good error for this situation. I
> >> >> decided to return a ABORT_ERR error, but if anyone has other
> >> >> suggestions, or think we should use should use a dedicated error, I'm
> fine with that too.
> >> >>
> >> >> If people feel that we should use another solution, please do
> >> >> speak up. It's easy to back out my patch :-)
> >> >>
> >> >> / Jonas
> >> >
> >> > Thanks for making these changes to the spec so quickly.  Here is
> >> > some
> >> feedback:
> >> >
> >> > * In order to support Section 4.1 step #4,  it seems we want to
> >> > follow the
> >> same advance number logic and not allow any zero or negative numbers
> >> to be defined by the developer for the version.  This will allow this
> >> step to happen without any issues.  Otherwise, we will be creating a
> >> default db with version "0" and the value would be negative or "0"
> >> which would cause step
> >> #5 to execute.  It seems we want to throw an exception with a
> >> NON_TRANSIENT_ERR if the developer specifies any zero or negative
> >> version values (similar to advance).
> >>
> >> Agreed. We can do that using the new [EnforceRange] WebIDL syntax. We
> >> should use that for cursor.advance too.
> >
> > This makes sense.  I believe this will change the current signatures to:
> > *IDBFactory: IDBOpenDBRequest open (DOMString name, [EnforceRange]
> > unsigned long long version);
> > *IDBCursor:  void       advance ([EnforceRange] unsigned long count)
> > raises (IDBDatabaseException);
> >
> > We will also throw an exception if the value for version or count equals 0.
> > Correct?
> 
> Yup.
> 
> >> > * We also believe that the version number should be optional.  We
> >> > see
> >> scenarios where the developer would want to use the latest db and not
> >> have to worry about the version number.  If a version number is not
> >> specified and the db doesn't exists, we will throw a NOT_FOUND_ERR.
> >> We only way to create a new db when the version is specified.  This
> >> will prevent the onupgradeneeded event handler from being called when
> >> there is no version specified.
> >>
> >> This was a tough one. After thinking about it for some time, and
> >> discussing with others, I agree that we need this. One use-case is
> >> for wrapping libraries which doesn't care about versions but instead
> >> lazily instantiates object stores and indexes as needed. One example is
> here:
> >>
> >> http://nparashuram.com/IndexedDB/jquery/index.html
> >>
> >> Such a library would want to open the database in its current
> >> version, but likely doesn't know what version the database has.
> >>
> >> There is however a edge case that we need to deal with somehow. What
> >> do we do if someone calls .open and doesn't specify a version, but
> >> the database doesn't yet exist? I could see a few options:
> >>
> >> 1. Fire the "success" event (and thus not start a VERSION_CHANGE
> >> transaction) and provide a database with version 0 which doesn't have
> >> any object stores.
> >> 2. Fire the "success" event (and thus not start a VERSION_CHANGE
> >> transaction) and set .result to null.
> >> 3. Fire a "error" event and set the error code of the request to some
> >> new error.
> >> 4. Fire a "upgradeneeded" event (and thus *do* start a VERSION_CHANGE
> >> transaction) and provide a database with version 1 which doesn't yet
> >> have any object stores.
> >>
> >> Option 1 is somewhat useless since the page won't be able to do
> >> anything with such a database. However arguably option 2 and 3 isn't
> >> much more useful.
> >>
> >> Option 4 scares me a little since it's somewhat inconsistent. I.e. in
> >> all other instances, not specifying a version never results in
> >> "upgradeneeded" being fired.
> >>
> >> I'm leaning towards 2 or 4, but I'm very interested in hearing what
> >> others think. Or if I've missed some additional option.
> >
> > We feel that developers should be specific when they want to create a
> database and therefore should provide a version.  Adopting this philosophy
> would allow us to determine developers didn't intend for a database to be
> created when there is no version.  It will also signal libraries that the
> database doesn't exists.  Therefore, option 4 doesn't seem ideal.
> >
> > We believe that Option 2 could be misleading and will introduce
> inconsistencies on how the onsuccess handler is coded.  Devs would have to
> do additional checks inside their onsuccess handler (.result != null) to
> validate a successful db open.
> >
> > Therefore, we would like to vote for option 3.
> 
> I was actually just going to write and say that I was leaning towards option 4
> :-)
> 
> My thinking was this: The main use-case I can think of for being able to not
> use a version when opening a database is for libraries that want to do a "lazy
> version upgrade" approach like the library I pointed out. Such a library would
> generally want to open the database in whatever version it currently has. If it
> needs to create a new objectStore it would close the database and open it in
> a version one higher than the previous version.
> 
> For such a library, the best behavior is for a upgradeneeded event to fire if
> the database doesn't exist. That way it can immediately create the object
> store it wants to use and then commit the VERSION_CHANGE transaction.
> 
> For options 1-3 it would first call open with no version, if that fails due to the
> database not existing, call open again with version one. However at this point
> the database might already exist in version one but the desired object store
> not exist (due to the database being created in another tab). So it would then
> call open again with version
> 2 etc.
> 
> So at least in this scenario it seems better, though not required, to go with
> option 4.

The explanation helped!  After discussing it with my team, we're okay with option 4.

> >> > * We don't believe developers shouldn't be allowed to start a new
> >> transaction from the onabort handler associated with a
> VERSION_CHANGE
> >> transaction.  The reason is that we don't consider the db to be in a
> >> valid state because the open operation associated with the db handle
> >> failed to execute properly.
> >>
> >> Agreed. My intent was that step 8 in the steps for opening a database
> >> requires this (http://dvcs.w3.org/hg/IndexedDB/raw-
> >> file/tip/Overview.html#opening)
> >>
> >> > * Similarly, if the VERSION_CHANGE transaction fails to commit (db
> >> > failure
> >> or abort method is called) we believe the db version should be set to
> >> undefined on the onabort handler.  We don't see the point of going
> >> back to the server to get the last committed version number because
> >> the intent of the developer was to operate on the new schema which
> >> failed to commit.  It seems that if they want to use the latest
> >> schema they could open the db without specifying a version.
> >>
> >> I don't really feel strongly here, but you'll already have the last
> >> committed version since that is what the "oldVersion" property on the
> "upgradeneeded"
> >> event contains this value.
> >
> > By setting the current version to undefined in a db transaction failure all
> we're saying is that the current upgrade operation failed and the requested
> version wasn't set.  Devs can still go ahead and obtain the last good version
> of their database from the IDBVersionChangeEvent as you pointed out.
> 
> That's fine with me.
> 
> / Jonas

Do you want Eliot and I to update the spec or will you do that?

Israel

Received on Wednesday, 28 September 2011 22:56:31 UTC