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

Avoid reconnecting to a broker that failed on us #61

Merged
merged 3 commits into from
Nov 9, 2017
Merged

Avoid reconnecting to a broker that failed on us #61

merged 3 commits into from
Nov 9, 2017

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Oct 31, 2017

Currently, broker_selector_usage returns 0 when no broker is ready. That's just wrong, a selector should never return a broker that is not ready. It should return -1 instead, like the others, which will make broker selection restart.

EDIT: I expanded the scope of the PR; see below.

@RalfJung
Copy link
Member Author

RalfJung commented Nov 1, 2017

While I was at it, I also improved the logging of the broker usage.

@RalfJung RalfJung changed the title fix broker_selector_usage for when there is no broker Avoid reconnecting to a broker that failed on us Nov 7, 2017
@RalfJung
Copy link
Member Author

RalfJung commented Nov 7, 2017

I expanded the scope of the PR to also include a new feature: If connecting to a broker fails, we will not try that broker again because we assume it is broken. This only applies to failure during the initial connection establishment; if the broker disconnects later (e.g. because it got restarted), that's fine. When all brokers got disabled, we enable them all again.

This avoids silly loops where for some reason, one broker doesn't work, and hence it has the lowest usage, and hence everyone picks it, and hence nobody can connect. It doesn't catch more higher-level failure though (e.g. l2tp works, but then there is no batman connection).

@kostko kostko merged commit ea0773f into wlanslovenija:master Nov 9, 2017
@RalfJung RalfJung deleted the no-broker branch November 25, 2017 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants