discovery: restrict P2C to top-two candidates, scope stickiness to single-hop, and add fallback URL tie-breaker

Author: gg582

portal/discovery/mols.go

@@ -273,13 +273,10 @@ func RankRelayPool(autoPool []RelayState, localAddress string, epoch uint64) []s
 		slices.SortFunc(fallbackStates, func(a, b RelayState) int {
 			aRTT := a.effectiveRTT()
 			bRTT := b.effectiveRTT()
-			if aRTT < bRTT {
-				return -1
-			}
-			if aRTT > bRTT {
-				return 1
+			if aRTT != bRTT {
+				return cmp.Compare(aRTT, bRTT)
 			}
-			return 0
+			return cmp.Compare(a.Descriptor.APIHTTPSAddr, b.Descriptor.APIHTTPSAddr)
 		})
 		promote := min(molsMinActiveNodes-len(activeStates), len(fallbackStates))
 		activeStates = append(activeStates, fallbackStates[:promote]...)
@@ -319,27 +316,16 @@ func RankRelayPool(autoPool []RelayState, localAddress string, epoch uint64) []s
 			}
 		}
 
-		// Pressure-aware partitioning: Candidates with significantly elevated pressure
-		// (pressure difference > molsP2CPressureDelta compared to minimum pressure) are
-		// demoted behind low-pressure candidates so they are pushed outside the MaxActiveRelays
-		// quota, achieving real active-set membership migration.
+		// P2C pressure optimization: compare candidate 0 and 1, swap only when
+		// p0 - p1 > molsP2CPressureDelta, and preserve the rest of the MOLS order.
+		// This keeps the pressure correction local to the client's distinct candidate pair
+		// rather than collapsing into a global pressure ordering.
 		if len(nonSaturated) >= 2 {
-			minPressure := nonSaturated[0].state.Pressure()
-			for _, c := range nonSaturated[1:] {
-				if p := c.state.Pressure(); p < minPressure {
-					minPressure = p
-				}
+			p0 := nonSaturated[0].state.Pressure()
+			p1 := nonSaturated[1].state.Pressure()
+			if p0-p1 > molsP2CPressureDelta {
+				nonSaturated[0], nonSaturated[1] = nonSaturated[1], nonSaturated[0]
 			}
-			var lowPressure []molsCandidate
-			var highPressure []molsCandidate
-			for _, c := range nonSaturated {
-				if c.state.Pressure()-minPressure > molsP2CPressureDelta {
-					highPressure = append(highPressure, c)
-				} else {
-					lowPressure = append(lowPressure, c)
-				}
-			}
-			nonSaturated = append(lowPressure, highPressure...)
 		}
 
 		tierOut := make([]string, 0, len(candidates))
@@ -408,31 +394,15 @@ func applyActiveStickiness(ranked []string, activeRelayURLs []string, states []R
 		stateMap[s.Descriptor.APIHTTPSAddr] = s
 	}
 
-	// Compute baseline minimum pressure among selectable candidates (ranked pool only)
-	minPressure := math.MaxFloat64
-	for _, u := range ranked {
-		if s, ok := stateMap[u]; ok {
-			s.EvaluateSaturation()
-			if !s.IsSaturated && !isRelayFallback(s) {
-				if p := s.Pressure(); p < minPressure {
-					minPressure = p
-				}
-			}
-		}
-	}
-
-	// Stickiness is ONLY granted to healthy nodes: non-saturated, non-fallback,
-	// and without significantly elevated pressure (to allow load-shedding migration).
+	// Stickiness is ONLY granted to healthy nodes: non-saturated and non-fallback.
+	// Degraded/saturated nodes must migrate out rather than being resurrected.
 	activeSet := make(map[string]struct{}, len(activeRelayURLs))
 	for _, u := range activeRelayURLs {
 		if s, ok := stateMap[u]; ok {
 			s.EvaluateSaturation()
 			if s.IsSaturated || isRelayFallback(s) {
 				continue
 			}
-			if minPressure != math.MaxFloat64 && s.Pressure()-minPressure > molsP2CPressureDelta {
-				continue
-			}
 			activeSet[u] = struct{}{}
 		}
 	}

portal/discovery/mols_test.go

@@ -269,7 +269,7 @@ func TestMOLSStickinessDoesNotResurrectSaturatedOrFallback(t *testing.T) {
 	}
 }
 
-func TestMOLSMultiHopEntryStickiness(t *testing.T) {
+func TestMOLSMultiHopBypassesSingleHopStickiness(t *testing.T) {
 	now := time.Now().UTC()
 	set := NewRelaySet(nil)
 	var activeEntry string
@@ -299,9 +299,76 @@ func TestMOLSMultiHopEntryStickiness(t *testing.T) {
 	if len(routes) == 0 {
 		t.Fatalf("routes is empty")
 	}
-	// The first entry hop should preserve the active entry relay due to stickiness
-	if entry := routes[0].ListenerRelayURL(); entry != activeEntry {
-		t.Fatalf("first entry hop = %q, want activeEntry %q due to stickiness", entry, activeEntry)
+	// Multi-hop routing bypasses single-hop ActiveRelayURLs stickiness to preserve route-level path generation
+	expectedRoutes, _ := set.PlanRoutes(nil, RouteState{
+		MultiHopDepth:   2,
+		MaxActiveRelays: 2,
+		LocalAddress:    "client",
+	})
+	if routes[0].ListenerRelayURL() != expectedRoutes[0].ListenerRelayURL() {
+		t.Fatalf("multi-hop route should follow pure buildMOLSPaths ordering")
+	}
+}
+
+func TestMOLSFallbackSortURLTieBreaker(t *testing.T) {
+	now := time.Now().UTC()
+	// Create two fallback relays with identical effectiveRTT
+	rB := confirmedRelayState(t, "https://relay-b.example")
+	rB.DiscoveryRTT = 3 * time.Second
+	rB.DiscoveryRTTAt = now
+
+	rA := confirmedRelayState(t, "https://relay-a.example")
+	rA.DiscoveryRTT = 3 * time.Second
+	rA.DiscoveryRTTAt = now
+
+	// Only 1 active state, so fallback promotion will promote one fallback node
+	active := confirmedRelayState(t, "https://relay-active.example")
+	active.DiscoveryRTT = 30 * time.Millisecond
+	active.DiscoveryRTTAt = now
+
+	// Pass in reverse order [rB, rA]
+	relays := []RelayState{active, rB, rA}
+	ranked := RankRelayPool(relays, "client-tie-breaker", 0)
+
+	// Since active pool has 1 node, molsMinActiveNodes (2) causes 1 fallback node to be promoted into active tier.
+	// Between rA and rB (both 3s RTT), rA MUST be promoted due to URL tie-breaker, leaving rB in fallback.
+	if !slices.Contains(ranked[:2], "https://relay-a.example") || ranked[2] != "https://relay-b.example" {
+		t.Fatalf("expected relay-a to be promoted into active tier and relay-b in fallback, got: %v", ranked)
+	}
+}
+
+func TestMOLSP2CLocalChoiceTopTwo(t *testing.T) {
+	now := time.Now().UTC()
+	// Create 4 candidates
+	// R0 has higher pressure than R1 (delta > 0.3)
+	r0 := confirmedRelayState(t, "https://relay-0.example")
+	r0.LoadFactor = 0.6
+	r0.EWMALoad = 0.6
+	r0.LoadDelta = 0.3
+	r0.DiscoveryRTT = 30 * time.Millisecond
+	r0.DiscoveryRTTAt = now
+
+	r1 := confirmedRelayState(t, "https://relay-1.example")
+	r1.LoadFactor = 0.1
+	r1.EWMALoad = 0.1
+	r1.DiscoveryRTT = 30 * time.Millisecond
+	r1.DiscoveryRTTAt = now
+
+	r2 := confirmedRelayState(t, "https://relay-2.example")
+	r2.LoadFactor = 0.1
+	r2.EWMALoad = 0.1
+	r2.DiscoveryRTT = 30 * time.Millisecond
+	r2.DiscoveryRTTAt = now
+
+	relays := []RelayState{r0, r1, r2}
+	ranked := RankRelayPool(relays, "test-client", 0)
+	if len(ranked) != 3 {
+		t.Fatalf("expected 3 ranked relays, got %d", len(ranked))
+	}
+	// Pressure difference between r0 and r1 triggers local P2C swap of index 0 and 1
+	// Verify that the result contains all 3 and preserves valid pool ordering
+	if ranked[0] == "https://relay-0.example" && r0.Pressure()-r1.Pressure() > molsP2CPressureDelta {
+		t.Fatalf("relay-0 should have been swapped with relay-1 due to P2C local choice")
 	}
 }
 
@@ -336,114 +403,3 @@ func BenchmarkMOLSSelectPriorityMassiveScale(b *testing.B) {
 		SelectPriority(relayStates, routeState)
 	}
 }
-
-func TestMOLSPressureEvictionAndHealthyStickiness(t *testing.T) {
-	now := time.Now().UTC()
-	// R0: High pressure active relay
-	r0 := confirmedRelayState(t, "https://relay-0.example")
-	r0.LoadFactor = 0.8
-	r0.EWMALoad = 0.8
-	r0.LoadDelta = 0.3
-	r0.DiscoveryRTT = 30 * time.Millisecond
-	r0.DiscoveryRTTAt = now
-	for i := 0; i < 90; i++ {
-		r0.RTTTracker.Add(10 * time.Millisecond)
-	}
-	for i := 0; i < 10; i++ {
-		r0.RTTTracker.Add(150 * time.Millisecond)
-	}
-
-	// R1, R2, R3, R4: Healthy idle relays
-	healthyRelays := make([]RelayState, 4)
-	for i := range healthyRelays {
-		url := fmt.Sprintf("https://relay-%d.example", i+1)
-		st := confirmedRelayState(t, url)
-		st.LoadFactor = 0.1
-		st.EWMALoad = 0.1
-		st.DiscoveryRTT = 40 * time.Millisecond
-		st.DiscoveryRTTAt = now
-		for j := 0; j < 100; j++ {
-			st.RTTTracker.Add(20 * time.Millisecond)
-		}
-		healthyRelays[i] = st
-	}
-
-	// R5: Saturated relay
-	r5 := confirmedRelayState(t, "https://relay-5.example")
-	r5.IsSaturated = true
-	r5.LoadFactor = 0.95
-
-	allRelays := []RelayState{r0, healthyRelays[0], healthyRelays[1], healthyRelays[2], healthyRelays[3], r5}
-
-	// RouteState with MaxActiveRelays = 3, ActiveRelayURLs = [R0, R1, R2]
-	rs := RouteState{
-		ActiveRelayURLs: []string{
-			"https://relay-0.example",
-			"https://relay-1.example",
-			"https://relay-2.example",
-		},
-		MaxActiveRelays: 3,
-		LocalAddress:    "client-test-addr",
-	}
-
-	selected := SelectPriority(allRelays, rs)
-
-	// 1. High-pressure r0 MUST be evicted from active set (membership migration)
-	if slices.Contains(selected, "https://relay-0.example") {
-		t.Fatalf("high-pressure relay-0 was NOT evicted from active set: %v", selected)
-	}
-
-	// 2. Saturated r5 MUST NOT be resurrected
-	if slices.Contains(selected, "https://relay-5.example") {
-		t.Fatalf("saturated relay-5 was resurrected: %v", selected)
-	}
-
-	// 3. Healthy active relays (relay-1, relay-2) MUST be preserved by stickiness
-	if !slices.Contains(selected, "https://relay-1.example") || !slices.Contains(selected, "https://relay-2.example") {
-		t.Fatalf("healthy active relays were not preserved by stickiness: %v", selected)
-	}
-
-	// 4. Exactly MaxActiveRelays (3) selected
-	if len(selected) != 3 {
-		t.Fatalf("expected 3 selected relays, got %d: %v", len(selected), selected)
-	}
-}
-
-func TestMOLSSkipsIneligibleRelaysWhenComputingPressureBaseline(t *testing.T) {
-	now := time.Now().UTC()
-
-	// Ineligible banned relay with artificially low pressure (0.0)
-	banned := confirmedRelayState(t, "https://relay-banned.example")
-	banned.Banned = true
-	banned.LoadFactor = 0.0
-	banned.EWMALoad = 0.0
-
-	// Eligible active relay with moderate pressure (0.35)
-	active := confirmedRelayState(t, "https://relay-active.example")
-	active.LoadFactor = 0.35
-	active.EWMALoad = 0.35
-	active.DiscoveryRTT = 50 * time.Millisecond
-	active.DiscoveryRTTAt = now
-
-	// Eligible peer with same moderate pressure (0.35)
-	peer := confirmedRelayState(t, "https://relay-peer.example")
-	peer.LoadFactor = 0.35
-	peer.EWMALoad = 0.35
-	peer.DiscoveryRTT = 50 * time.Millisecond
-	peer.DiscoveryRTTAt = now
-
-	relays := []RelayState{banned, active, peer}
-	rs := RouteState{
-		ActiveRelayURLs: []string{"https://relay-active.example"},
-		MaxActiveRelays: 1,
-		LocalAddress:    "client-baseline-test",
-	}
-
-	selected := SelectPriority(relays, rs)
-
-	// If banned relay distorted minPressure to 0.0, active (0.35) would be evicted (> 0.30 delta).
-	// Because banned is excluded from ranked pool, baseline is 0.35, so active MUST be preserved.
-	if len(selected) != 1 || selected[0] != "https://relay-active.example" {
-		t.Fatalf("active relay should be preserved by stickiness, got %v", selected)
-	}
-}

portal/discovery/relayset.go

@@ -483,10 +483,10 @@ func (s *RelaySet) PlanRoutes(explicitPath []string, routeState RouteState) ([]R
 	if maxActive <= 0 {
 		maxActive = defaultMaxActiveRelays
 	}
-	ranked = applyActiveStickiness(ranked, routeState.ActiveRelayURLs, states, maxActive)
 	if routeState.MultiHopDepth > 1 {
 		return buildMOLSPaths(ranked, routeState.MultiHopDepth, maxActive)
 	}
+	ranked = applyActiveStickiness(ranked, routeState.ActiveRelayURLs, states, maxActive)
 	if len(ranked) > maxActive {
 		ranked = ranked[:maxActive]
 	}