aaronpowell / db.js

db.js is a wrapper for IndexedDB to make it easier to work against
http://aaronpowell.github.com/db.js/
MIT License
818 stars 142 forks source link

Transaction management #95

Closed dumbmatter closed 8 years ago

dumbmatter commented 9 years ago

I like the idea of db.js – a simple promises-based API that transparently wraps the IndexedDB API (some other libraries have a bit too much magic behind the scenes for me to be comfortable). I really like the “modify” syntax for updating objects too. There are a few minor features I want to recommend adding… but that is for another time.

My main problem with db.js is transaction support. Currently, each db.js operation runs in its own transaction. This is nice for simplicity, but it completely kills performance. Try doing 1000 adds in a single transaction versus 1000 adds in 1000 different transactions.

The problem is, transaction handling is kind of messy. Compared to the current db.js, adding transactions to the API would be one more thing for users to worry about. And IndexedDB transactions are pretty confusing to work with due to how auto-committing works.

How might adding transactions to the db.js API work? Here’s one (probably stupid) idea. Currently, if you wanted to do 1000 adds, you might do something like this:

var promises = [];
for (var i = 0; i < 1000; i++) {
    promises.push(server.whatever.add(data[i]));
}
return Promise.all(promises);

But that uses 1000 transactions. Here's an alternative:

var tx = server.tx("whatever", "readwrite");
for (var i = 0; i < 1000; i++) {
    tx.whatever.add(data[i]);
}
return tx.complete();

This would do all the adds in a single transaction tx created by server.tx. server.tx is a wrapper around IDBDatabase.transaction which adds access to the object stores specified in the first argument and complete, a function that returns a promise that resolves after the oncomplete event for the transaction fires. And the first code example would still work just as it does today, the transaction API would be purely optional to use.

This API was not handed down to me on clay tablets from God. It's just an idea. The larger point is, something is needed to deal with transactions or it’ll continue to be impossible to write performant IDB-heavy apps with db.js.

Is there interest in adding something like this to db.js? Or do you see db.js as something that should not have this level of complexity?

aaronpowell commented 9 years ago

I'll admit that bulk data inserting is not a problem I've had to deal with, hence why db.js doesn't do anything special in that space.

I do like your idea of generating a transaction "wrapper" where you push everything into and then a bulk operation runs against it, and if you're willing to have a crack and send a PR then I'm all ears :smiley:

mikemorton commented 9 years ago

I'm not sure if this is new functionality since this Issue was originally opened but it does seem like the .add and .update do take in arrays as parameters and will bulk insert/update on a single transaction.

To flush out the original example, server.add('whatevertable', data); would do the trick assuming data is your array of stuff.

dumbmatter commented 9 years ago

I actually have a half-baked implementation of my original idea here that I am using in production, and I was planning to try to merge it with db.js at some point. But I'm not 100% convinced that it's actually stable (hard to tell given the browser bugs that exist), and the general relationship between IndexedDB transactions and promises is terrifying and unlikely to improve soon. So I'm pretty wary of recommending that other people do what I'm doing.

aaronpowell commented 9 years ago

It comes down to what you see the promise covers, is it the transaction or the add/update request. db.js treats the whole transaction as the UoW which is to be 'promises'.

mikemorton commented 9 years ago

I've been thinking about this and was wondering if you would approve a PR if I added a function that allows the following?

server.transaction([
  {'add','myFirstStore', {...} },
  {'add', 'mySecondStore', [ ... ] },
  {'update, 'myThirdStore', { ... } },
  {'update', 'myFourthStore', [ ... ] }
]);

Basically the function would take in a list of objects, where each object is an operation name (add, remove and update could be supported), a store (a single readwrite transaction would be opened on all of the specified stores), and an object or array of objects to be added/updated/removed.

The implementation of the function wouldn't be too difficult and I can't think of any pitfalls to this approach.

I'm not in love with transaction as the function name and am open to other suggestions.

dumbmatter commented 9 years ago

IMHO (as the guy who submitted this issue and otherwise has no involvement with the project)....

It does allow you to do more than the current db.js, which is good. But it doesn't allow you to do everything - for instance, reading and writing in the same transactions, or dynamically generating objects and inserting them in the same transaction without pre-generating them all in memory. Ideally you'd have an API that can support all of that functionality, like my original syntax. I'm just wary of the promises/transactions issue.

On Fri, May 1, 2015 at 9:37 PM, Mike Morton notifications@github.com wrote:

I've been thinking about this and was wondering if you would approve a PR if I added a function that allows the following?

server.transaction([ {'add','myFirstStore', {...} }, {'add', 'mySecondStore', [ ... ] }, {'update, 'myThirdStore', { ... } }, {'update', 'myFourthStore', [ ... ] } ]);

Basically the function would take in a list of objects, where each object is an operation name (add, remove and update could be supported), a store (a single readwrite transaction would be opened on all of the specified stores), and an object or array of objects to be added/updated/removed.

The implementation of the function wouldn't be too difficult and I can't think of any pitfalls to this approach.

I'm not in love with transaction as the function name and am open to other suggestions.

— Reply to this email directly or view it on GitHub https://github.com/aaronpowell/db.js/issues/95#issuecomment-98278409.

aaronpowell commented 8 years ago

There's a method on the db instance that the open promise resolves that allows you to access the underlying IndexedDB object, getIndexedDB. Would that suite for doing the more edge-case IndexedDB operations, such as you described here?

dumbmatter commented 8 years ago

Technically yes, but in the long run something like the API I wrote in the original issue here would be nicer.

However I think you should wait until https://github.com/inexorabletash/indexeddb-promises/ becomes part of IndexedDB (God willing!) before doing anything about it, otherwise it'll probably have weird cross-browser compatibility bugs.

dumbmatter commented 8 years ago

I implemented the API proposed here in another library. I did it there rather than a PR here due to the aforementioned browser compatibility issues - my library only works in Chrome and Firefox, which kind of sucks. And even in Firefox it only works with certain third-party Promise libraries. If the browser support situation ever changes, maybe we can join forces.

mikemorton commented 8 years ago

Very interesting @dumbmatter

aaronpowell commented 8 years ago

Promises come up semi regularly on the IndexedDB mailing list and there's no consistent solution, short of introducing some major breaking changed for IDB2.

I'm going to close this for now as there are other options if you need to bulk inserts, such as @dumbmatter's library.