Nodejs Async get request

I made a small application that makes async calls to an api (local in this case). The target application will make a million of calls per day.

With maxSockets set to 1 I had a time of 45 seconds. With maxSockets = 10, the execution time has dropped to 40 seconds.

How can I make this code go faster please?

var request = require('request');
var mysql      = require('mysql');
var async = require('async');

var connection = mysql.createConnection({
    host     : 'localhost',
    user     : 'root',
    password : '',
    database : 'testAPI'
});
var inputData = [];
for(i=1; i "file1" body
        // results[1] -> "file2" body
        // results[2] -> "file3" body
    }

})
var checkIfFinished = setInterval(function(){
    if(counter === limit){
        console.log('finished')
        connection.end();
        clearInterval(checkIfFinished);
    }
},1000)

When you use a control-flow library, you don't poll. That's what the callbacks are for. They get called when operations complete. That's how you know you're done.

Let's start by eliminating dependencies as they are black boxes, and we don't really know how they are built. If you use newer versions of Node.js, you can use Promises which should pretty much replace async. As for request, you can simply use Node's http.get to read the remote file.

All your API calls can also be transformed into promise-returning functions. If you can't find an existing module that already does it for you, you can simply wrap the call in a Promise and call resolve or reject accordingly. What I normally suggest is to split operations into a promise-returning function, and leave it to the caller to chain them up. In this case, log manages the sequence of readFile and insertToDb.

function insertToDb(body){
  return new Promise((resolve, reject) => {
    var time = Date.now();
    var query = `insert into testAPI (name) values ('${time}${body}')`;
    connection.query(query, (err, rows, fields) => {
      if(err) reject(err);
      else resolve(body);
    });
  });
}

function readFile(file){
  return new Promise((resolve, reject) => {
    http.get(url, body => {
      if(error) reject(error);
      else resolve(body);
    });
  });
}

function log(file){
  return readFile(file)
    .then(fileData => {
      // Now we have read the file, time to insert to db
      return insertToDb(fileData);
    }, error => {
      // Failed to read file
    })
    .then(fileData => {
      // Insert succeeded, we conclude by resolving with the file contents
      return fileData;
    }, error => {
      // DB operation failed
    });
}

async.map simply just calls each value against an iterator then calls the callback when everything's done. This is something easily done with array.map and Promise.all. We can use array.map to call log and collect the returned promises to an array, then we use Promise.all to wait for all promises to resolve.

Promise.all(inputData.map(file => log(file))).then(results => {
  // results[0] -> "file1" body
  // results[1] -> "file2" body
  // results[2] -> "file3" body

  console.log('finished');
}, () => {
  // Something failed
});

Now reading a remote file 9999 times (and over HTTP) and writing to the db 9999 times in rapid succession is the real bottleneck here. You're essentially flooding both servers. Even on localhost, servers still have CPU and memory constraints. No matter how you make this script run faster, the script will run no faster than the slowest of the two.

I believe what you're simply doing is trying to log changes of the file into the DB. I suggest you read and insert at a slower interval, like say every 5 seconds. That should give the servers enough time to breathe. Besides, do you really need to read the file in a way that reads are milliseconds apart. That's some file you got there.

Other alternatives would be to stream the data instead of reading the file at an interval. For the DB, try looking into solutions like Redis.

A few other things:

-
You can use Template Strings instead of string concatenation to build strings.

-
Instead of new Date().getTime(), use Date.now(). Does the same thing without creating a new Date object.

-
fetch isn't really "fetching", it appears to read a file and store it to the DB. It's technically "logging". Name your variables and functions properly.

StackExchange Code Review Q#116009, answer score: 3