mirror of
https://github.com/tailscale/tailscale.git
synced 2025-12-01 17:49:02 +00:00
ipn/ipnlocal: do not stall event processing for appc route updates (#17663)
A follow-up to #17411. Put AppConnector events into a task queue, as they may take some time to process. Ensure that the queue is stopped at shutdown so that cleanup will remain orderly. Because events are delivered on a separate goroutine, slow processing of an event does not cause an immediate problem; however, a subscriber that blocks for a long time will push back on the bus as a whole. See https://godoc.org/tailscale.com/util/eventbus#hdr-Expected_subscriber_behavior for more discussion. Updates #17192 Updates #15160 Change-Id: Ib313cc68aec273daf2b1ad79538266c81ef063e3 Signed-off-by: M. J. Fromberger <fromberger@tailscale.com>
This commit is contained in:
@@ -87,6 +87,7 @@ import (
|
||||
"tailscale.com/util/clientmetric"
|
||||
"tailscale.com/util/dnsname"
|
||||
"tailscale.com/util/eventbus"
|
||||
"tailscale.com/util/execqueue"
|
||||
"tailscale.com/util/goroutines"
|
||||
"tailscale.com/util/mak"
|
||||
"tailscale.com/util/osuser"
|
||||
@@ -187,6 +188,7 @@ type LocalBackend struct {
|
||||
statsLogf logger.Logf // for printing peers stats on change
|
||||
sys *tsd.System
|
||||
eventClient *eventbus.Client
|
||||
appcTask execqueue.ExecQueue // handles updates from appc
|
||||
|
||||
health *health.Tracker // always non-nil
|
||||
polc policyclient.Client // always non-nil
|
||||
@@ -613,12 +615,14 @@ func (b *LocalBackend) onAppConnectorRouteUpdate(ru appctype.RouteUpdate) {
|
||||
// We need to find a way to ensure that changes to the backend state are applied
|
||||
// consistently in the presnce of profile changes, which currently may not happen in
|
||||
// a single atomic step. See: https://github.com/tailscale/tailscale/issues/17414
|
||||
if err := b.AdvertiseRoute(ru.Advertise...); err != nil {
|
||||
b.logf("appc: failed to advertise routes: %v: %v", ru.Advertise, err)
|
||||
}
|
||||
if err := b.UnadvertiseRoute(ru.Unadvertise...); err != nil {
|
||||
b.logf("appc: failed to unadvertise routes: %v: %v", ru.Unadvertise, err)
|
||||
}
|
||||
b.appcTask.Add(func() {
|
||||
if err := b.AdvertiseRoute(ru.Advertise...); err != nil {
|
||||
b.logf("appc: failed to advertise routes: %v: %v", ru.Advertise, err)
|
||||
}
|
||||
if err := b.UnadvertiseRoute(ru.Unadvertise...); err != nil {
|
||||
b.logf("appc: failed to unadvertise routes: %v: %v", ru.Unadvertise, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func (b *LocalBackend) onAppConnectorStoreRoutes(ri appctype.RouteInfo) {
|
||||
@@ -1082,6 +1086,7 @@ func (b *LocalBackend) Shutdown() {
|
||||
// 1. Event handlers also acquire b.mu, they can deadlock with c.Shutdown().
|
||||
// 2. Event handlers may not guard against undesirable post/in-progress
|
||||
// LocalBackend.Shutdown() behaviors.
|
||||
b.appcTask.Shutdown()
|
||||
b.eventClient.Close()
|
||||
|
||||
b.em.close()
|
||||
|
||||
Reference in New Issue
Block a user