Re: [IndexedDB] New version API checked in

On Mon, Sep 12, 2011 at 2:37 PM, Israel Hilerio <israelh@microsoft.com> wrote:
> On Monday, September 12, 2011 1:56 PM, Israel Hilerio 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).
>>
>> * 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.
>>
>> * 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.
>>
>> * 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.
>>
>> * It seems that if a user aborts the VERSION_CHANGE transaction, we should
>> call the onerror handler of the open database method.  This will ensure that
>> the onsuccess logic that follows a clean transaction commit will not be
>> executed, possibly generating additional errors.
>>
>> Israel
>>
>
> One more thing, in section 4.8, we probably should set the done flag to true in order to specify that the attributes associated with the request are accessible. We don't believe this should have any side effects on the onsuccess or onerror handlers.

Hmm.. I think you're right. That would be an additional sub-step
before the current sub-steps in step 9?

/ Jonas

Received on Thursday, 22 September 2011 02:14:24 UTC