neumino / thinky

JavaScript ORM for RethinkDB
http://justonepixel.com/thinky/
Other
1.12k stars 128 forks source link

relations / EventEmitter warning #77

Closed mshick closed 10 years ago

mshick commented 10 years ago

I've been getting the following error when trying to register some relations:

(node) warning: possible EventEmitter memory leak detected. 11 listeners added. Use emitter.setMaxListeners() to increase limit.

I'm having trouble figuring out the cause though. My initial thought was that it had to do with the number of relations I was creating with my user model. But, upon playing around, I discovered that the number didn't seem to matter -- it was a specific relationship causing the issue. So, I tried changing the load order of those relations, still, no dice. I've replicated the relationship (1-many) on a different model, and have had success getting that one working, so I'm a bit bewildered.

Sorry to be light on specifics, it'd be hard to get into it without going through so much that is going on in my app. Hoping you can jumpstart my debugging with your experience.

mshick commented 10 years ago

Jeez, n/m.

I was accidentally registering an index on one model with the same name as the foreignKey in the relation. Doh!

mshick commented 10 years ago

Sort of oddly, I'm now experiencing this as a transient bug. I thought I isolated the warning to the following group of four relations (the specifics aren't important, but removing any one of these 4 types would resolve the warning, and adding any one of the 4 back would trigger it again):

Model.hasAndBelongsToMany()
Model.belongsTo()
Model.belongsTo()
Model.belongsTo()

Leading to:

(node) warning: possible EventEmitter memory leak detected. 11 listeners added. Use emitter.setMaxListeners() to increase limit.

Which I isolated on another model. Then, I undid that change, reverted my original model to have the same group of relations, and magically the warning vanished. I did play with process.setMaxListeners(0) just to see, but that seemed to have no effect and I've removed the setting.

I've re-opened this ticket hoping maybe you can respond an give some insight (if possible) as to what you think might trigger these problems w/r/t relationships.

mshick commented 10 years ago

And, just as suddenly, restoring the other side of a 1-n relationship has the warning back...

It's like there's some maximum amount of back-and-forth allowed in total in the process?

I've tried updating node to 0.10.28, but no dice, and the setMaxListeners(0). Going to attempt an update to node 0.11...

Is it possible there is some recursion happening somewhere? Maybe you can help me find where I might try to trace the issue? More broadly, do event emitters play a major role in the architecture of Thinky relationships?

mshick commented 10 years ago

Okay, no possible way with node 0.11. I didn't realize how much code wouldn't work there.

So, after trying several tests in isolation, it seems like the real problem is related to the number of indexes being created on a single model. Since relations create indexes for the keys, and ensureIndex will try to create an index, and everything registers to the "ready" event (I think that's the name) once you hit 10 total it triggers the emitter warning. Or perhaps there is a system total?

I may still be a little off base with this, but I believe this is mostly correct in that it's somehow related to the initialization process.

mshick commented 10 years ago

After a bunch of playing around I believe I have finally discovered the issue.

I was attempting to register additional models and relations, but without waiting for a Model ready event first. I suspect the variety of actions needed before triggering ready were piling up, leading to the eventEmitter warning -- which was transient, because things would complete in slightly different times.

So, I wait for one model to be ready before triggering the initialization of the next and my warnings and other problems seem resolved. I use pseudo-code like this:

ModelA.once('ready', function () {
  ModelB.createModel();
  ModelB.once('ready', function () {
    // and on...
  });
});

Hopefully this helps somebody searching these threads someday!

neumino commented 10 years ago

Reopening, something smells fishy here.

The warning is harmless, but it should happen only if you create more than 10 indexes/relations.

jcspencer commented 10 years ago

You should be able to write Model.listeners(event) to get a list of all it's listeners. I'm going to investigate

neumino commented 10 years ago

While fixing #75 and #83, I found where the bug is.

I basically use on instead of once in a few places. I'll fix everything at the same time.

neumino commented 10 years ago

Released in 1.1.1