CODEWITHSUNDEEP
javascriptnode.jsasynchronousconcurrencycouchdb
Priyath Gregory
Priyath Gregory

Reputation: 987

node.js data consistency when iterating asynchronously

I have a tool who's basic idea is as follows:

//get a bunch of couchdb databases. this is an array
    const jsonFile = require('jsonfile');
    let dbList = getDbList();
    const filePath = 'some/path/to/file';
    const changesObject = {};


     //iterate the db list. do asynchronous stuff on each iteration
     dbList.forEach(function(db){
        let merchantDb = nano.use(db);

        //get some changes from the database. validate inside callback 
        merchantDb.get("_changes", function(err,changes){
          validateChanges(changes);
          changesObject['db'] = changes.someAttribute;
          //write changes to file
          jsonFile.writeFile(filePath, changesObject, function (err) {
            if (err) {
              logger.error("Unable to write to file: ");
            }
          });
    })

const validateChanges = function(changes) {
 if (!validateLogic(changes) sendAlertMail();
}

For performance improvements the iteration is not done synchronously. Therefore there can be multiple iterations running in 'parallel'. My question is can this cause any data inconsistencies and/or any issues with the file writing process?

Edit: The same file gets written to on each iteration.

Edit:2 The changes are stored as a JSON object with key value pairs. The key being the db name.

Upvotes: 1

Views: 559

Answers (1)

T.J. Crowder
T.J. Crowder

Reputation: 1074575

If you're really writing to a single file, which you appear to be (though it's hard to be sure), then no; you have a race condition in which multiple callbacks will try to write to the same file, possibly at the same time (remember, I/O isn't done on the JavaScript thread in Node unless you use the *Sync functions), which will at best mean the last one wins and will at worst mean I/O errors because of overlap.

If you're writing to separate files for each db, then provided there's no cross-talk (shared state) amongst validateChanges, validateLogic, sendAlertMail, etc., that should be fine.

Just for detail: It will start tasks (jobs) getting the changes and then writing them out; the callbacks of the calls to get won't be run until later, when all of those jobs are queued.

You are creating closures in loops, but the way you're doing it is okay, both because you're doing it within the forEach callback and because you're not using db in the get callback (which would be fine with the forEach callback but not with some other ways you might loop arrays). Details on that aspect in this question's answers if you're interested.

This line is suspect, though:

let merchantDb = nano.use('db');

I suspect you meant (no quotes):

let merchantDb = nano.use(db);

For what it's worth, it sounds from the updates to the question and your various comments like the better solution would be not to write out the file separately each time. Instead, you want to gather up the changes and then write them out.

You can do that with the classic Node-callback APIs you're using like this:

let completed = 0;
//iterate the db list. do asynchronous stuff on each iteration
dbList.forEach(function(db) {
    let merchantDb = nano.use(db);

    //get some changes from the database. validate inside callback 
    merchantDb.get("_changes", function(err, changes) {
        if (err) {
            // Deal with the fact there was an error (don't return)
        } else {
            validateChanges(changes);
            changesObject[db] = changes.someAttribute; // <=== NOTE: This line had 'db' rather than db, I assume that was meant to be just db
        }
        if (++completed === dbList.length) {
            // All done, write changes to file
            jsonFile.writeFile(filePath, changesObject, function(err) {
                if (err) {
                    logger.error("Unable to write to file: ");
                }
            });
        }
    })
});

Upvotes: 2

Related Questions