resilient-http / resilient.js

Fault tolerant and reactive HTTP client for node.js and browsers
183 stars 13 forks source link

resilient is trying to resolve servers during resolving servers in prev request #143

Open jangot opened 6 years ago

jangot commented 6 years ago

I have a problem: I send request number 1. Resilient sends request to discovery service for getting hosts of app. At that time I send request number 2. Resilient sends request to discovery service for getting hosts of app again. Now this is what happens: request 1 falls down because second request kills response 1 from discovery service. It keeps happening until resilient gets new requests before it gets a reply from a discovery service. Is there a solution of this problem without fixing it in your code? Or maybe you can tell me how can I hardfix it in my fork?

h2non commented 6 years ago

Discovery process and requests are indepent workflows across requests. This should not be possible. It's true that discovery request can happen multiple times if the first discovery process was not satisfied. This is something that can be changed using a synchronization mechanism.

h2non commented 6 years ago

If you provide more details, such as the type of error you're getting, and some code to reproduce it, I might be able to assist you better.

jangot commented 6 years ago

This is my example

const resilientMiddlewareEureka = require('./resilient-middleware-eureka');
const Resilient = require('resilient');
let http = require('http');

// Resilient initialisations
const options = {
    balancer: {
        random: true,
        roundRobin: false
    },
    discovery: {
        servers: ['http://localhost:7777/eureka/v1/apps']
    },
    service: {}
};

let client = new Resilient(options);

client.use(resilientMiddlewareEureka('PROJECTS-API'));

// Run requests
const ITERATIONS_COUNT = 10;
let success = 0;
let promise = Promise.resolve(0);

// send 10 portions of requests
for (let i = 0; i <= ITERATIONS_COUNT; i++) {
    promise = promise
        .then((successCount) => {
            console.log(`success ${i}`);
            success += successCount;

            // send one portion (100 requests parallel)
            return makePackRequests(10);
        });
}

promise
    .then((successCount) => {
        success += successCount;
        console.log(`SUCCESS COUNT: ${success}`);
    })
    .catch((e) => {
        console.log(`error:`, e);
    });

function makePackRequests(count) {
    let promises = [];
    let successCount = 0;
    for (let i = 0; i < count; i++) {
        let promise = client.get('/projects')
            .then(() => {
                successCount++;
            })
            .catch((e) => {
                console.log(`ERROR`);
                console.log(e.message);
            });
        promises.push(promise);
    }

    return Promise
        .all(promises)
        .then(() => {
            return successCount;
        });
}

This is my result

success 0
OIUT
ERROR
Discovery server response is invalid or empty
ERROR
Discovery server response is invalid or empty
ERROR
Discovery server response is invalid or empty
ERROR
Discovery server response is invalid or empty
ERROR
Discovery server response is invalid or empty
ERROR
Discovery server response is invalid or empty
ERROR
Discovery server response is invalid or empty
ERROR
Discovery server response is invalid or empty
ERROR
Discovery server response is invalid or empty
success 1
success 2
success 3
success 4
success 5
success 6
success 7
success 8
success 9
success 10
SUCCESS COUNT: 101

And, how can I use a synchronization mechanism? Need I set other http client?

h2non commented 6 years ago

You can implement the synchronization inside your Eureka middleware by keeping a global state of the discovery requests. E.g: during the first discovery request call within your middleware logic, you create a promise that would be resolved only when you get the response from Eureka. Subsequent concurrent discovery calls would check if that promise exists and would subscribe to it instead of performing the discovery process again. That being said, I'm not fully sure where the problem origin actually is, but I suspect it's related to the custom middleware. As far as I know resilient.js behavior is consistent.

jangot commented 6 years ago

I wanted to use your solution but it makes overhead for requests. And I think that several equal requests is bad approach. I made small fix for my team and I hope it will make the lib better. https://github.com/resilient-http/resilient.js/pull/144