Re: [IndexedDB] Current editor's draft

On Wed, Jul 7, 2010 at 5:57 AM, Jonas Sicking <jonas@sicking.cc> wrote:

> On Tue, Jul 6, 2010 at 9:36 AM, Nikunj Mehta <nikunj@o-micron.com> wrote:
> > Hi folks,
> >
> > There are several unimplemented proposals on strengthening and
> > expanding IndexedDB. The reason I have not implemented them yet is
> > because I am not convinced they are necessary in toto. Here's my
> > attempt at explaining why. I apologize in advance for not responding
> > to individual proposals due to personal time constraints. I will
> > however respond in detail on individual bug reports, e.g., as I did
> > with 9975.
> >
> > I used the current editor's draft asynchronous API to understand where
> > some of the remaining programming difficulties remain. Based on this
> > attempt, I find several areas to strengthen, the most prominent of
> > which is how we use transactions. Another is to add the concept of a
> > catalog as a special kind of object store.
>
> Hi Nikunj,
>
> Thanks for replying! I'm very interested in getting this stuff sorted
> out pretty quickly as almost all other proposals in one way or another
> are affected by how this stuff develops.
>
> > Here are the main areas I propose to address in the editor's spec:
> >
> > 1. It is time to separate the dynamic and static scope transaction
> > creation so that they are asynchronous and synchronous respectively.
>
> I don't really understand what this means. What are dynamic and static
> scope transaction creation? Can you elaborate?
>

This is the difference in the API in my email between openTransaction and
transaction. Dynamic and static scope have been defined in the spec for a
long time.


>
> > 2. Provide a catalog object that can be used to atomically add/remove
> > object stores and indexes as well as modify version.
>
> It seems to me that a catalog object doesn't really provide any
> functionality over the proposal in bug 10052? The advantage that I see
> with the syntax proposal in bug 10052 is that it is simpler.
>
> http://www.w3.org/Bugs/Public/show_bug.cgi?id=10052
>
> Can you elaborate on what the advantages are of catalog objects?
>

To begin with, 10052 shuts down the "users" of the database completely when
only one is changing its structure, i.e., adding or removing an object
store. How can we make it less draconian? Secondly, I don't see how that
approach can produce atomic changes to the database. Thirdly, we shouldn't
need to change version in order to perform database changes. Finally, I am
not sure why you consider the syntax proposal simpler. Note that I am not
averse to the version change event notification.

> 3.  Cursors may produce a null key or a null value. I don't see how
> > this is valid signaling for non-preloaded cursors. I think we need to
> > add a new flag on the cursor to find out if the cursor is exhausted.
>
> Our proposal was that IDBEvent.result would normally contain the
> cursor object, but once the end is reached it returns null. To be
> clear:
>
> When a value is found:
> event.result;   // returns cursor object, never null
> event.result.key;  // returns key, may be null
> event.result.value;  // returns value, may be null
>
> When end is reached:
> event.result;  // returns null
>
>
Got it. I will try out this approach.


>
> > A couple of additional points:
> >
> > 1. I did not see any significant benefits of preloaded cursors in
> > terms of programming ease.
>
> Yes, there seems to be agreement that preloaded cursors should be
> removed. I've removed them from our proposal.
>
> > 2. *_NO_DUPLICATE simplifies programming as well as aids in good
> > performance. I have shown one example that illustrates this.
>
> I'll have to analyze the examples below. My gut instinct though is
> that I agree with you that they are needed.
>
> > 3. Since it seems continue is acceptable to implementers, I am also
> > suggesting we use delete instead of remove, for consistency sake.
>
> Agreed.
>
> > ------- IDL --------
> >
> > [NoInterfaceObject]
> > interface IDBDatabase {
> >  readonly attribute DOMString     name;
> >  readonly attribute DOMString     description;
> >  readonly attribute DOMStringList objectStores;
> >  /*
> >  Open an object store in the specified transaction. The transaction can
> be
> >  dynamic scoped, or the requested object store must be in the static
> scope.
> >  Returns IDBRequest whose success event of IDBTransactionEvent type
> contains
> >  a result with IDBObjectStore and transaction is an
> IDBTransactionRequest.
> >  */
> >  IDBRequest openObjectStore(in DOMString name, in IDBTransaction txn,
> >  in optional unsigned short mode /* defaults to READ_WRITE */);
>
> I don't understand the advantage of this proposal over mozillas
> proposal.


The above proposal allows opening multiple object stores in the same
transaction in dynamic scope, even without having explicitly identified each
one of them at the time of creating the transaction. Secondly, where static
scope is desired, in order to run to completion without being aborted due to
unavailable objects, this proposal ensures that locks are obtained at the
time of creating the transaction.

My examples illustrates this approach. It uses both dynamic and static
scoped transactions. The dynamic scope transactions are synchronously
created since no locks have to be obtained when creating the transaction.


> One of our main points was to make getting objectStore
> objects a synchronous operation as to avoid having to nest multiple
> levels of asynchronous calls. Compare
>
> var req = db.openObjectStore("foo", trans);
> req.onerror = errorHandler;
> req.onsuccess = function(e) {
>  var fooStore = e.result;
>  var req = fooStore.get(12);
>  req.onerror = errorHandler;
>  req.onsuccess = resultHandler;
> }
>
> to
>
> var fooStore = db.openObjectStore("foo", trans);
> var req = fooStore.get(12);
> req.onerror = errorHandler;
> req.onsuccess = resultHandler;
>
>
> I also don't understand the advantage of having the transaction as an
> argument to openObjectStore rather than having openObjectStore live on
> transaction. Compare
>
> db.openObjectStore("foo", trans);
>
> to
>
> trans.openObjectStore("foo");
>
> I also don't understand the meaning of specifying a mode when a
> objectStore is opened, rather than specifying the mode when the
> transaction is created.


Have you reviewed the examples? Different object stores in a transaction are
used in different modes, and that requires us to identify the mode when
opening the object store. This also increases concurrency. This is
particularly useful for dynamic transactions.


> Unless we're planning on making all
> transactions dynamic (I hope not), locks have to be grabbed when the
> transaction is created, right? If a transaction is holding a READ_ONLY
> lock for a given objectStore, then attempting to open that objectStore
> as READ_WRITE should obviously fail. Consecutively, if a transaction
> is holding a READ_WRITE lock for a given objectStore, then opening
> that objectStore as READ_ONLY doesn't seem to have any benefit over
> opening it as READ_WRITE. In short, I can't see any situation when
> you'd want to open an objectStore in a different mode than what was
> used when the transaction was created.
>
> Finally, I would stronly prefer to have READ_ONLY be the default
> transaction type if none is specified by the author. It is better to
> default to what results in higher performance, and have people notice
> when they need to switch to the slower mode. This is because people
> will very quickly notice if they use READ_ONLY when they need to use
> READ_WRITE, since the code will never work. However if people use
> READ_WRITE when all they need is READ_ONLY, then the only effect is
> likely to be an application that runs somewhat slower, when they will
> unlikely detect.
>

This approach is also likely to cause exceptions upon put, remove, and add.
I would prefer to not cause exceptions as the default behavior.


>
> >  /*
> >  Open the database catalog in the specified transaction for exclusive
> access.
> >  Returns IDBRequest whose success event of IDBTransactionEvent type
> contains
> >  a result with IDBCatalog and transaction is an IDBTransactionRequest.
> >  */
> >  IDBRequest openCatalog(in IDBTransaction txn);
>
> See above regarding catalog objects.
>
> >  /*
> >  Create a new static scoped transaction asynchronously.
> >  Returns IDBRequest whose success event of IDBSuccessEvent type contains
> a
> >  result with IDBTransactionRequest.
> >  */
> >  IDBRequest openTransaction (in optional DOMStringList storeNames /*
> > defaults to mean all object stores */,
> >  in optional unsigned short mode /* defaults to READ_WRITE */,
> >  in optional IDBTransaction parent /* defaults to null */,
> >  in optional unsigned long timeout /* defaults to no timeout*/);
> >  /*
> >  Create a new dynamic scoped transaction. This returns a transaction
> handle
> >  synchronously.
> >  */
> >  IDBTransaction transaction (in optional IDBTransaction parent /*
> > defaults to null */,
> >  in optional unsigned long timeout /* defaults to no timeout*/);
>
> I don't understand this. When would you ever want to use the
> asynchronous version?
>

See my examples.


>
> Why can't you specify a list of objectStores in the synchronous version?
>

Because I don't see what is the benefit of doing that if you can't obtain
locks, which synchronous behavior cannot.


>
> Do we really need nested transactions? I do see that you added them in
> latest revision of the editor drafts, but it seems like a fairly
> advanced feature that would be nice to leave for a v2. If everyone
> else wants them, I can check how much work it would be for us to
> implement them, but my vote is to leave it for a later version.
>

I added them because they are useful in advanced scenarios. I am fine with
leaving them to a later date.


>
> I've left out commenting on the other interfaces for now, as it seems
> that the decisions we make for the questions above will likely affect
> the other interfaces anyway.
>
> / Jonas
>