neumino / rethinkdbdash

An advanced Node.js driver for RethinkDB with a connection pool, support for streams etc.
MIT License
848 stars 109 forks source link

How many times should I see the following log #326

Closed MikeFielden closed 7 years ago

MikeFielden commented 7 years ago

Creating a pool connected to localhost:28015

Im only attempting to create a single pool and have all the connections draw from it. If i get > 1 of the above message does indicate a possible problem with how i'm getting connections?

Here is my current code for managing connections:

const r            = require('rethinkdbdash')({
        host  : 'localhost'
        port  : 28015,
        buffer: 100, // random
        max   : 3000 // random
      })
  ;

module.exports = (databaseName, tableName) => {
  if (databaseName === undefined) {
    return r;
  }

  r.setArrayLimit(500000);

  const db = r.db(databaseName);

  return (tableName !== undefined) ? db.table(tableName) : db;
};

Running this i get the message about creating a pool twice.

If I move the r instantiation within the function I get 4 logs. Again, does this indicate that I'm doing it wrong?

I am attempting to run a beefy extract,tranform,load process with changefeeds turned on so I need to tweek the connection settings a bit but seems like the settings are high already. So some investigation on this issue lead me to ask why do i get multiple pool creations.

Any insight you could lend me would be great!

Thanks!

rosskukulinski commented 7 years ago

@MikeFielden how many RethinkDB servers do you have in your cluster? Running your code on my localhost with a single Server, I only see a single message.

MikeFielden commented 7 years ago

Just a single server in the cluster. The 2 that are reported come from the 2 change feeds i'm starting just not entirely sure why its reporting the creation of a different pool.

neumino commented 7 years ago

The log message appear once everytime you call (instantiate) the module

MikeFielden commented 7 years ago

@neumino Based on the code pasted above would you agree I only instantiate the module 1 time? At least thats my goal in moving the initial call to rethinkdbdash outside the exported function.

neumino commented 7 years ago

Yea your code is creating just one instance of rethinkdbdash.

On Fri, Feb 17, 2017, 05:51 Mike Fielden notifications@github.com wrote:

@neumino https://github.com/neumino Based on the code pasted above would you agree I only instantiate the module 1 time? At least thats my goal in moving the initial call to rethinkdbdash outside the exported function.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/neumino/rethinkdbdash/issues/326#issuecomment-280654478, or mute the thread https://github.com/notifications/unsubscribe-auth/ABZOu3PpP1N_ulUw05Gj0SQx6P1O1CV9ks5rdaXTgaJpZM4MAyRy .

MikeFielden commented 7 years ago

Sorry, forgive my ignorance just trying to understand :)

What would be the recommended why to manage your pool in a real production app. Calling

const r = require('rethinkdbdash')
, connection = r({...configs})
;

Each time i want to use it seems like id be creating a NEW pool each time. I dont think i want to do that. I'd like to create a pool when my app boots and draw from that pool. Is this a valid pattern for production use?

marshall007 commented 7 years ago

@MikeFielden the code in your original post should result in the desired behavior. Are you sure you're not running multiple instance of your Node app or spawning child processes that also require this file?

MikeFielden commented 7 years ago

@marshall007 We are running throng

When I pull the code out of the throng block it fires 1 console.log.

Creating a pool connected to localhost:28015
Listening on port 3000

When I set throng to have 4 workers

Creating a pool connected to localhost:28015
Started 4 workers
Creating a pool connected to localhost:28015
Creating a pool connected to localhost:28015
Creating a pool connected to localhost:28015
Creating a pool connected to localhost:28015
Worker #2 starting express
Worker #3 starting express
Worker #1 starting express
Worker #4 starting express
Listening on port 3000
Listening on port 3000
Listening on port 3000
Listening on port 3000

When i set throng to 1 worker:

Creating a pool connected to localhost:28015
Started 1 worker
Creating a pool connected to localhost:28015
Worker #1 starting express
Listening on port 3000

Seems like I'm definately doing something wrong it seems.

davefinster commented 7 years ago

@MikeFielden it looks as though your using throng to take advantage of multiple CPUs? It appears to this by instantiating multiple processes (i.e. one process per worker or CPU core). Each process is a completely isolated instance of the Node.js/V8 runtime. throng then appears to just combine the stdout of all the processes.

What your seeing would be expected behaviour for anything starting multiple node processes. Node processes can't share between each other, so each one will instantiate its own connection pool.

rosskukulinski commented 7 years ago

@davefinster is correct. The first one is the master process, then each worker that get forks also creates their own connection pool.

If you don't want the master to connect, you could wrap the db logic around cluster.isMaster

MikeFielden commented 7 years ago

@davefinster @rosskukulinski I completely agree that this is what is happening.

However I am running the code in question within the masterStarted documentation here

Here is the exact code I'm running

throng({
  workers: NUM_OF_WORKERS,
  master : () => {
    console.log(`Master started`);

    /** Rethinkdb ChangeFeeds **/
    require('./src/db/changeFeeds/genericLogTableChanges')();
    require('./src/db/changeFeeds/captureCrewChanges')();
  },
  start  : (id) => {
    console.log(`Worker #${id} starting express`);
    let app = require('./createWebserver');

    bootExpressServer(app, PORT, TIMEOUT);
  }
});
rosskukulinski commented 7 years ago

Sure - but how/where do you require() or import the code you first posted (https://github.com/neumino/rethinkdbdash/issues/326#issue-207587876)?

If either the master block or start block reference code that require/import your DB code, you'll see the log messages.

MikeFielden commented 7 years ago

This module actually lives in a different private repo that is a dependency of this one (the one using throng), I'm requiring in that repo in this file but not that file itself.

marshall007 commented 7 years ago

@MikeFielden based on the code in your previous comment (and to @rosskukulinski's point) it seems like your "master" process is starting changefeeds and your "worker" web-server processes presumably also connect to RethinkDB. The result is that your master process and each worker process are all going to use a separate connection pool (and thus log to console). This is why you see n+1 lines logged to console where n is the number of worker processes.

MikeFielden commented 7 years ago

Yes @marshall007 I would agree. Seems to be some needle in this haystack isn't behaving :) I shall investigate further! Thanks guys!