Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

algorithm: freeSockets pop/shift #8

Open
syndr0m opened this issue Sep 1, 2014 · 7 comments
Open

algorithm: freeSockets pop/shift #8

syndr0m opened this issue Sep 1, 2014 · 7 comments

Comments

@syndr0m
Copy link

syndr0m commented Sep 1, 2014

the current algorithm maximize the number of sockets opened, by re-using the first stacked free socket using freesockets.shift()

@see https://github.com/node-modules/agentkeepalive/blob/master/lib/_http_agent.js#L170

if (freeLen) {
    // we have a free socket, so use that.
    var socket = this.freeSockets[name].shift();
    debug('have free socket');
freesockets = [1][2][3][4][5]
# a socket is freed
freesockets = [1][2][3][4][5][6]
# new request arrive, current algorithm use "shift"
freesockets = [2][3][4][5][6]
# there might rarely be a timeout of any sockets.

an alternative could be to use freesockets.pop() to minimize the number of sockets opened

freesockets = [1][2][3][4][5]
# a socket is freed
freesockets = [1][2][3][4][5][6]
# new request arrive, using pop
freesockets = [1][2][3][4][5]
# first socket timeout
freesockets = [2][3][4][5]

implementation:

var algo = (this.options.algo === 'pop') ? 'pop' : 'shift';
var socket = this.freeSockets[name][algo]();

I tested this for 1K request/s, it divided the pool size by 5.

@fengmk2
Copy link
Member

fengmk2 commented Sep 2, 2014

@syndr0m good point! Can you impl this with a pr?

@FredKSchott
Copy link

@syndr0m this sounds awesome!

@fengmk2
Copy link
Member

fengmk2 commented Jan 29, 2015

Any progress?

@fengmk2
Copy link
Member

fengmk2 commented Jan 29, 2015

I think use pop instead of shift is enough

@tony-gutierrez
Copy link
Contributor

Did this ever happen?

@bfdill
Copy link

bfdill commented Mar 15, 2019

fyi, the referenced code is not currently in this project:

https://github.com/nodejs/node/blob/master/lib/_http_agent.js#L165

You'd probably need to replace addRequest and curry the selected algorithm into the original'ish version.

A quick thought here... While it would be nice to be able to have the algorithm choice, I believe the intended behavior of keep-alives is to keep connections alive to reduce the overhead of establishing connections. The max number of free connections is configurable, so I'm not sure if this functionality has a ton of value.

@dkMorlok
Copy link

Maybe related/solved by this nodejs/node#33278 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants