RSDK-11744 - Increase initial read timeout to 15s, settable by env var (#5234)

Author: cheukt

config/reader.go

@@ -227,7 +227,7 @@ func readFromCloud(
 	if !cfg.Cloud.SignalingInsecure && (checkForNewCert || tls.certificate == "" || tls.privateKey == "") {
 		logger.Debug("reading tlsCertificate from the cloud")
 
-		ctxWithTimeout, cancel := contextutils.GetTimeoutCtx(ctx, shouldReadFromCache, cloudCfg.ID)
+		ctxWithTimeout, cancel := contextutils.GetTimeoutCtx(ctx, shouldReadFromCache, cloudCfg.ID, logger)
 		certData, err := readCertificateDataFromCloudGRPC(ctxWithTimeout, cloudCfg, conn)
 		if err != nil {
 			cancel()
@@ -646,7 +646,7 @@ func getFromCloudOrCache(
 ) (*Config, bool, error) {
 	var cached bool
 
-	ctxWithTimeout, cancel := contextutils.GetTimeoutCtx(ctx, shouldReadFromCache, cloudCfg.ID)
+	ctxWithTimeout, cancel := contextutils.GetTimeoutCtx(ctx, shouldReadFromCache, cloudCfg.ID, logger)
 	defer cancel()
 
 	cfg, errorShouldCheckCache, err := getFromCloudGRPC(ctxWithTimeout, cloudCfg, logger, conn)

grpc/app_conn.go

@@ -40,7 +40,7 @@ func NewAppConn(ctx context.Context, appAddress, secret, id string, logger loggi
 		dialOpts = append(dialOpts, rpc.WithInsecure())
 	}
 
-	ctxWithTimeout, ctxWithTimeoutCancel := contextutils.GetTimeoutCtx(ctx, true, id)
+	ctxWithTimeout, ctxWithTimeoutCancel := contextutils.GetTimeoutCtx(ctx, true, id, logger)
 	defer ctxWithTimeoutCancel()
 	// there will always be a deadline
 	if deadline, ok := ctxWithTimeout.Deadline(); ok {

utils/contextutils/context.go

@@ -13,6 +13,7 @@ import (
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/metadata"
 
+	"go.viam.com/rdk/logging"
 	"go.viam.com/rdk/utils"
 )
 
@@ -30,8 +31,8 @@ const (
 	// to the time right after the point cloud was captured.
 	TimeReceivedMetadataKey = "viam-time-received"
 
-	// timeout values to use when reading a config either from App, from App behind a proxy, or from a local (cached) file.
-	readConfigFromCloudTimeout            = 5 * time.Second
+	// Timeout values to use when reading a config either from App behind a proxy, or from App with a local (cached) file.
+	// The timeout is far shorter when a cached config exists because the machine can always fall back to the cached config.
 	readConfigFromCloudBehindProxyTimeout = time.Minute
 	readCachedConfigTimeout               = 1 * time.Second
 )
@@ -86,10 +87,13 @@ func ContextWithTimeoutIfNoDeadline(ctx context.Context, timeout time.Duration)
 	return context.WithCancel(ctx)
 }
 
-// GetTimeoutCtx returns a context [and its cancel function] with a timeout value determined by whether we are behind a proxy and whether a
-// cached config exists.
-func GetTimeoutCtx(ctx context.Context, shouldReadFromCache bool, id string) (context.Context, func()) {
-	timeout := readConfigFromCloudTimeout
+// GetTimeoutCtx returns a context [and its cancel function] with a timeout value determined by whether an environment variable is set,
+// we are behind a proxy and whether a cached config exists. The timeout will always use the environment variable if set.
+func GetTimeoutCtx(ctx context.Context, shouldReadFromCache bool, id string, logger logging.Logger) (context.Context, func()) {
+	timeout, isDefault := utils.GetConfigReadTimeout(logger)
+	if !isDefault {
+		return context.WithTimeout(ctx, timeout)
+	}
 	// When environment indicates we are behind a proxy, bump timeout. Network
 	// operations tend to take longer when behind a proxy.
 	if proxyAddr := os.Getenv(rpc.SocksProxyEnvVar); proxyAddr != "" {

utils/env.go

@@ -31,6 +31,15 @@ const (
 	// that modules are allowed to startup.
 	ModuleStartupTimeoutEnvVar = "VIAM_MODULE_STARTUP_TIMEOUT"
 
+	// DefaultConfigReadTimeout is the default config read timeout. If there
+	// is a cached config on the machine, a shorter default timeout will be used.
+	DefaultConfigReadTimeout = 15 * time.Second
+
+	// ConfigReadTimeoutEnvVar is the environment variable that can
+	// be set to override DefaultConfigReadTimeout as the duration
+	// for config read.
+	ConfigReadTimeoutEnvVar = "VIAM_CONFIG_READ_TIMEOUT"
+
 	// AndroidFilesDir is hardcoded because golang inits before our android code can override HOME var.
 	AndroidFilesDir = "/data/user/0/com.viam.rdk.fgservice/cache"
 
@@ -83,26 +92,34 @@ var windowsPathRegex = regexp.MustCompile(`^(\w:)?(.+)$`)
 // timeout (env variable value if set, DefaultResourceConfigurationTimeout
 // otherwise).
 func GetResourceConfigurationTimeout(logger logging.Logger) time.Duration {
-	return timeoutHelper(DefaultResourceConfigurationTimeout, ResourceConfigurationTimeoutEnvVar, logger)
+	timeout, _ := timeoutHelper(DefaultResourceConfigurationTimeout, ResourceConfigurationTimeoutEnvVar, logger)
+	return timeout
 }
 
 // GetModuleStartupTimeout calculates the module startup timeout
 // (env variable value if set, DefaultModuleStartupTimeout otherwise).
 func GetModuleStartupTimeout(logger logging.Logger) time.Duration {
-	return timeoutHelper(DefaultModuleStartupTimeout, ModuleStartupTimeoutEnvVar, logger)
+	timeout, _ := timeoutHelper(DefaultModuleStartupTimeout, ModuleStartupTimeoutEnvVar, logger)
+	return timeout
+}
+
+// GetConfigReadTimeout returns the config read timeout set by the env variable value,
+// DefaultConfigReadTimeout otherwise.
+func GetConfigReadTimeout(logger logging.Logger) (time.Duration, bool) {
+	return timeoutHelper(DefaultConfigReadTimeout, ConfigReadTimeoutEnvVar, logger)
 }
 
-func timeoutHelper(defaultTimeout time.Duration, timeoutEnvVar string, logger logging.Logger) time.Duration {
+func timeoutHelper(defaultTimeout time.Duration, timeoutEnvVar string, logger logging.Logger) (time.Duration, bool) {
 	if timeoutVal := os.Getenv(timeoutEnvVar); timeoutVal != "" {
 		timeout, err := time.ParseDuration(timeoutVal)
 		if err != nil {
 			logger.Warnf("Failed to parse %s env var, falling back to default %v timeout",
 				timeoutEnvVar, defaultTimeout)
-			return defaultTimeout
+			return defaultTimeout, true
 		}
-		return timeout
+		return timeout, false
 	}
-	return defaultTimeout
+	return defaultTimeout, true
 }
 
 // PlatformHomeDir wraps Getenv("HOME"), except on android, where it returns the app cache directory.

web/server/entrypoint.go

@@ -77,6 +77,9 @@ func logViamEnvVariables(logger logging.Logger) {
 	if value, exists := os.LookupEnv("VIAM_MODULE_STARTUP_TIMEOUT"); exists {
 		viamEnvVariables = append(viamEnvVariables, "VIAM_MODULE_STARTUP_TIMEOUT", value)
 	}
+	if value, exists := os.LookupEnv("VIAM_CONFIG_READ_TIMEOUT"); exists {
+		viamEnvVariables = append(viamEnvVariables, "VIAM_CONFIG_READ_TIMEOUT", value)
+	}
 	if value, exists := os.LookupEnv("CWD"); exists {
 		viamEnvVariables = append(viamEnvVariables, "CWD", value)
 	}