marcello3d / node-mongolian

[project inactive] Mongolian DeadBeef is an awesome Mongo DB driver for node.js
https://groups.google.com/group/node-mongolian
zlib License
349 stars 50 forks source link

server.close() is not closing everything (node doesn't exit) #33

Closed goloroden closed 1 year ago

goloroden commented 13 years ago

Hi there,

I use mongolian inside a web app. Basically, when I connect to MongoDB I do not provide any parameters, the defaults seems to work fine for me :-).

Anyway: When a connection is not longer needed, is does not get closed. It is closed when I stop Node.js manually using Ctrl+C, but up to that point the connections stay open.

Playing around with the keepAlive parameter did not have any effect, and calling close() manually did not have any effect either.

What am I doing wrong?

Or is closing connections something you can let mongolian care about? Isn't there a problem if 100's or 1000's of connections are open at the same time?

Cheerio,

Golo

marcello3d commented 13 years ago

keepAlive was removed a few versions back, so it shouldn't have any effect.

Mongolian currently doesn't have any connection management of its own, though it will probably gain such functionality in the future. Why do you want to make 100s and 1000s of connections?

Mind including some sample code on close() not working? It looks like a bug.

goloroden commented 13 years ago

Ah, okay, that clarifies some things :-)

Maybe my question is a little bit naive, but: What would be the correct approach in a web app to establish connections? I am coming from ASP.NET, and there it is perfectly valid to establish a new connection for each request, and close it once you do not need it any longer. Actually, it's even considered best practice to keep connections open as short as possible.

So, how do you do this with Mongolian without opening 100s or 1000s of connections (when there are 100s or 1000s of requests)?

goloroden commented 13 years ago

PS: Sorry, "Closing" was a click on the wrong button ... of course the topic is NOT closed ;-)

goloroden commented 13 years ago

My code basically looks like this (CoffeeScript):

class MongoDbMappingProvider extends EventEmitter
  findOne: (id) ->
    server = new Mongolian()
    db = server.db 'myDb'
    mappings = db.collection 'mappings'
    mappings.findOne { alias: id }, (err, mapping) =>
      if not mapping?
        @emit 'mappingCouldNotBeFound'
        return
      @emit 'mappingAvailable', mapping

And this class is instantiated newly for each incoming request.

If you insert a call to

server.close()

in there, it gets executed, but does not close the connection.

sha1dy commented 13 years ago

I'm also having this problem - it seems not closing connection brakes nodeunit and expresso

sha1dy commented 13 years ago

I should have been more specific - server.close() doesnt work for me also. and without it working nodeunit and expresso hangs after finishing tests so I have to CTRL + C everytime.

marcello3d commented 13 years ago

server.close() is working for me. Here is my nodeunit test:

var Mongolian = require('../mongolian.js')
module.exports = {
    "test connection close": function(test) {
        var mongo = new Mongolian('mongo://localhost')
        mongo.dbNames(function(error,dbs) {
            mongo.close()
            test.ifError(error)
            test.ok(dbs)
            console.log("dbs = "+dbs)
            test.done()
        })
    }
}
goloroden commented 13 years ago

Uhm, why did you close it?

Just that it works on your machine does not mean that it works correctly - and obviously, there is a problem. Should this bug not be reopened for further examination?

marcello3d commented 13 years ago

Apologies, I closed this unintentionally—github needs a way of assigning problems back to the reporter. :-)

Can you give me a quick hint on how to run your example? I've never used coffeescript, and I'm not sure where you inserted the close statement in your code (which otherwise seems ok—again, not a coffeescript guy).

Couple notes: making a request after closing a connection will re-open it, and closing a connection before a connection has been made will be a no-op. I hope to improve this a bit in the future once I have a better understanding as to how Mongolian is being used (the requirements for a webapp are different than those for a commandline tool).

goloroden commented 13 years ago

Ah, okay ;-)

I will build a short, but complete example and come back to you.

asleepysamurai commented 13 years ago

I have also been facing this issue recently. In effect calling server.close() seems to do nothing.

I did some further digging and managed to trace the issue to this piece of code in server.js.

MongolianServer.prototype.close = function(error) {
    error = error || new Error("Connection closed")
    if (this._connection.value) {
        var callbacks = this._callbacks
        this._callbacks = {}
        this._callbackCount = 0
        this._connection.value.close()
        this._connection.reset()
        delete this._type
        for (var requestId in callbacks) {
            callbacks[requestId](error)
        }
        this.mongolian._replicaSet.emit('lost', this)
    }
}

Specifically this._connection.value is undefined. For now i managed to get close working by using the following code:

MongolianServer.prototype.close = function(error) {
    error = error || new Error("Connection closed")
    var that = this;
    this._connection(function(err,val){
        if (!err && val) {
            var callbacks = that._callbacks
            that._callbacks = {}
            that._callbackCount = 0
            val.close()
            //that._connection.reset()
            delete that._type
            for (var requestId in callbacks) {
                callbacks[requestId](error)
            }
            that.mongolian._replicaSet.emit('lost', that)
        }
    });
}

I have commented out the _connection.reset since this was for some reason causing another connection being established and then disconnected. This works fine with synchronous db connections, but fails for async connections. I believe caching the connection with taxman has something to do with it, but I haven't really looked into it much.

marcello3d commented 13 years ago

Thanks for digging into this, I think I might have an idea what's going on. I'm probably going to re-implement the underlying connection code directly in Mongolian to be able to access all the errors/failures directly (currently it's using mongodb-native's connection code).

dimroc commented 13 years ago

Just wanted to chime in and say that I'm also running into the issue where my node app will not exit because I am unable to close the mongolian db connection. the .close() function doesn't even seem to be on my mongolian object.

anttip commented 12 years ago

dimroc, maybe you are connecting straight to a database with your connection string? Then use db.server.close():

var db = new Mongolian("mongo://localhost/test");
db.server.close();
reggi commented 11 years ago

http://stackoverflow.com/questions/13575988/close-mongo-mongolian-connection-right-after-opening-it