moleculerjs / moleculer

:rocket: Progressive microservices framework for Node.js
https://moleculer.services/
MIT License
6.11k stars 580 forks source link

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

Closed nmarus closed 1 year ago

nmarus commented 5 years ago

Prerequisites

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

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.

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)
AndreMaz commented 5 years ago

Could you create a repro repo?

nmarus commented 5 years ago

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));
AndreMaz commented 5 years ago

@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

zllovesuki commented 5 years ago

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 commented 5 years ago

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

zllovesuki commented 4 years ago

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

icebob commented 4 years ago

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

zllovesuki commented 4 years ago

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