5

I am using node-sqlite3, but I am sure this problem appears in another database libraries too. I have discovered a bug in my code with mixing transactions and async code. 

function insertData(arrayWithData, callback) {
    // start a transaction
    db.run("BEGIN", function() {
        // do multiple inserts
        slide.asyncMap(
            arrayWithData,
            function(cb) {
                db.run("INSERT ...", cb);
            },
            function() {
                // all done
                db.run("COMMIT");
            }
        );
    });
}

// some other insert
setInterval(
    function() { db.run("INSERT ...", cb); },
    100
);

You can also run the full example.

The problem is that some other code with insert or update query can be launched during the async pause after begin or insert. Then this extra query is run in the transaction. This is not a problem when the transaction is committed. But if the transaction is rolled back the change made by this extra query is also rolled back. Hoops we've just unpredictably lost data without any error message.

I thought about this issue and I think that one solution is to create a wrapper class that will make sure that:

  • Only one transaction is running at the same time.
  • When transaction is running only queries which belong to the transaction are executed.
  • All the extra queries are queued and executed after the current transaction is finished.
  • All attempts to start a transaction when one is already running will also get queued.

But it sounds like too complicated solution. Is there a better approach? How do you deal with this problem?

Improve this question

3 Answers 3

Reset to default
4
+50

At first, I would like to state that I have no experience with SQLite. My answer is based on quick study of node-sqlite3.

The biggest problem with your code IMHO is that you try to write to DB from different locations. As I understand SQLite, you have no control of different parallel "connections" as you have in PostgreSQL, so you probably need to wrap all your communication with DB. I modified your example to use always insertData wrapper. Here is the modified function:

function insertData(callback, cmds) {
  // start a transaction
  db.serialize(function() {
    db.run("BEGIN;");
    //console.log('insertData -> begin');
    // do multiple inserts
    cmds.forEach(function(item) {
      db.run("INSERT INTO data (t) VALUES (?)", item, function(e) {
        if (e) {
          console.log('error');
          // rollback here
        } else {
          //console.log(item);
        }
      });
    });
    // all done
    //here should be commit
    //console.log('insertData -> commit');
    db.run("ROLLBACK;", function(e) {
      return callback();
    });
  });
}

Function is called with this code:

init(function() {
  // insert with transaction
  function doTransactionInsert(e) {
    if (e) return console.log(e);
    setTimeout(insertData, 10, doTransactionInsert, ['all', 'your', 'base', 'are', 'belong', 'to', 'us']);
  }

  doTransactionInsert();

  // Insert increasing integers 0, 1, 2, ...
  var i=0;

  function doIntegerInsert() {
    //console.log('integer insert');
    insertData(function(e) {
      if (e) return console.log(e);
      setTimeout(doIntegerInsert, 9);
    }, [i++]);
  }

  ...

I made following changes:

  • added cmds parameter, for simplicity I added it as last parameter but callback should be last (cmds is an array of inserted values, in final implementation it should be an array of SQL commands)
  • changed db.exec to db.run (should be quicker)
  • added db.serialize to serialize requests inside transaction
  • ommited callback for BEGIN command
  • leave out slide and some underscore

Your test implementation now works fine for me.

Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

So basically what you are saying is - eliminate all async operations during the transaction. Seems like very elegant solution, I will try it out. Thanks!
it works for me, thx for the hint
3

I have end up doing full wrapper around sqlite3 to implement locking the database in a transaction. When DB is locked all queries are queued and executed after the current transaction is over.

https://github.com/Strix-CZ/sqlite3-transactions

Improve this answer

2 Comments

However, I am going to accept Ivoszz's answer because it is sufficient for most use cases.
Another possibility is to use existing ORM which support SQLite3 and transactions, eg. node-persist
0

IMHO there are some problems with the ivoszz's answer:

  1. Since all db.run are async you cannot check the result of the whole transaction and if one run has error result you should rollback all commands. For do this you should call db.run("ROLLBACK") in the callback in the forEach loop. The db.serialize function will not serialize async run and so a "cannot start transaction within transaction occurs".
  2. The "COMMIT/ROLLBACK" after the forEach loop has to check the result of all statements and you cannot run it before all the previous run finished.

IMHO there are only one way to make a safe-thread (obv referred to the background thread pool) transaction management: create a wrapper function and use the async library in order to serialize manually all statements. In this way you can avoid db.serialize function and (more important) you can check all single db.run result in order to rollback the whole transaction (and return the promise if needed). The main problem of the node-sqlite3 library related to transaction is that there aren't a callback in the serialize function in order to check if one error occurs

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.