moleculer icon indicating copy to clipboard operation
moleculer copied to clipboard
verified publisher icon moleculerjs

When using "Latency" strategy console warning "possible EventEmitter memory leak detected"

Open nmarus opened this issue 6 years ago • 8 comments

Prerequisites

Please answer the following questions for yourself before submitting an issue.

  • [x] I am running the latest version
  • [x] I checked the documentation and found no answer
  • [x] I checked to make sure that this issue has not already been filed
  • [x] I'm reporting the issue to the correct repository

Current Behavior

When using the latency strategy, node posts max event listeners error. When using other strategy, error does not happen. No obseverd negative side effects other than the console message. This seems to only happen when there are 3 or more mole nodes on the same service bus. The debug output is using the TCP transporter, but get the same error with MQTT transporter.

Expected Behavior

No error. :)

Failure Information

With node --trace-warnings I get the following console messages (below).

Steps to Reproduce

Please provide detailed steps for reproducing the issue.

  1. Moleculer running 3 nodes.
  2. Config set to:
registry: {
    strategy: 'Latency',
    strategyOptions: {
      sampleCount: 5,
      lowLatency: 10,
      collectCount: 5,
      pingInterval: 10,
    },
  },

Context

Please provide any relevant information about your setup. This is important in case the issue is not reproducible except for under certain conditions.

  • Moleculer version: 0.13.9
  • NodeJS version: 10.15.3 or 10.16.0
  • Operating System: OSX 10.13.6

Failure Logs

(node:52645) MaxListenersExceededWarning: (node) warning: possible EventEmitter memory leak detected. 101 listeners added. Use emitter.setMaxListeners() to increase limit.
    at EventEmitter.logPossibleMemoryLeak (/<redacted>/node_modules/eventemitter2/lib/eventemitter2.js:52:15)
    at EventEmitter.growListenerTree (/<redacted>/node_modules/eventemitter2/lib/eventemitter2.js:228:35)
    at EventEmitter._on (/<redacted>/node_modules/eventemitter2/lib/eventemitter2.js:557:24)
    at EventEmitter.on (/<redacted>/node_modules/eventemitter2/lib/eventemitter2.js:504:17)
    at new LatencyStrategy (/<redacted>/node_modules/moleculer/src/strategies/latency.js:83:24)
    at new EndpointList (/<redacted>/node_modules/moleculer/src/registry/endpoint-list.js:33:19)
    at ActionCatalog.add (/<redacted>/node_modules/moleculer/src/registry/action-catalog.js:51:11)
    at _.forIn.action (/<redacted>/node_modules/moleculer/src/registry/registry.js:196:17)
    at /<redacted>/node_modules/lodash/lodash.js:4905:15
    at Function.forIn (/<redacted>/node_modules/lodash/lodash.js:12950:11)
    at Registry.registerActions (/<redacted>/node_modules/moleculer/src/registry/registry.js:183:5)
    at serviceList.forEach.svc (/<redacted>/node_modules/moleculer/src/registry/registry.js:101:10)
    at Array.forEach (<anonymous>)
    at Registry.registerServices (/<redacted>/node_modules/moleculer/src/registry/registry.js:88:15)
    at NodeCatalog.processNodeInfo (/<redacted>/node_modules/moleculer/src/registry/node-catalog.js:191:18)
    at Object.keys.forEach.nodeID (/<redacted>/node_modules/moleculer/src/transporters/tcp.js:624:25)
    at Array.forEach (<anonymous>)
    at TcpTransporter.processGossipResponse (/<redacted>/node_modules/moleculer/src/transporters/tcp.js:604:33)
    at TcpTransporter.onIncomingMessage (/<redacted>/node_modules/moleculer/src/transporters/tcp.js:268:42)
    at Parser.parser.on (/<redacted>/node_modules/moleculer/src/transporters/tcp/tcp-reader.js:100:21)
    at Parser.emit (events.js:189:13)
    at Parser._write (/<redacted>/node_modules/moleculer/src/transporters/tcp/parser.js:65:10)
(node:52645) MaxListenersExceededWarning: (node) warning: possible EventEmitter memory leak detected. 101 listeners added. Use emitter.setMaxListeners() to increase limit.
    at EventEmitter.logPossibleMemoryLeak (/<redacted>/node_modules/eventemitter2/lib/eventemitter2.js:52:15)
    at EventEmitter.growListenerTree (/<redacted>/node_modules/eventemitter2/lib/eventemitter2.js:228:35)
    at EventEmitter._on (/<redacted>/node_modules/eventemitter2/lib/eventemitter2.js:557:24)
    at EventEmitter.on (/<redacted>/node_modules/eventemitter2/lib/eventemitter2.js:504:17)
    at new LatencyStrategy (/<redacted>/node_modules/moleculer/src/strategies/latency.js:80:25)
    at new EndpointList (/<redacted>/node_modules/moleculer/src/registry/endpoint-list.js:43:24)
    at ActionCatalog.add (/<redacted>/node_modules/moleculer/src/registry/action-catalog.js:51:11)
    at _.forIn.action (/<redacted>/node_modules/moleculer/src/registry/registry.js:196:17)
    at /<redacted>/node_modules/lodash/lodash.js:4905:15
    at Function.forIn (/<redacted>/node_modules/lodash/lodash.js:12950:11)
    at Registry.registerActions (/<redacted>/node_modules/moleculer/src/registry/registry.js:183:5)
    at serviceList.forEach.svc (/<redacted>/node_modules/moleculer/src/registry/registry.js:101:10)
    at Array.forEach (<anonymous>)
    at Registry.registerServices (/<redacted>/node_modules/moleculer/src/registry/registry.js:88:15)
    at NodeCatalog.processNodeInfo (/<redacted>/node_modules/moleculer/src/registry/node-catalog.js:191:18)
    at Object.keys.forEach.nodeID (/<redacted>/node_modules/moleculer/src/transporters/tcp.js:624:25)
    at Array.forEach (<anonymous>)
    at TcpTransporter.processGossipResponse (/<redacted>/node_modules/moleculer/src/transporters/tcp.js:604:33)
    at TcpTransporter.onIncomingMessage (/<redacted>/node_modules/moleculer/src/transporters/tcp.js:268:42)
    at Parser.parser.on (/<redacted>/node_modules/moleculer/src/transporters/tcp/tcp-reader.js:100:21)
    at Parser.emit (events.js:189:13)
    at Parser._write (/<redacted>/node_modules/moleculer/src/transporters/tcp/parser.js:65:10)

nmarus avatar Jul 31 '19 14:07 nmarus

Could you create a repro repo?

AndreMaz avatar Jul 31 '19 23:07 AndreMaz

I can, but this single file example will generate the error on single instance for me. After some testing with this, it seems related to number of services. 3 service with 3 actions each does not generate the error, 10 with 10 each does.

const { ServiceBroker } = require('moleculer');

const brokerConfig = {
  metrics: false,
  cacher: {
    type: 'Memory',
  },
  registry: {
    strategy: 'Latency',
    strategyOptions: {
      sampleCount: 5,
      lowLatency: 10,
      collectCount: 5,
      pingInterval: 10,
    },
  },
  transporter: 'TCP',
};

const broker = new ServiceBroker(brokerConfig);

function genService(sSount, aCount) {
  // create and attach some services
  for (let i = 0; i < sSount; i += 1) {
    const service = {
      name: `service${i}`,
      actions: {},
    };

    // create some actions on this service
    for (let j = 0; j < aCount; j += 1) {
      service.actions[`test${j}`] = {
        handler: (ctx) => {
          const { a, b } = ctx.params;
          return a + b;
        },
      }
    }

    // attach service to broker
    broker.createService(service);
  }
}

genService(10, 10);
broker.start().catch(err => console.error(err));

nmarus avatar Aug 01 '19 13:08 nmarus

@zllovesuki I think that this is something related with your code. Could you take a look at this?

The MaxListenersExceededWarning seems to be caused by $node.latencySlave https://github.com/moleculerjs/moleculer/blob/f2164f99ad2dbc057685e52aa8e646a5b476b869/src/strategies/latency.js#L78-L83

that's being called twice for each action (lines 33 and 43) https://github.com/moleculerjs/moleculer/blob/f2164f99ad2dbc057685e52aa8e646a5b476b869/src/registry/endpoint-list.js#L29-L44

so the localBus gets filled pretty fast https://github.com/moleculerjs/moleculer/blob/f2164f99ad2dbc057685e52aa8e646a5b476b869/src/service-broker.js#L154-L158

For 10 nodes and 10 actions we have 212 $node.latencySlave listeners: 2 * ($node.list + $node.services + $node.actions + $node.events + $node.health + $node.options + 10(nodes)*10(actions))

image

AndreMaz avatar Aug 03 '19 11:08 AndreMaz

Ouch, that’s some bad stuff. Maybe only listen on actual actions instead of all. For now it is harmless, but I will take a look at it.

zllovesuki avatar Aug 24 '19 04:08 zllovesuki

IIRC each action gets its own registry, thus this was the unintended side effect.

zllovesuki avatar Aug 24 '19 04:08 zllovesuki

I wonder if the broker has a "master" bus?

zllovesuki avatar Mar 04 '20 01:03 zllovesuki

There is a local bus for the core components via broker.localBus

icebob avatar Mar 04 '20 17:03 icebob

right, my code was listening on broker.localBus. However, I think my code added a listener for each action/service. Is there a way to share states internally? As I was noted in the code:

Since Strategy can be instantiated multiple times, therefore,
we need to have a "master" instance to send ping, and each
individual "slave" instance will update their list dynamically

zllovesuki avatar Mar 05 '20 00:03 zllovesuki