net/netcheck: ensure recent history has a full report
suggestExitNodeLocked now ranks exit node candidates using the per-region latency tracked by the netcheck Client (RecentRegionLatency), which merges the reports retained in c.prev. That history is only useful for far-away regions if it contains a full netcheck report, since incremental reports only re-probe the home region and a handful of the fastest ones. The full-report cadence in GetReport and the c.prev retention window were two independent 5-min constants - the way we schedule netchecks ensured that the history always contaned a full report, but it was not a strong contract and we did not have any checks around this. Now full report interval and retention window are driven by the same var, and a test confirms that the history contains a full report. Updates tailscale/corp#17516 Signed-off-by: Anton Tolchanov <anton@tailscale.com>
This commit is contained in:
committed by
Anton Tolchanov
parent
f442cda999
commit
e9e209673e
@@ -853,7 +853,7 @@ func (c *Client) GetReport(ctx context.Context, dm *tailcfg.DERPMap, opts *GetRe
|
|||||||
}
|
}
|
||||||
|
|
||||||
doFull := false
|
doFull := false
|
||||||
if c.nextFull || now.Sub(c.lastFull) > 5*time.Minute {
|
if c.nextFull || now.Sub(c.lastFull) > fullReportInterval {
|
||||||
doFull = true
|
doFull = true
|
||||||
}
|
}
|
||||||
// If the last report had a captive portal and reported no UDP access,
|
// If the last report had a captive portal and reported no UDP access,
|
||||||
@@ -1329,6 +1329,12 @@ func (c *Client) timeNow() time.Time {
|
|||||||
}
|
}
|
||||||
|
|
||||||
const (
|
const (
|
||||||
|
// fullReportInterval is the maximum time between full netcheck reports.
|
||||||
|
// Once this long has elapsed since the last full report, the next GetReport
|
||||||
|
// re-probes every DERP region rather than only the home and fastest regions
|
||||||
|
// (see GetReport). It also informs retention window for report history
|
||||||
|
// (c.prev).
|
||||||
|
fullReportInterval = 5 * time.Minute
|
||||||
// preferredDERPAbsoluteDiff specifies the minimum absolute difference
|
// preferredDERPAbsoluteDiff specifies the minimum absolute difference
|
||||||
// in latencies between two DERP regions that would cause a node to
|
// in latencies between two DERP regions that would cause a node to
|
||||||
// switch its PreferredDERP ("home DERP"). This ensures that if a node
|
// switch its PreferredDERP ("home DERP"). This ensures that if a node
|
||||||
|
|||||||
@@ -481,6 +481,61 @@ func TestAddReportHistoryAndSetPreferredDERP(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestRecentReportsRetainFullNetcheck confirms that the recent-report history
|
||||||
|
// (c.prev) always retains at least one full netcheck report, so
|
||||||
|
// RecentRegionLatency covers every DERP region even when the most recent
|
||||||
|
// reports are incremental.
|
||||||
|
func TestRecentReportsRetainFullNetcheck(t *testing.T) {
|
||||||
|
dm := &tailcfg.DERPMap{
|
||||||
|
Regions: map[int]*tailcfg.DERPRegion{
|
||||||
|
1: {RegionID: 1},
|
||||||
|
2: {RegionID: 2},
|
||||||
|
3: {RegionID: 3},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
allRegions := []int{1, 2, 3}
|
||||||
|
incrementalRegions := []int{1, 2} // home + fastest; never includes region 3
|
||||||
|
|
||||||
|
var now time.Time
|
||||||
|
c := &Client{TimeNow: func() time.Time { return now }}
|
||||||
|
|
||||||
|
mkReport := func(regions []int) *Report {
|
||||||
|
r := &Report{RegionLatency: map[int]time.Duration{}}
|
||||||
|
for _, rid := range regions {
|
||||||
|
r.RegionLatency[rid] = 10 * time.Millisecond
|
||||||
|
}
|
||||||
|
return r
|
||||||
|
}
|
||||||
|
|
||||||
|
// Run one netcheck per minute for an hour, spanning many full-report
|
||||||
|
// intervals.
|
||||||
|
const tick = time.Minute
|
||||||
|
start := time.Unix(1700000000, 0)
|
||||||
|
var lastFull time.Time // zero => first report is full, as in GetReport
|
||||||
|
for i := 0; i < 60; i++ {
|
||||||
|
now = start.Add(time.Duration(i) * tick)
|
||||||
|
|
||||||
|
// Mirror GetReport's full-vs-incremental decision.
|
||||||
|
doFull := now.Sub(lastFull) > fullReportInterval
|
||||||
|
regions := incrementalRegions
|
||||||
|
if doFull {
|
||||||
|
regions = allRegions
|
||||||
|
lastFull = now
|
||||||
|
}
|
||||||
|
c.addReportHistoryAndSetPreferredDERP(&reportState{c: c, start: now}, mkReport(regions), dm.View())
|
||||||
|
|
||||||
|
// Recent latency must always cover every region, which is only
|
||||||
|
// possible while a full report remains in c.prev.
|
||||||
|
got := c.RecentRegionLatency()
|
||||||
|
for _, rid := range allRegions {
|
||||||
|
if _, ok := got[rid]; !ok {
|
||||||
|
t.Fatalf("after report %d at +%s (full=%v): region %d missing from RecentRegionLatency %v; no full report retained in c.prev",
|
||||||
|
i, now.Sub(start), doFull, rid, got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestMakeProbePlan(t *testing.T) {
|
func TestMakeProbePlan(t *testing.T) {
|
||||||
// basicMap has 5 regions. each region has a number of nodes
|
// basicMap has 5 regions. each region has a number of nodes
|
||||||
// equal to the region number (1 has 1a, 2 has 2a and 2b, etc.)
|
// equal to the region number (1 has 1a, 2 has 2a and 2b, etc.)
|
||||||
|
|||||||
Reference in New Issue
Block a user