New Feature: sub-circuits

[peter-vdc]

We wanted to use this library with our Mongo-DB connection. We noticed we needed to create a brake for every type of query we are doing. This seemed a bit of overkill to us so we came up with this sub-circuit model. The idea is that you can add multiple pairs of promises (primary-service & fallback-service) all on the same brake that monitors the connection to Mongo.

[peter-vdc]

I have no idea why Istanbul keeps failing most of the time with the error: "after each" hook. I give up trying to get it to work. (locally the fail/success ratio is about 50/50 but here the fails seems to be much higher)

[awolden]

Hey @peter-vdc,

Glad to hear you guys are interested in the library! The problem you guys are running into is a common one with circuits. The main Java client this is based on (Hystrix) has a similar problem. Fortunately, for us there is a fairly elegant way for us to solve this problem in Javascript-land :).

IMHO, the best solution for this particular problem is to create a wrapper function that can then be used to create a new circuit. The wrapper function will proxy all requests to the mongo client and make it so we don't have to manage multiple different circuits.

See this example (note: this is basically freehand, so it won't execute as written)

const MongoClient = require('mongodb').MongoClient
const Promise = require('bluebird');

Promise.promisifyAll(MongoClient);

const client = MongoClient.connectAsync('mongodb://localhost:27017/test')
    .then(function(db) {

        // create a wrapper function that proxies requests from the circuit
        // to the mongodb client
        function mongoWrapper(method, opts) {
            return db[method](opts);
        }

        // instantiate circuit
        const mongoBrake = new Brakes(mongoWrapper);

        // make a call to the circuit
        return mongoBrake('findOneAsync', {
            id: 'someId'
        }).then(document => {
          return mongoBrake('findAsync');
        });
    })
    .catch(function(err) {});

When taking this approach, I would suggest you make a separate proxy function for reads and writes.

-Alexander Wolden

[peter-vdc]

Hi Alexander,

Thank you for your feedback. We did try your suggested path ourselves at first, but came to the conclusion that this is fine if you have just a few queries in MongoDB, but when it's getting more complex and especially if you want to implement fallback promises for each (eg: to S3) it's getting very complicated quickly. That is why we came up with these 'sub-circuits' (I still don't like the name, but can't think of anything better).

I just made the pull request to see if you were interested to include it into your library as well.

PS: still no idea why Istanbul keeps failing in your checks. The coverage check usually does work when I'm running it locally & I did as well in my original pull request.

Cheers, Peter

[jquatier]

@peter-vdc Could you elaborate a little more on how the sub-circuits pattern makes things easier than declaring a new instance of Brakes for each? It seems like you would still need to define the promise and fallback promise (same as calling new Brakes()), so I'm not sure what complexity is avoided. Are there other settings on the Brakes instance you are not wanting to duplicate? Just curious. Thanks for any additional info!

[peter-vdc]

It's not easier per-sé, just more convenient I thought. Let's say you have about 20 Brakes to the same service. If the service goes down, it goes down for all 20 Brakes. Each of them will probably go to circuit-open at different times so that's a lot more time-outs than there need to be. Also All 20 of them will then try to re-establish a connection back to the service (health-check). Arguably you'll have statistics on each Brakes, but I'm not sure how interesting that is.

By putting several circuits on the same Brakes, once the service is down for one circuit, it's down for all and there is only one Brakes that will try to re-establish the connection to it.

[jquatier]

Thanks for the additional details. I see the goal you are trying to achieve. I would mention though, that setting things up in this way comes at great cost because you lose individual statistics for each type of action. The Hystrix stream and dashboard become much less helpful when real performance issues do arise. For example, if find operations suddenly started timing out or had a large increase in response time, you would only be able to see that MongoDB wasn't happy. The nice thing about having a circuit per action is you get really detailed stats for each and the Hystrix dashboard becomes really useful for getting quick info on the state of the dependency. If queries were suddenly slow you could look at things like the dataset itself to see if there was a large increase in records, or examine your composite indexes to see if full table scans were occurring. All the while, your reads are probably still fine in this scenario and don't need to be short circuited.

[peter-vdc]

Maybe, but in my experience, once one thing in MongoDB is starting to go slow, everything is. It seemed to queue the requests. I know that this is not always the case and depends on a lot of factors.

Anyway, I did not pay any attention to the statistics because I have no idea how to set the Hystrix dashboard up in our environment (Heroku). Since I don't have direct access to any of the servers (only through a load-balancer / router) I have no idea which of the servers in a cluster I'm connected to for the Histrix stream (and these servers are volatile anyway).

[awolden]

I understand the use case peter, and I think the ability to handle that situation will be a valuable addition to the module, but there are a couple issues with the implementation that I would like to clean up.

The copied logic between the main Brakes.js file and Circuit.js is an anti pattern. I don't want to have to maintain that logic in 2 places. I propose that we strip all circuit making/executing logic from Brakes.js and rely solely on Circuit.js. When a new master brake is created it will create a masterCircuit by calling createCircuit(). In this pattern, I would also like to make passing a function to the main Brakes.js constructor optional. Functions themselves aren't really hierarchical, so it probably doesn't make sense to have one master circuit and bunch of subCircuits. In that way the main circuit created by new Brakes(...) could be considered more of a container than anything else.

I will clone your branch and see if there is a quick way I can make these improvements.

[awolden]

closing in favor of #33