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

Prioritize alive message over other messages #159

Merged
merged 3 commits into from
Aug 9, 2018
Merged

Conversation

armon
Copy link
Member

@armon armon commented Aug 8, 2018

This PR modifies the way UDP messages are handled to add message priorities. Previously, all messages would be decoded and added FIFO to the handoff queue. Under high pressure, delayed message processing can cause flapping in the cluster. By prioritizing alive messages we minimize the churn by ensuring timely processing and allowing us to ignore older suspect / dead messages.

We add two linked lists, a low and high priority queue. Each queue is LIFO, and we always check the high priority queue for work first. LIFO ordering is preferred to ensure more recent messages are processed first when there is slow processing to avoid churn.

Depends on #158

@bosconi
Copy link
Contributor

bosconi commented Aug 9, 2018

Good stuff. Nice use of LIFO.

@banks
Copy link
Member

banks commented Aug 9, 2018

@armon Hmm you merged #158 but this PR diff still shows the changes introduced there. I rebased it locally and force pushed so you'll need to fetch/checkout again locally if you do more.

I also couldn't get test suite to pass at all locally due to some things that were order-dependent or broken by changes to Go (vet warnings that now fail tests) so some random fixes that at least get it to pass... sometimes. Still lots of flaky timeouts but I have some confidence this is OK now at least.

@banks banks merged commit 2e1a43a into master Aug 9, 2018
@banks banks deleted the f-msg-priority branch August 9, 2018 12:49
mkeeler pushed a commit to hashicorp/consul that referenced this pull request Aug 9, 2018
This includes fixes that improve gossip scalability on very large (> 10k node) clusters.

The Serf changes:
 - take snapshot disk IO out of the critical path for handling messages hashicorp/serf#524
 - make snapshot compaction much less aggressive - the old fixed threshold caused snapshots to be constantly compacted (synchronously with request handling) on clusters larger than about 2000 nodes! hashicorp/serf#525

Memberlist changes:
 - prioritize handling alive messages over suspect/dead to improve stability, and handle queue in LIFO order to avoid acting on info that 's already stale in the queue by the time we handle it. hashicorp/memberlist#159
 - limit the number of concurrent pushPull requests being handled at once to 128. In one test scenario with 10s of thousands of servers we saw channel and lock blocking cause over 3000 pushPulls at once which ballooned the memory of the server because each push pull contained a de-serialised list of all known 10k+ nodes and their tags for a total of about 60 million objects and 7GB of memory stuck. While the rest of the fixes here should prevent the same root cause from blocking in the same way, this prevents any other bug or source of contention from allowing pushPull messages to stack up and eat resources. hashicorp/memberlist#158
thaJeztah added a commit to thaJeztah/libnetwork that referenced this pull request Aug 26, 2019
full diff: hashicorp/memberlist@3d8438d...v0.1.4

- hashicorp/memberlist#158 Limit concurrent push/pull connections
- hashicorp/memberlist#159 Prioritize alive message over other messages
- hashicorp/memberlist#168 Add go.mod
- hashicorp/memberlist#167 Various changes to improve the cpu impact of TransmitLimitedQueue in large clusters
- hashicorp/memberlist#169 added back-off to accept loop to avoid a tight loop
- hashicorp/memberlist#178 Avoid to take into account wrong versions of protocols in Vsn
- hashicorp/memberlist#189 Allow a dead node's name to be taken by a new node

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah added a commit to thaJeztah/libnetwork that referenced this pull request Aug 26, 2019
full diff: hashicorp/memberlist@3d8438d...v0.1.4

- hashicorp/memberlist#158 Limit concurrent push/pull connections
- hashicorp/memberlist#159 Prioritize alive message over other messages
- hashicorp/memberlist#168 Add go.mod
- hashicorp/memberlist#167 Various changes to improve the cpu impact of TransmitLimitedQueue in large clusters
- hashicorp/memberlist#169 added back-off to accept loop to avoid a tight loop
- hashicorp/memberlist#178 Avoid to take into account wrong versions of protocols in Vsn
- hashicorp/memberlist#189 Allow a dead node's name to be taken by a new node

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah added a commit to thaJeztah/libnetwork that referenced this pull request Feb 26, 2020
full diff: hashicorp/memberlist@3d8438d...v0.1.4

- hashicorp/memberlist#158 Limit concurrent push/pull connections
- hashicorp/memberlist#159 Prioritize alive message over other messages
- hashicorp/memberlist#168 Add go.mod
- hashicorp/memberlist#167 Various changes to improve the cpu impact of TransmitLimitedQueue in large clusters
- hashicorp/memberlist#169 added back-off to accept loop to avoid a tight loop
- hashicorp/memberlist#178 Avoid to take into account wrong versions of protocols in Vsn
- hashicorp/memberlist#189 Allow a dead node's name to be taken by a new node

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah added a commit to thaJeztah/libnetwork that referenced this pull request May 11, 2021
full diff: hashicorp/memberlist@3d8438d...v0.1.4

- hashicorp/memberlist#158 Limit concurrent push/pull connections
- hashicorp/memberlist#159 Prioritize alive message over other messages
- hashicorp/memberlist#168 Add go.mod
- hashicorp/memberlist#167 Various changes to improve the cpu impact of TransmitLimitedQueue in large clusters
- hashicorp/memberlist#169 added back-off to accept loop to avoid a tight loop
- hashicorp/memberlist#178 Avoid to take into account wrong versions of protocols in Vsn
- hashicorp/memberlist#189 Allow a dead node's name to be taken by a new node

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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.

3 participants