0

I am trying to create some APIs for my mobile app. I'm using Node+Express+Mongo+Firebase. Based on suggestions give in various other places (How to properly reuse connection to Mongodb across NodeJs application and modules) I've created below code structure:

My code structure looks like this

This is what I have in my mongoUtils.js:

const MongoClient = require('mongodb').MongoClient
const uri = "mongodb+srv://user:password@blabla.azure.mongodb.net/bla?retryWrites=true&w=majority";

let _db

const connectDB = async (callback) => {
    try {
        MongoClient.connect(uri,{ useNewUrlParser: true }, (err, client) => {
            _db = client.db('ABC');
            return callback(err)
        })
    } catch (e) {
        throw e
    }
}

const getDB = () => _db
const disconnectDB = () => _db.close()
module.exports = { connectDB, getDB, disconnectDB }

This is what I have in my index.js:

const functions = require('firebase-functions');
const express = require('express');
const app = express();
var cors = require('cors');
app.use(cors());
const admin = require('firebase-admin');
const bodyParser = require('body-parser')
const assert = require('assert');

const MongoDB = require('./db/mongoUtils')

admin.initializeApp(functions.config().firebase);
app.use(bodyParser.urlencoded({ extended: true }))
app.use(bodyParser.json())

app.post('/getUserByPhone',(request,response)=>{

    MongoDB.connectDB(async (err) => {
        assert.equal(null, err);
        console.log("Connected correctly to server");
        const db = MongoDB.getDB();
        const collection = db.collection('users');
        console.log(request.body.phone);
        // Find some documents
        collection.find({/*Query here*/}).toArray(function(err, docs) {
            assert.equal(err, null);
            console.log("Found the following records");
            console.log(docs)
            response.send(docs);
        })
    })
})

app.post('/getUserById',(request,response)=>{
    var ObjectId = require('mongodb').ObjectId; 
    var id = request.body.userId;
    var userId = new ObjectId(id);

    MongoDB.connectDB(async (err) => {
        assert.equal(null, err);
        const db = MongoDB.getDB();
        console.log("Connected correctly to server");
        const collection = db.collection('users');
        collection.find({/*Query*/}).toArray(function(err, docs) {
            assert.equal(err, null);
            console.log("Found the following records");
            console.log(docs)
            response.send(docs);
        })
    })
})

app.post('/moreFunction',async(request,response)=>{
    var currTime = new Date();

    MongoDB.connectDB(async (err) => {
        assert.equal(null, err);
        const db = MongoDB.getDB();
        const collection = db.collection('coll_2');

        collection.find().toArray(function(err, docs) {
            assert.equal(err, null);
            response.send(docs);
        });
    })     
})

app.post('/anotherFunction',async(request,response)=>{
    MongoDB.connectDB(async (err) => {
        assert.equal(null, err);
        const db = MongoDB.getDB();
        console.log("Connected correctly to server");
        const collection = db.collection('coll_name');
        collection.find().toArray(function(err, docs) {
            assert.equal(err, null);
            response.send(docs);
        });
    })     
})

exports.api = functions.https.onRequest(app);

Now, whenever I am testing my app by running it on my Android device, I am seeing a spike of connections in my MongoDb console. Instead of one connection, I am seeing app use around 25-30 connections as I navigate between screens. What's wrong with my approach? Why does it use so many connections?

This app is not published and I'm the only user.

halfer
  • 19,824
  • 17
  • 99
  • 186
Vikalp Jain
  • 583
  • 2
  • 11
  • 25
  • 2
    I believe here you are using connectDB() --> for every request which basically creates new connection, instead of reusing the connection. – Lucia Oct 11 '19 at 06:21
  • That's what this article recommends: https://stackoverflow.com/questions/24621940/how-to-properly-reuse-connection-to-mongodb-across-nodejs-application-and-module. It says "The reason this works is that in node, when modules are require'd, they only get loaded/sourced once so you will only ever end up with one instance of _db and mongoUtil.getDb() will always return that same instance." – Vikalp Jain Oct 11 '19 at 07:43
  • 2
    Connect to the database at application level only once, and at route level use that connection. Right now at every route you are establishing a new connection. – Sándor Bakos Oct 11 '19 at 08:59
  • 1
    @VikalpJain but you are not calling MongoDB.connectDB(async (err)) -- on every request, so that always creates new connection. You should connect to DB only once in your mongoUtil class initialisation. And in every request you should just use MongoDB.getDB() – Lucia Oct 11 '19 at 09:08
  • Thanks @Lucia and Sandor Bakos. I now understand the mistake I'm making. To overcome I tried changing my getDB function to : const getDB = async() => { try{ if (!_db){await connectDB()} return _db } catch (e) { throw e } } And instead of ConnectDB, now I am calling getDB from my routes functions. But this is giving 'callback is not a function' error. – Vikalp Jain Oct 11 '19 at 11:10
  • Will be very helpful, if you can help me with some high level code on the correct way of doing this. – Vikalp Jain Oct 11 '19 at 11:11

3 Answers3

0

You've got a weird mix of async/await and callbacks going on here, and while I'm not going to write your code for you, I might offer a couple suggestions.

Ideally, you'll have some initialization that can connect the MongoDB adapter before you need to call on it so you're not awaiting a previously resolved promise (extra code) or destroying and recreating a new connection every time you get a request (extra code and takes extra time for the TLS 3-way handshake). I've written a solution for reusing connection and initialization code before here.

const MongoClient = require('mongodb').MongoClient
const uri = "mongodb+srv://user:password@blabla.azure.mongodb.net/bla?retryWrites=true&w=majority";

const client = new MongoClient(uri);
// client.connect returns a promise that can be awaited
const connectDB = () => client.connect();


const getDB = () => client.db;
// so does db.close
const disconnectDB = () => client.db.close();
module.exports = { connectDB, getDB, disconnectDB }

const MongoDB = require('./db/mongoUtils')

app.post('/getUserByPhone', async (request,response)=>{
    try {
      await MongoDB.connectDB();
      const db = MongDB.getDB();
      const collection = db.collection('users');
      const docs = await collection.find({/*Query here*/}).toArray();
      response.send(docs);
    } catch (e) {
      // handleErr(e);
      return response.send({ok: false}).status(500);
    }
})
EddieDean
  • 1,806
  • 11
  • 13
0

I know this is a nodejs question, but below design will solve this problems regardless of language you choose.

Regardless of what code you write and in which language, if you create new objects in each script, even with connection pooling on MongoDB side, you will see a spike. We had a Java implementation and we saw similar spike. Our connection pool rose to 340 out 350 max connections on Atlas, even with static connection objects, along with all the best practices.

What you have to do it somehow create 1 object of const MongoDB = require('./db/mongoUtils') on server startup, and store it somewhere (in cache/serialized.. anything) and make sure it is accessible throughout the application. And enforce all your nodejs classes/script to use that same object via some common function/getter.

After doing that in Java using ServletContext to store one connection object, we saw our connections were limited to 40-50 max at a time on Atlas MongoDB.

zookastos
  • 917
  • 10
  • 37
0

The answer above from @EddieDean is a great place to start. Only thing I suggest is changing the following:

// client.connect() will only execute when client is not connected
const connectDB = () => (!client.isConnected() ? client.connect() : client)

This prevents this error from happening.

John Bae
  • 1
  • 2