Stop loading the entire node set into memory per tsh ssh connection #12014
Conversation
rosstimothy
force-pushed
the
tross/node_watcher
branch
7 times, most recently
from
7f09c3f to
95524ce
Compare
github-actions
bot
added
the
tsh
rosstimothy
added
backport-required
robustness
|
friendly ping @smallinsky @ibeckermayer |
There was a problem hiding this comment.
@rosstimothy
Sorry for the delay. I have left one question about NodeWatcher approach vs access point cache where the NodeType is already watched
teleport/lib/cache/collections.go
Line 691 in 1aa38f4
@smallinsky the teleport/lib/srv/regular/proxy.go Line 333 in 1aa38f4 By leveraging the |
There was a problem hiding this comment.
By leveraging the NodeWatcher we can be sure that the entire node set is only loaded into memory once, and by storing them as types.Server we don't have to pay the cost to unmarshal from JSON. The method by which a caller retrieves nodes from the NodeWatcher is also meant to ensure that only copies of the matched subset are made.
All right, thanks for the explanation.
I was curios what is actually benefit and checked benchmark for old cache approach vs watcher for 40_000 nodes:
BenchmarkListNodesWatcher-8 16 68339640 ns/op 29017941 B/op 40183 allocs/op
BenchmarkListMaxNodes-8 5 229637793 ns/op 80429200 B/op 1520193 allocs/op
I have left some minor comment otherwise it LGTM.
When doing a `tsh ssh node` the proxySubsys would load the entire set of nodes into memory to determine which server to route the request to. Under heavy load, like an automated bot that periodically spawns numerous session, a proxy could easily consume all available memory. To do prevent this, proxies now utilize a NodeWatcher, that maintains a single node set in memory. This prevents loading the nodes into memroy more than once, and also eliminates the need to unmarshal the types.Server on each retrieval of the nodes. The NodeWatcher only provides a GetNodes function that require a filter function to make it intentionally challenging to retrieve a copy of the entire node set.
rosstimothy
force-pushed
the
tross/node_watcher
branch
from
038b4c2 to
ff8723d
Compare
There was a problem hiding this comment.
@rosstimothy apologies, I've been head down and wasn't giving needed attention to my GitHub notifications.
Possibly a naive question and/or the premise may be false, but aren't we using the GetNodes you're replacing in many other parts of the codebase? Why should those not also be replaced with a call to a NodeWatcher to prevent a similar potential problem?
Good question. Anything service other than auth which could potentially call |
Sounds good, your call whether to refactor them here or just create an issue for it. |
Addressed this in 18f9570 |
…12014) * Prevent proxy from loading entire node set into memory more than once When establishing a new session to a node, the proxy would load the entire node set into memory in an attempt to find the matching host. For smaller clusters this may not be that problematic. But on larger clusters, loading >40k nodes into memory from the cache can be quite expensive. This problem is compounded by the fact that it happened**per** session, which could potentially cause the proxy to consume all available memory and be OOM killed. A new `NodeWatcher` is introduced which will maintain an in memory list of all nodes per process. The watcher leverages the existing resource watcher system and stores all nodes as types.Server, to eliminate the cost incurred by unmarshalling the nodes from the cache. The `NodeWatcher` provides a way to retrieve a filtered list of nodes in order to reduce the number of copies made to only the matches. (cherry picked from commit fa12352) # Conflicts: # lib/reversetunnel/api.go # lib/service/service.go # lib/services/watcher.go # lib/srv/regular/proxy.go # lib/srv/regular/sshserver.go # lib/web/apiserver.go # lib/web/apiserver_test.go # lib/web/terminal.go # tool/tsh/proxy_test.go # tool/tsh/tsh.go
…12014) * Prevent proxy from loading entire node set into memory more than once When establishing a new session to a node, the proxy would load the entire node set into memory in an attempt to find the matching host. For smaller clusters this may not be that problematic. But on larger clusters, loading >40k nodes into memory from the cache can be quite expensive. This problem is compounded by the fact that it happened**per** session, which could potentially cause the proxy to consume all available memory and be OOM killed. A new `NodeWatcher` is introduced which will maintain an in memory list of all nodes per process. The watcher leverages the existing resource watcher system and stores all nodes as types.Server, to eliminate the cost incurred by unmarshalling the nodes from the cache. The `NodeWatcher` provides a way to retrieve a filtered list of nodes in order to reduce the number of copies made to only the matches. (cherry picked from commit fa12352) # Conflicts: # lib/services/watcher.go # lib/web/apiserver_test.go
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
…12014) * Prevent proxy from loading entire node set into memory more than once When establishing a new session to a node, the proxy would load the entire node set into memory in an attempt to find the matching host. For smaller clusters this may not be that problematic. But on larger clusters, loading >40k nodes into memory from the cache can be quite expensive. This problem is compounded by the fact that it happened**per** session, which could potentially cause the proxy to consume all available memory and be OOM killed. A new `NodeWatcher` is introduced which will maintain an in memory list of all nodes per process. The watcher leverages the existing resource watcher system and stores all nodes as types.Server, to eliminate the cost incurred by unmarshalling the nodes from the cache. The `NodeWatcher` provides a way to retrieve a filtered list of nodes in order to reduce the number of copies made to only the matches. (cherry picked from commit fa12352)
…12014) (#12571) * Prevent proxy from loading entire node set into memory more than once When establishing a new session to a node, the proxy would load the entire node set into memory in an attempt to find the matching host. For smaller clusters this may not be that problematic. But on larger clusters, loading >40k nodes into memory from the cache can be quite expensive. This problem is compounded by the fact that it happened**per** session, which could potentially cause the proxy to consume all available memory and be OOM killed. A new `NodeWatcher` is introduced which will maintain an in memory list of all nodes per process. The watcher leverages the existing resource watcher system and stores all nodes as types.Server, to eliminate the cost incurred by unmarshalling the nodes from the cache. The `NodeWatcher` provides a way to retrieve a filtered list of nodes in order to reduce the number of copies made to only the matches. (cherry picked from commit fa12352)
…12014) * Prevent proxy from loading entire node set into memory more than once When establishing a new session to a node, the proxy would load the entire node set into memory in an attempt to find the matching host. For smaller clusters this may not be that problematic. But on larger clusters, loading >40k nodes into memory from the cache can be quite expensive. This problem is compounded by the fact that it happened**per** session, which could potentially cause the proxy to consume all available memory and be OOM killed. A new `NodeWatcher` is introduced which will maintain an in memory list of all nodes per process. The watcher leverages the existing resource watcher system and stores all nodes as types.Server, to eliminate the cost incurred by unmarshalling the nodes from the cache. The `NodeWatcher` provides a way to retrieve a filtered list of nodes in order to reduce the number of copies made to only the matches. (cherry picked from commit fa12352)
…12014) * Prevent proxy from loading entire node set into memory more than once When establishing a new session to a node, the proxy would load the entire node set into memory in an attempt to find the matching host. For smaller clusters this may not be that problematic. But on larger clusters, loading >40k nodes into memory from the cache can be quite expensive. This problem is compounded by the fact that it happened**per** session, which could potentially cause the proxy to consume all available memory and be OOM killed. A new `NodeWatcher` is introduced which will maintain an in memory list of all nodes per process. The watcher leverages the existing resource watcher system and stores all nodes as types.Server, to eliminate the cost incurred by unmarshalling the nodes from the cache. The `NodeWatcher` provides a way to retrieve a filtered list of nodes in order to reduce the number of copies made to only the matches. (cherry picked from commit fa12352)
…12014) * Prevent proxy from loading entire node set into memory more than once When establishing a new session to a node, the proxy would load the entire node set into memory in an attempt to find the matching host. For smaller clusters this may not be that problematic. But on larger clusters, loading >40k nodes into memory from the cache can be quite expensive. This problem is compounded by the fact that it happened**per** session, which could potentially cause the proxy to consume all available memory and be OOM killed. A new `NodeWatcher` is introduced which will maintain an in memory list of all nodes per process. The watcher leverages the existing resource watcher system and stores all nodes as types.Server, to eliminate the cost incurred by unmarshalling the nodes from the cache. The `NodeWatcher` provides a way to retrieve a filtered list of nodes in order to reduce the number of copies made to only the matches. (cherry picked from commit fa12352)
…12014) * Prevent proxy from loading entire node set into memory more than once When establishing a new session to a node, the proxy would load the entire node set into memory in an attempt to find the matching host. For smaller clusters this may not be that problematic. But on larger clusters, loading >40k nodes into memory from the cache can be quite expensive. This problem is compounded by the fact that it happened**per** session, which could potentially cause the proxy to consume all available memory and be OOM killed. A new `NodeWatcher` is introduced which will maintain an in memory list of all nodes per process. The watcher leverages the existing resource watcher system and stores all nodes as types.Server, to eliminate the cost incurred by unmarshalling the nodes from the cache. The `NodeWatcher` provides a way to retrieve a filtered list of nodes in order to reduce the number of copies made to only the matches. (cherry picked from commit fa12352)
…12014) (#12562) * Prevent proxy from loading entire node set into memory more than once When establishing a new session to a node, the proxy would load the entire node set into memory in an attempt to find the matching host. For smaller clusters this may not be that problematic. But on larger clusters, loading >40k nodes into memory from the cache can be quite expensive. This problem is compounded by the fact that it happened**per** session, which could potentially cause the proxy to consume all available memory and be OOM killed. A new `NodeWatcher` is introduced which will maintain an in memory list of all nodes per process. The watcher leverages the existing resource watcher system and stores all nodes as types.Server, to eliminate the cost incurred by unmarshalling the nodes from the cache. The `NodeWatcher` provides a way to retrieve a filtered list of nodes in order to reduce the number of copies made to only the matches. (cherry picked from commit fa12352)
…12014) (#12573) * Prevent proxy from loading entire node set into memory more than once When establishing a new session to a node, the proxy would load the entire node set into memory in an attempt to find the matching host. For smaller clusters this may not be that problematic. But on larger clusters, loading >40k nodes into memory from the cache can be quite expensive. This problem is compounded by the fact that it happened**per** session, which could potentially cause the proxy to consume all available memory and be OOM killed. A new `NodeWatcher` is introduced which will maintain an in memory list of all nodes per process. The watcher leverages the existing resource watcher system and stores all nodes as types.Server, to eliminate the cost incurred by unmarshalling the nodes from the cache. The `NodeWatcher` provides a way to retrieve a filtered list of nodes in order to reduce the number of copies made to only the matches. (cherry picked from commit fa12352)
rosstimothy
changed the title
When doing a
tsh ssh nodethe proxySubsys would load the entireset of nodes into memory to determine which server to route the
request to. Under heavy load, like an automated bot that periodically
spawns numerous session, a proxy could easily consume all
available memory.
To do prevent this, proxies now utilize a NodeWatcher, that maintains
a single node set in memory. This prevents loading the nodes into
memroy more than once, and also eliminates the need to unmarshal the
types.Server on each retrieval of the nodes. The NodeWatcher only
provides a GetNodes function that require a filter function to make
it intentionally challenging to retrieve a copy of the entire node set.