Supporting nodejs in cluster mode + 'this' context in callback/fallback

[idanh]

Hi. I see that there is not support for cluster mode. Could this be done with minimum overhead (aggregate before sending to master)? Because stats are collected on workers, and master will expose the SSE stream, I'm unsure this works out of the box.

As well as context support. for example, I'd like to execute a method from my class. Currently, I see that you're using .apply(this (where this is Circuit.js), args);. Is it possible to configure this per circuit?

thanks!

[idanh]

I'm messing with a custom solution, and have a suggestion: Can we add an option for a slave circuit to report stats to master but via IPC? (today, it's reporting but not via IPC, like in your example: examples /slave-circuit.js)

I guess another issue arises from the fact that each worker have it's own circuit. Master node should decide when to enter the state of 'failure' or 'success', and notify relevant slave circuits. The master should control the threshold configuration, or any other circuit decision configuration so it'll be synchronized among all slave circuits.

[awolden]

Hey @idanh,

There are 2 issues here that I will address separately:

1. The use of .apply when executing the service function.

This is a by-product of having to proxy N # of arguments to the passed service call. Without the es6 spread operator, I am not aware of how to do that without stomping over the this context of the function. I could add in the option to pass a custom this object when defining the service call. That could look something like:

  const brake = new Brakes(asyncCall, {timeout: 150});

  brake
    .context(myClass) // to be used in the .apply call
    .exec('bar')
    .then((result) =>{
      console.log(`result: ${result}`);
    })
    .catch(err =>{
      console.error(`error: ${err}`);
    });

Let me know if you think that is something that will work for your use case.

2. Cluster support

This is something that can be added. I don't think it will be too hard to add some logic where the globalStats tracker automatically detects if it is in a child or master process, and behaves accordingly. The child process would send stats to the master process via IPC on the same snapshot interval it currently does, and the master process would aggregate those stats, determine thresholds, and communicate open/close state back to the work processes.

But, I would want this to work in a way that is generalizable to load balancing methods besides the cluster module. Many production system use nginx or something similar instead of the cluster module because it is faster. In a set-up where the cluster module isn't being used, it isn't as simple because you can't use straight-forward master-child IPC.

I think something like the gossip or swim protocols would be very useful here. It would allow all processes to remain in sync and enable a certain amount of self discovery among the processes. It could also be used as a plug-and-play solution when using the cluster module, because the processes could coordinate what UDP/TCP ports they are using. Since this would be a bit of a build, I am going to flag it as a possible v3 improvement.

-Alex

[idanh]

Hi @awolden, About 2. It'll be great if you could use an adapter pattern for it. We're using IPC with no intention in changing it in the near future (for internal reasons). It'd be great if I could select "Use IPC/Gossip/Swim/Whatever".

Thanks! -Idan

[03eltond]

I know this is a bit of an old issue, but wanted to also raise my hand for the 'this' context in callback/fallback. Often times we have objects/classes that live for only the life of an express request, and so a pattern we commonly have to use with Brakes is to explicitly pass a context argument. One way this could be solved, although it would be a big change in design direction, would be if there were a way to call a function like exec in a way where we pass both a one time function to execute (which will return a promise) as well as a one time fallback for failure, where each could be an arrow function and be able to use this. This way we don't have to declare a more "static" function + fallback function. Maybe something like:

const circuitBreaker = new Brakes(<options without a function>);

class MyClass {
  async function myInstanceMethod() {
    const result = await circuitBreaker.dynamicExec(() => {
      this.logger.log('now "this" works without passing context');
      return this.asyncWork();
    }, () => {
      this.logger.log('we are in a fallback, and still have access to "this"');
      return somethingUseful;
    });

    // do something with result
  }
}

This pattern of not requiring the work to be declared statically also has the benefit of being able to, say, guard a whole slew of possible redis gets/sets/deletes/etc with the same circuit breaker and not requiring individual slave circuits for each possible unit of work.

[03eltond]

Wanted to follow up on my previous post - I think it can be disregarded. I had a misunderstanding of how slave circuits worked. I thought of them as long lived, like a brake is, but realized after looking through the code that i could run single use slave circuits any time I want and thus can use as many arrow functions as needed. Thanks for having code that is so approachable!