Merge pull request #2437 from kuzzleio/improve-node-eviction-prevention

Overall improvement of the Debug Controller

Author: Shiranuit

lib/api/controllers/debugController.ts

@@ -22,6 +22,7 @@
 import { KuzzleRequest } from "../request";
 import { NativeController } from "./baseController";
 import * as kerror from "../../kerror";
+import get from "lodash/get";
 
 /**
  * @class DebugController
@@ -49,13 +50,29 @@ export class DebugController extends NativeController {
    * Connect the debugger
    */
   async enable() {
+    if (!get(global.kuzzle.config, "security.debug.native_debug_protocol")) {
+      throw kerror.get(
+        "core",
+        "debugger",
+        "native_debug_protocol_usage_denied"
+      );
+    }
+
     await global.kuzzle.ask("core:debugger:enable");
   }
 
   /**
    * Disconnect the debugger and clears all the events listeners
    */
   async disable() {
+    if (!get(global.kuzzle.config, "security.debug.native_debug_protocol")) {
+      throw kerror.get(
+        "core",
+        "debugger",
+        "native_debug_protocol_usage_denied"
+      );
+    }
+
     await global.kuzzle.ask("core:debugger:disable");
   }
 
@@ -64,6 +81,14 @@ export class DebugController extends NativeController {
    * See: https://chromedevtools.github.io/devtools-protocol/v8/
    */
   async post(request: KuzzleRequest) {
+    if (!get(global.kuzzle.config, "security.debug.native_debug_protocol")) {
+      throw kerror.get(
+        "core",
+        "debugger",
+        "native_debug_protocol_usage_denied"
+      );
+    }
+
     const method = request.getBodyString("method");
     const params = request.getBodyObject("params", {});
 
@@ -75,6 +100,14 @@ export class DebugController extends NativeController {
    * See events from: https://chromedevtools.github.io/devtools-protocol/v8/
    */
   async addListener(request: KuzzleRequest) {
+    if (!get(global.kuzzle.config, "security.debug.native_debug_protocol")) {
+      throw kerror.get(
+        "core",
+        "debugger",
+        "native_debug_protocol_usage_denied"
+      );
+    }
+
     if (request.context.connection.protocol !== "websocket") {
       throw kerror.get(
         "api",
@@ -98,6 +131,14 @@ export class DebugController extends NativeController {
    * Remove the websocket connection from the events' listeners
    */
   async removeListener(request: KuzzleRequest) {
+    if (!get(global.kuzzle.config, "security.debug.native_debug_protocol")) {
+      throw kerror.get(
+        "core",
+        "debugger",
+        "native_debug_protocol_usage_denied"
+      );
+    }
+
     if (request.context.connection.protocol !== "websocket") {
       throw kerror.get(
         "api",

lib/api/funnel.js

@@ -51,6 +51,7 @@ const debug = require("../util/debug")("kuzzle:funnel");
 const processError = kerror.wrap("api", "process");
 const { has } = require("../util/safeObject");
 const { HttpStream } = require("../types");
+const get = require("lodash/get");
 
 // Actions of the auth controller that does not necessite to verify the token
 // when cookie auth is active
@@ -179,6 +180,12 @@ class Funnel {
       throw processError.get("not_enough_nodes");
     }
 
+    const isRequestFromDebugSession = get(
+      request,
+      "context.connection.misc.internal.debugSession",
+      false
+    );
+
     if (this.overloaded) {
       const now = Date.now();
 
@@ -226,7 +233,8 @@ class Funnel {
      */
     if (
       this.pendingRequestsQueue.length >=
-      global.kuzzle.config.limits.requestsBufferSize
+        global.kuzzle.config.limits.requestsBufferSize &&
+      !isRequestFromDebugSession
     ) {
       const error = processError.get("overloaded");
       global.kuzzle.emit("log:error", error);
@@ -239,7 +247,13 @@ class Funnel {
         request.internalId,
         new PendingRequest(request, fn, context)
       );
-      this.pendingRequestsQueue.push(request.internalId);
+
+      if (isRequestFromDebugSession) {
+        // Push at the front to prioritize debug requests
+        this.pendingRequestsQueue.unshift(request.internalId);
+      } else {
+        this.pendingRequestsQueue.push(request.internalId);
+      }
 
       if (!this.overloaded) {
         this.overloaded = true;

lib/cluster/node.js

@@ -256,6 +256,15 @@ class ClusterNode {
    */
   preventEviction(evictionPrevented) {
     this.publisher.sendNodePreventEviction(evictionPrevented);
+    // This node is subscribed to the other node and might not receive their heartbeat while debugging
+    // so this node should not have the responsability of evicting others when his own eviction is prevented
+    // when debugging.
+    // Otherwise when recovering from a debug session, all the other nodes will be evicted.
+    for (const subscriber of this.remoteNodes.values()) {
+      subscriber.handleNodePreventEviction({
+        evictionPrevented,
+      });
+    }
   }
 
   /**

lib/cluster/subscriber.js

@@ -680,7 +680,15 @@ class ClusterSubscriber {
    * to recover, otherwise we evict it from the cluster.
    */
   async checkHeartbeat() {
-    if (this.state === stateEnum.EVICTED || this.remoteNodeEvictionPrevented) {
+    if (this.remoteNodeEvictionPrevented) {
+      // Fake the heartbeat while the node eviction prevention is enabled
+      // otherwise when the node eviction prevention is disabled
+      // the node will be evicted if it did not send a heartbeat before disabling the protection.
+      this.lastHeartbeat = Date.now();
+      return;
+    }
+
+    if (this.state === stateEnum.EVICTED) {
       return;
     }
 

lib/cluster/workers/IDCardRenewer.js

@@ -29,6 +29,10 @@ class IDCardRenewer {
       const redisConf = config.redis || {};
       await this.initRedis(redisConf.config, redisConf.name);
     } catch (error) {
+      // eslint-disable-next-line no-console
+      console.error(
+        `Failed to connect to redis, could not refresh ID card: ${error.message}`
+      );
       this.parentPort.postMessage({
         error: `Failed to connect to redis, could not refresh ID card: ${error.message}`,
       });

lib/core/debug/kuzzleDebugger.ts

@@ -1,7 +1,7 @@
 import Inspector from "inspector";
 import * as kerror from "../../kerror";
 import { JSONObject } from "kuzzle-sdk";
-import get from "lodash/get";
+import HttpWsProtocol from "../../core/network/protocols/httpwsProtocol";
 
 const DEBUGGER_EVENT = "kuzzle-debugger-event";
 
@@ -15,7 +15,11 @@ export class KuzzleDebugger {
    */
   private events = new Map<string, Set<string>>();
 
+  private httpWsProtocol?: HttpWsProtocol;
+
   async init() {
+    this.httpWsProtocol = global.kuzzle.entryPoint.protocols.get("websocket");
+
     this.inspector = new Inspector.Session();
 
     // Remove connection id from the list of listeners for each event
@@ -97,6 +101,20 @@ export class KuzzleDebugger {
     this.inspector.disconnect();
     this.debuggerStatus = false;
     await global.kuzzle.ask("cluster:node:preventEviction", false);
+
+    // Disable debug mode for all connected sockets that still have listeners
+    if (this.httpWsProtocol) {
+      for (const eventName of this.events.keys()) {
+        for (const connectionId of this.events.get(eventName)) {
+          const socket =
+            this.httpWsProtocol.socketByConnectionId.get(connectionId);
+          if (socket) {
+            socket.internal.debugSession = false;
+          }
+        }
+      }
+    }
+
     this.events.clear();
   }
 
@@ -109,21 +127,33 @@ export class KuzzleDebugger {
       throw kerror.get("core", "debugger", "not_enabled");
     }
 
-    if (!get(global.kuzzle.config, "security.debug.native_debug_protocol")) {
-      throw kerror.get(
-        "core",
-        "debugger",
-        "native_debug_protocol_usage_denied"
-      );
-    }
-
-    // Always disable report progress because this params causes a segfault.
+    // Always disable report progress because this parameter causes a segfault.
     // The reason this happens is because the inspector is running inside the same thread
-    // as the Kuzzle Process and reportProgress forces the inspector to send events
-    // to the main thread, while it is being inspected by the HeapProfiler, which causes javascript code
-    // to be executed as the HeapProfiler is running, which causes a segfault.
+    // as the Kuzzle Process and reportProgress forces the inspector to call function in the JS Heap
+    // while it is being inspected by the HeapProfiler, which causes a segfault.
     // See: https://github.com/nodejs/node/issues/44634
-    params.reportProgress = false;
+    if (params.reportProgress) {
+      // We need to send a fake HeapProfiler.reportHeapSnapshotProgress event
+      // to the inspector to make Chrome think that the HeapProfiler is done
+      // otherwise, even though the Chrome Inspector did receive the whole snapshot, it will not be parsed.
+      //
+      // Chrome inspector is waiting for a HeapProfiler.reportHeapSnapshotProgress event with the finished property set to true
+      // The `done` and `total` properties are only used to show a progress bar, so there are not important.
+      // Sending this event before the HeapProfiler.addHeapSnapshotChunk event will not cause any problem,
+      // in fact, Chrome always do that when taking a snapshot, it receives the HeapProfiler.reportHeapSnapshotProgress event
+      // before the HeapProfiler.addHeapSnapshotChunk event.
+      // So this will have no impact and when receiving the HeapProfiler.addHeapSnapshotChunk event, Chrome will wait to receive
+      // a complete snapshot before parsing it if it has received the HeapProfiler.reportHeapSnapshotProgress event with the finished property set to true before.
+      this.inspector.emit("inspectorNotification", {
+        method: "HeapProfiler.reportHeapSnapshotProgress",
+        params: {
+          done: 0,
+          finished: true,
+          total: 0,
+        },
+      });
+      params.reportProgress = false;
+    }
 
     return this.inspectorPost(method, params);
   }
@@ -137,6 +167,18 @@ export class KuzzleDebugger {
       throw kerror.get("core", "debugger", "not_enabled");
     }
 
+    if (this.httpWsProtocol) {
+      const socket = this.httpWsProtocol.socketByConnectionId.get(connectionId);
+      if (socket) {
+        /**
+         * Mark the socket as a debugging socket
+         * this will bypass some limitations like the max pressure buffer size,
+         * which could end the connection when the debugger is sending a lot of data.
+         */
+        socket.internal.debugSession = true;
+      }
+    }
+
     let listeners = this.events.get(event);
     if (!listeners) {
       listeners = new Set();
@@ -159,6 +201,32 @@ export class KuzzleDebugger {
     if (listeners) {
       listeners.delete(connectionId);
     }
+
+    if (!this.httpWsProtocol) {
+      return;
+    }
+
+    const socket = this.httpWsProtocol.socketByConnectionId.get(connectionId);
+    if (!socket) {
+      return;
+    }
+
+    let removeDebugSessionMarker = true;
+    /**
+     * If the connection doesn't listen to any other events
+     * we can remove the debugSession marker
+     */
+    for (const eventName of this.events.keys()) {
+      const eventListener = this.events.get(eventName);
+      if (eventListener && eventListener.has(connectionId)) {
+        removeDebugSessionMarker = false;
+        break;
+      }
+    }
+
+    if (removeDebugSessionMarker) {
+      socket.internal.debugSession = false;
+    }
   }
 
   /**

lib/core/network/clientConnection.js

@@ -31,10 +31,11 @@ const uuid = require("uuid");
  * @param {object} [headers] - Optional extra key-value object. I.e., for http, will receive the request headers
  */
 class ClientConnection {
-  constructor(protocol, ips, headers = null) {
+  constructor(protocol, ips, headers = null, internal = null) {
     this.id = uuid.v4();
     this.protocol = protocol;
     this.headers = {};
+    this.internal = {};
 
     if (!Array.isArray(ips)) {
       throw new TypeError(`Expected ips to be an Array, got ${typeof ips}`);
@@ -45,6 +46,10 @@ class ClientConnection {
       this.headers = headers;
     }
 
+    if (isPlainObject(internal)) {
+      this.internal = internal;
+    }
+
     Object.freeze(this);
   }
 }

lib/core/network/protocols/httpwsProtocol.js

@@ -282,6 +282,7 @@ class HttpWsProtocol extends Protocol {
     res.upgrade(
       {
         headers,
+        internal: {},
       },
       req.getHeader("sec-websocket-key"),
       req.getHeader("sec-websocket-protocol"),
@@ -292,7 +293,12 @@ class HttpWsProtocol extends Protocol {
 
   wsOnOpenHandler(socket) {
     const ip = Buffer.from(socket.getRemoteAddressAsText()).toString();
-    const connection = new ClientConnection(this.name, [ip], socket.headers);
+    const connection = new ClientConnection(
+      this.name,
+      [ip],
+      socket.headers,
+      socket.internal
+    );
 
     this.entryPoint.newConnection(connection);
     this.connectionBySocket.set(socket, connection);
@@ -457,8 +463,17 @@ class HttpWsProtocol extends Protocol {
       const buffer = this.backpressureBuffer.get(socket);
       buffer.push(payload);
 
-      // Client socket too slow: we need to close it
-      if (buffer.length > WS_BACKPRESSURE_BUFFER_MAX_LENGTH) {
+      /**
+       * Client socket too slow: we need to close it
+       *
+       * If the socket is marked as a debugSession, we don't close it
+       * the debugger might send a lot of messages and we don't want to
+       * loose the connection while debugging and loose important information.
+       */
+      if (
+        !socket.internal.debugSession &&
+        buffer.length > WS_BACKPRESSURE_BUFFER_MAX_LENGTH
+      ) {
         socket.end(WS_FORCED_TERMINATION_CODE, WS_BACKPRESSURE_MESSAGE);
       }
     }

lib/kuzzle/kuzzle.ts

@@ -249,7 +249,6 @@ class Kuzzle extends KuzzleEventEmitter {
         seed: this.config.internal.hash.seed,
       });
 
-      await this.debugger.init();
       await new CacheEngine().init();
       await new StorageEngine().init();
       await new RealtimeModule().init();
@@ -279,6 +278,8 @@ class Kuzzle extends KuzzleEventEmitter {
       // before opening connections to external users
       await this.entryPoint.init();
 
+      await this.debugger.init();
+
       this.pluginsManager.application = application;
       const pluginImports = await this.pluginsManager.init(options.plugins);
       this.log.info(