Expotential backoff for reconnection #617

Closed
wants to merge 10 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

poohlty commented Feb 9, 2014

Discussion branch for now, not for immediate merge yet!

Contains basic logic for using exponential back-off, instead of delay for reconnection. Still lacks proper documentation/clean up for the old delay logic. @guille @defunctzombie

get commented Feb 9, 2014

👍

Contributor

rauchg commented Feb 9, 2014

Needs docs.

Contributor

poohlty commented Feb 9, 2014

Just added some docs for new methods :)

Contributor

poohlty commented Feb 9, 2014

@guille The API now allows user to pass in a custom back off method. Exponential back off is used by default

Contributor

rauchg commented Feb 9, 2014

Did you document the readme? That's key

Contributor

poohlty commented Feb 15, 2014

Terribly sorry for the delay, last week was kinda crazy for me for all the quizzes and make-up works :( So it seems that the change in code affect the interface, basically that several constants for delay are no longer needed. Instead, some other ones for exp back off are used. Should I just delete those getter/setter method for delay constants, and update them to the new ones for exp back off?

@rauchg rauchg commented on an outdated diff Mar 1, 2014

- `timeout` connection timeout before a `connect_error`
and `connect_timeout` events are emitted (`10000`)
+ - `initialDelay` how long is the first delay, default is 1000 (`1000`)
+ - `backoffMultipler` how much should be multiplied to increase the delay time, default is 1.5 (`1.5`)
+ - `randomizeFactor` how much should the multiplied time be randomized default is (`0`)
+ - `backoffMethod` a fuction that returns the backoff method of delays (`function(){ return 1000; }`)
@rauchg

rauchg Mar 1, 2014

Contributor

Please prefix all the options with reconnection. Otherwise initialDelay for example is too generic.

Contributor

poohlty commented Mar 9, 2014

@guille do I need more updates on this? Or is it good to merge? :P

Contributor

poohlty commented Apr 4, 2014

@guille I think the branch is good to merge now! ✌️

This issue was closed.

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