Simplify map session management #1931
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kradalby
force-pushed
the
kradalby/close-via-notifier
branch
from
1979589 to
567b34b
Compare
kradalby
changed the title
Signed-off-by: Kristoffer Dalby <[email protected]>
add gauge metrics for waiting for notifier lock add gauge metrics for waiting for notifier batcher lock add gauge metrics for current pending updates add metric and count to String method count mapresps ended add metric for tracking mapresp.close returns add timeout to sendall Signed-off-by: Kristoffer Dalby <[email protected]>
Signed-off-by: Kristoffer Dalby <[email protected]>
this commit removes the mapresponse map as it complicated and caused more problems than it solved. instead, notifier is now changed to always use the last connected nodes channel and to ensure it is not remove until that client is disconnected. as part of this, offline/route change is a bit more concervative and only get sent if the channel was removed from the notifier and not if the clients longpolling session ended (there might be more than one at the time, particularly if the client is connecting.) Signed-off-by: Kristoffer Dalby <[email protected]>
Signed-off-by: Kristoffer Dalby <[email protected]>
Signed-off-by: Kristoffer Dalby <[email protected]>
Signed-off-by: Kristoffer Dalby <[email protected]>
Signed-off-by: Kristoffer Dalby <[email protected]>
Signed-off-by: Kristoffer Dalby <[email protected]>
Signed-off-by: Kristoffer Dalby <[email protected]>
Signed-off-by: Kristoffer Dalby <[email protected]>
Signed-off-by: Kristoffer Dalby <[email protected]>
Signed-off-by: Kristoffer Dalby <[email protected]>
Signed-off-by: Kristoffer Dalby <[email protected]>
Signed-off-by: Kristoffer Dalby <[email protected]>
Signed-off-by: Kristoffer Dalby <[email protected]>
Signed-off-by: Kristoffer Dalby <[email protected]>
Signed-off-by: Kristoffer Dalby <[email protected]>
Signed-off-by: Kristoffer Dalby <[email protected]>
Signed-off-by: Kristoffer Dalby <[email protected]>
Signed-off-by: Kristoffer Dalby <[email protected]>
Signed-off-by: Kristoffer Dalby <[email protected]>
Signed-off-by: Kristoffer Dalby <[email protected]>
kradalby
force-pushed
the
kradalby/close-via-notifier
branch
from
bd7d904 to
00aa63c
Compare
Signed-off-by: Kristoffer Dalby <[email protected]>
|
This not also should fix the two test failures we saw on most PRs |
Signed-off-by: Kristoffer Dalby <[email protected]>
kradalby
force-pushed
the
kradalby/close-via-notifier
branch
from
3c1b83f to
a49755e
Compare
|
Great, will release a new alpha ones its reviewed and merged. |
4 tasks
This was
linked to
issues
May 21, 2024
| RUN apt-get update \ | ||
| && apt-get install -y dnsutils git iptables ssh ca-certificates \ | ||
| && rm -rf /var/lib/apt/lists/* | ||
| FROM golang:1.22-alpine AS build-env |
There was a problem hiding this comment.
Please never use alpine images. Musl C is so broken.
There was a problem hiding this comment.
Please never use alpine images. Musl C is so broken.
Why in general agree with you, this particular case will reproduce the head image so it's more or less the same as all the other ones which we pull from dockerhub and is built by that file.
So in this case this is fixing the odd one out.
juanfont
approved these changes
May 24, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR removes the complicated session management introduced in #1791 which kept track of the sessions in a map, in addition to the channel already kept track of in the notifier.
Instead of trying to close the mapsession, it will now be replaced by the new one and closed after so all new updates goes to the right place.
The map session serve function is also split into a streaming and a non-streaming version for better readability.
RemoveNode in the notifier will not remove a node if the channel is not matching the one that has been passed (e.g. it has been replaced with a new one).
A new tuning parameter has been added to added to set timeout before the notifier gives up to send an update to a node.
Add a keep alive resetter so we wait with sending keep alives if a node has just received an update.
In addition it adds a bunch of env debug flags that can be set:
HEADSCALE_DEBUG_HIGH_CARDINALITY_METRICS: make certain metrics include per node.id, not recommended to use in prod.HEADSCALE_DEBUG_PROFILING_ENABLED: activate tracingHEADSCALE_DEBUG_PROFILING_PATH: where to store tracesHEADSCALE_DEBUG_DUMP_CONFIG: callsspew.Dumpon the config object startupHEADSCALE_DEBUG_DEADLOCK: enable go-deadlock to dump goroutines if it looks like a deadlock has occured, enabled in integration tests.Signed-off-by: Kristoffer Dalby [email protected]