Permalink
Browse files

Improved async capabilities:

The auto-commit was committing when it shouldn't, something like: add to queue in callback
of a query executed after .commit was called. It was messy, but upon consideration it seems
that the auto-commit functionality is unnecessary anyway. Consider:
1) Auto-commit is only enabled when the user calls .commit
2) Auto-commit is only enacted when the user creates a new Queue
3) New Queues do not become active when they are created
4) New Queues will not become active until the active queue is completed
5) The active queue does not complete until you call .commit or .rollback

The only case where auto-commit makes sense is if a user calls .commit and then
tries to begin a new transaction on the old Queue object... but this is not supported
since the Queue object does not have a .startTransaction method.

This fix enabled some code that otherwise failed, consisting of parallel multiple inserts,
all tests pass.
  • Loading branch information...
1 parent edf9c6d commit c13711830934e58bd46670f0baa5da4137ac4893 Kris Reeves committed May 10, 2012
Showing with 21 additions and 42 deletions.
  1. +17 −39 index.js
  2. +4 −3 package.json
View
56 index.js
@@ -15,19 +15,6 @@ module.exports = function(db, debug) {
//Create a new executable query Queue
db.createQueue = function() {
return new Queue(function() {return dbQuery.apply(db, arguments);}, function () {
- //If the current Queue is a transaction that has not yet been committed, commit it
- var ceq = options.currentlyExecutingQueue;
- if(ceq != null && ceq.commit != null)
- {
- //Also, warn the user that relying on this behavior is a bad idea
- if(ceq._autoCommit !== true)
- console.warn("WARNING: mysql-queues: Database transaction was " +
- "implicitly committed.\nIt is HIGHLY recommended that you " +
- "explicitly commit all transactions.\n" +
- "The last query to run was:", ceq.lastExecuted.sql);
- ceq.commit(ceq._autoCommitCB);
- return;
- }
options.currentlyExecutingQueue = null;
//Called when a Queue has completed its processing and main queue should be executed
while(options.mainQueue.length > 0)
@@ -67,7 +54,7 @@ function Queue(dbQuery, resumeMainQueue, options) {
/* Execute all queries on the Queue in order and prevent other queries from executing until
all queries have been completed.
*/
- this.execute = function() {
+ this.execute = function(commit) {
if(this.paused === true) return this;
var that = this;
//If another Queue is currently running, we put this on the mainQueue
@@ -93,17 +80,10 @@ function Queue(dbQuery, resumeMainQueue, options) {
//Execute the original callback first (which may add more queries to this Queue)
if(item.cb != null)
item.cb.apply(this, arguments);
+
//When the entire queue has completed...
if(++done == total)
- {
- if(that.paused === true) return;
- /* The query's callback may have queued more queries on this Queue.
- If so, execute this Queue again; otherwise, resumeMainQueue() */
- if(that.queue.length == 0)
- resumeMainQueue();
- else
- that.execute();
- }
+ that.execute();
});
} catch(e) {
if(options.debug)
@@ -117,8 +97,15 @@ function Queue(dbQuery, resumeMainQueue, options) {
//All queued queries are running, but we don't resume the main queue just yet
//console.log("Queue Complete:", options.currentlyExecutingQueue);
}
- else if(options.currentlyExecutingQueue == this)
- resumeMainQueue();
+ else if(options.currentlyExecutingQueue == this) {
+ if(commit) {
+ dbQuery("COMMIT", function() {
+ delete that;
+ resumeMainQueue();
+ });
+ return;
+ }
+ }
return this; //Chaining :)
};
this.pause = function(maxWaitTime) {
@@ -132,29 +119,20 @@ function Queue(dbQuery, resumeMainQueue, options) {
}
return this; //Chaining
}
- this.resume = function() {
+ this.resume = function(commit) {
if(this.pauseTimer)
clearTimeout(this.pauseTimer);
this.paused = false;
- this.execute();
+ this.execute(commit || false);
return this; //Chaining
}
}
Queue.isNowTransaction = function(q, dbQuery) {
q.query("START TRANSACTION");
q.commit = function(cb) {
- if(this.queue.length > 0)
- {
- this._autoCommit = true;
- this._autoCommitCB = cb;
- this.resume();
- }
- else
- {
- delete this.commit;
- delete this._autoCommit;
- this.query("COMMIT", cb).resume();
- }
+ delete this.commit;
+ delete this.rollback;
+ this.resume(true);
}
q.rollback = function(cb) {
this.queue = [];
View
7 package.json
@@ -6,10 +6,11 @@
"homepage": "https://github.com/bminer/node-mysql-queues",
"repository": {
"type": "git",
- "url": "git://github.com/bminer/node-mysql-queues.git"
+ "url": "git://github.com/myndzi/node-mysql-queues.git"
},
"contributors": [
- "Tom Atkinson <atkinson.tommy@neosoft.ba>"
+ "Tom Atkinson <atkinson.tommy@neosoft.ba>",
+ "Kris Reeves <krisreeves@searchfanatics.com>"
],
"main": "index.js",
"engines": {
@@ -19,7 +20,7 @@
"mysql": ">=0.9.5"
},
"bugs": {
- "url": "https://github.com/bminer/node-mysql-queues/issues"
+ "url": "https://github.com/myndzi/node-mysql-queues/issues"
},
"devDependencies": {},
"keywords": [

0 comments on commit c137118

Please sign in to comment.