Merge pull request #708 from rod-hynes/proxy-max-backoff-tactic

Proxy max backoff tactic

psiphon/common/inproxy/coordinator.go

@@ -182,6 +182,7 @@ type BrokerDialCoordinator interface {
 	SessionHandshakeRoundTripTimeout() time.Duration
 	AnnounceRequestTimeout() time.Duration
 	AnnounceDelay() time.Duration
+	AnnounceMaxBackoffDelay() time.Duration
 	AnnounceDelayJitter() float64
 	AnswerRequestTimeout() time.Duration
 	OfferRequestTimeout() time.Duration

psiphon/common/inproxy/coordinator_test.go

@@ -50,6 +50,7 @@ type testBrokerDialCoordinator struct {
 	sessionHandshakeRoundTripTimeout  time.Duration
 	announceRequestTimeout            time.Duration
 	announceDelay                     time.Duration
+	announceMaxBackoffDelay           time.Duration
 	announceDelayJitter               float64
 	answerRequestTimeout              time.Duration
 	offerRequestTimeout               time.Duration
@@ -149,6 +150,12 @@ func (t *testBrokerDialCoordinator) AnnounceDelay() time.Duration {
 	return t.announceDelay
 }
 
+func (t *testBrokerDialCoordinator) AnnounceMaxBackoffDelay() time.Duration {
+	t.mutex.Lock()
+	defer t.mutex.Unlock()
+	return t.announceMaxBackoffDelay
+}
+
 func (t *testBrokerDialCoordinator) AnnounceDelayJitter() float64 {
 	t.mutex.Lock()
 	defer t.mutex.Unlock()

psiphon/common/inproxy/proxy.go

@@ -272,13 +272,14 @@ loop:
 // proxyOneClient fails. As having no broker clients is a possible
 // proxyOneClient failure case, GetBrokerClient errors are ignored here and
 // defaults used in that case.
-func (p *Proxy) getAnnounceDelayParameters() (time.Duration, float64) {
+func (p *Proxy) getAnnounceDelayParameters() (time.Duration, time.Duration, float64) {
 	brokerClient, err := p.config.GetBrokerClient()
 	if err != nil {
-		return proxyAnnounceDelay, proxyAnnounceDelayJitter
+		return proxyAnnounceDelay, proxyAnnounceMaxBackoffDelay, proxyAnnounceDelayJitter
 	}
 	brokerCoordinator := brokerClient.GetBrokerDialCoordinator()
 	return common.ValueOrDefault(brokerCoordinator.AnnounceDelay(), proxyAnnounceDelay),
+		common.ValueOrDefault(brokerCoordinator.AnnounceMaxBackoffDelay(), proxyAnnounceMaxBackoffDelay),
 		common.ValueOrDefault(brokerCoordinator.AnnounceDelayJitter(), proxyAnnounceDelayJitter)
 
 }
@@ -384,6 +385,10 @@ func (p *Proxy) proxyClients(
 		backOff, err := p.proxyOneClient(
 			ctx, logAnnounce, signalAnnounceDone)
 
+		if !backOff || err == nil {
+			failureDelayFactor = 1
+		}
+
 		if err != nil && ctx.Err() == nil {
 
 			// Apply a simple exponential backoff based on whether
@@ -396,17 +401,12 @@ func (p *Proxy) proxyClients(
 			// prevents both excess local logging and churning in the former
 			// case and excessive bad service to clients or unintentionally
 			// overloading the broker in the latter case.
-			//
-			// TODO: specific tactics parameters to control this logic.
 
-			delay, jitter := p.getAnnounceDelayParameters()
+			delay, maxBackoffDelay, jitter := p.getAnnounceDelayParameters()
 
-			if !backOff {
-				failureDelayFactor = 1
-			}
 			delay = delay * failureDelayFactor
-			if delay > proxyAnnounceMaxBackoffDelay {
-				delay = proxyAnnounceMaxBackoffDelay
+			if delay > maxBackoffDelay {
+				delay = maxBackoffDelay
 			}
 			if failureDelayFactor < 1<<20 {
 				failureDelayFactor *= 2
@@ -608,7 +608,7 @@ func (p *Proxy) proxyOneClient(
 	// any deliberate delay.
 
 	requestDelay := time.Duration(0)
-	announceDelay, announceDelayJitter := p.getAnnounceDelayParameters()
+	announceDelay, _, announceDelayJitter := p.getAnnounceDelayParameters()
 	p.nextAnnounceMutex.Lock()
 	nextDelay := prng.JitterDuration(announceDelay, announceDelayJitter)
 	if p.nextAnnounceBrokerClient != brokerClient {

psiphon/common/parameters/parameters.go

@@ -411,6 +411,7 @@ const (
 	InproxyProxyAnnounceRequestTimeout                 = "InproxyProxyAnnounceRequestTimeout"
 	InproxyProxyAnnounceDelay                          = "InproxyProxyAnnounceDelay"
 	InproxyProxyAnnounceDelayJitter                    = "InproxyProxyAnnounceDelayJitter"
+	InproxyProxyAnnounceMaxBackoffDelay                = "InproxyProxyAnnounceMaxBackoffDelay"
 	InproxyProxyAnswerRequestTimeout                   = "InproxyProxyAnswerRequestTimeout"
 	InproxyClientOfferRequestTimeout                   = "InproxyClientOfferRequestTimeout"
 	InproxyClientOfferRequestPersonalTimeout           = "InproxyClientOfferRequestPersonalTimeout"
@@ -930,6 +931,7 @@ var defaultParameters = map[string]struct {
 	InproxyProxyAnnounceRequestTimeout:                 {value: 2*time.Minute + 10*time.Second, minimum: time.Duration(0)},
 	InproxyProxyAnnounceDelay:                          {value: 100 * time.Millisecond, minimum: time.Duration(0)},
 	InproxyProxyAnnounceDelayJitter:                    {value: 0.5, minimum: 0.0},
+	InproxyProxyAnnounceMaxBackoffDelay:                {value: 1 * time.Minute, minimum: time.Duration(0)},
 	InproxyProxyAnswerRequestTimeout:                   {value: 10*time.Second + 10*time.Second, minimum: time.Duration(0)},
 	InproxyClientOfferRequestTimeout:                   {value: 10*time.Second + 10*time.Second, minimum: time.Duration(0)},
 	InproxyClientOfferRequestPersonalTimeout:           {value: 5*time.Second + 10*time.Second, minimum: time.Duration(0)},

psiphon/config.go

@@ -996,6 +996,7 @@ type Config struct {
 	InproxyMaxCompartmentIDListLength                       *int
 	InproxyProxyAnnounceRequestTimeoutMilliseconds          *int
 	InproxyProxyAnnounceDelayMilliseconds                   *int
+	InproxyProxyAnnounceMaxBackoffDelayMilliseconds         *int
 	InproxyProxyAnnounceDelayJitter                         *float64
 	InproxyProxyAnswerRequestTimeoutMilliseconds            *int
 	InproxyClientOfferRequestTimeoutMilliseconds            *int
@@ -2529,6 +2530,10 @@ func (config *Config) makeConfigParameters() map[string]interface{} {
 		applyParameters[parameters.InproxyProxyAnnounceDelay] = fmt.Sprintf("%dms", *config.InproxyProxyAnnounceDelayMilliseconds)
 	}
 
+	if config.InproxyProxyAnnounceMaxBackoffDelayMilliseconds != nil {
+		applyParameters[parameters.InproxyProxyAnnounceMaxBackoffDelay] = fmt.Sprintf("%dms", *config.InproxyProxyAnnounceMaxBackoffDelayMilliseconds)
+	}
+
 	if config.InproxyProxyAnnounceDelayJitter != nil {
 		applyParameters[parameters.InproxyProxyAnnounceDelayJitter] = *config.InproxyProxyAnnounceDelayJitter
 	}

psiphon/inproxy.go

@@ -275,6 +275,7 @@ type InproxyBrokerClientInstance struct {
 	sessionHandshakeTimeout       time.Duration
 	announceRequestTimeout        time.Duration
 	announceDelay                 time.Duration
+	announceMaxBackoffDelay       time.Duration
 	announceDelayJitter           float64
 	answerRequestTimeout          time.Duration
 	offerRequestTimeout           time.Duration
@@ -520,6 +521,7 @@ func NewInproxyBrokerClientInstance(
 		sessionHandshakeTimeout:       p.Duration(parameters.InproxySessionHandshakeRoundTripTimeout),
 		announceRequestTimeout:        p.Duration(parameters.InproxyProxyAnnounceRequestTimeout),
 		announceDelay:                 p.Duration(parameters.InproxyProxyAnnounceDelay),
+		announceMaxBackoffDelay:       p.Duration(parameters.InproxyProxyAnnounceMaxBackoffDelay),
 		announceDelayJitter:           p.Float(parameters.InproxyProxyAnnounceDelayJitter),
 		answerRequestTimeout:          p.Duration(parameters.InproxyProxyAnswerRequestTimeout),
 		offerRequestTimeout:           p.Duration(parameters.InproxyClientOfferRequestTimeout),
@@ -1007,6 +1009,11 @@ func (b *InproxyBrokerClientInstance) AnnounceDelay() time.Duration {
 	return b.announceDelay
 }
 
+// Implements the inproxy.BrokerDialCoordinator interface.
+func (b *InproxyBrokerClientInstance) AnnounceMaxBackoffDelay() time.Duration {
+	return b.announceMaxBackoffDelay
+}
+
 // Implements the inproxy.BrokerDialCoordinator interface.
 func (b *InproxyBrokerClientInstance) AnnounceDelayJitter() float64 {
 	return b.announceDelayJitter

psiphon/net.go

@@ -156,20 +156,29 @@ type HasIPv6RouteGetter interface {
 // provider, which returns an identifier for the host's current active
 // network.
 //
-// The identifier is a string that should indicate the network type and
+// The identifier is a string that indicates the network type and
 // identity; for example "WIFI-<BSSID>" or "MOBILE-<MCC/MNC>". As this network
 // ID is personally identifying, it is only used locally in the client to
 // determine network context and is not sent to the Psiphon server. The
 // identifer will be logged in diagnostics messages; in this case only the
 // substring before the first "-" is logged, so all PII must appear after the
 // first "-".
 //
-// NetworkIDGetter.GetNetworkID should always return an identifier value, as
+// NetworkIDGetter.GetNetworkID must always return an identifier value, as
 // logic that uses GetNetworkID, including tactics, is intended to proceed
-// regardless of whether an accurate network identifier can be obtained. By
-// convention, the provider should return "UNKNOWN" when an accurate network
+// regardless of whether an accurate network identifier can be obtained. The
+// the provider shall return "UNKNOWN" when an accurate network
 // identifier cannot be obtained. Best-effort is acceptable: e.g., return just
 // "WIFI" when only the type of the network but no details can be determined.
+//
+// The network type is sent to Psiphon servers and logged as
+// server_tunnel.network_type. To ensure consistency in stats, all providers
+// must use the same network type string values, currently consisting of:
+// - "WIFI" for a Wi-Fi network
+// - "MOBILE" for a mobile/cellular network
+// - "WIRED" for a wired network
+// - "VPN" for a VPN network
+// - "UNKNOWN" for when the network type cannot be determined
 type NetworkIDGetter interface {
 	GetNetworkID() string
 }