Merge #144028

144028: *: audit all duration settings to ensure they have validation r=yuzefovich a=yuzefovich

This PR contains a couple of commits that audit all duration settings and ensure that they have validation functions. In most cases where there wasn't one, non-negative validation is added. The exceptions are:
- positive validation:
  - `kv.gc.txn_cleanup_threshold`
  - `jobs.scheduler.pace`
  - `sql.stats.max_timestamp_age`.
- non-positive validation:
  - `sql.crdb_internal.table_row_statistics.as_of_time`.

Non-negative validation is now assumed to be the default, so all its explicit usages are removed.

See each commit for details.

Epic: None
Release note: None

Co-authored-by: Yahor Yuzefovich <[email protected]>

Author: craig[bot]

pkg/backup/backup_processor.go

@@ -53,21 +53,18 @@ var (
 		"bulkio.backup.read_with_priority_after",
 		"amount of time since the read-as-of time above which a BACKUP should use priority when retrying reads",
 		time.Minute,
-		settings.NonNegativeDuration,
 		settings.WithPublic)
 	delayPerAttempt = settings.RegisterDurationSetting(
 		settings.ApplicationLevel,
 		"bulkio.backup.read_retry_delay",
 		"amount of time since the read-as-of time, per-prior attempt, to wait before making another attempt",
 		time.Second*5,
-		settings.NonNegativeDuration,
 	)
 	timeoutPerAttempt = settings.RegisterDurationSetting(
 		settings.ApplicationLevel,
 		"bulkio.backup.read_timeout",
 		"amount of time after which a read attempt is considered timed out, which causes the backup to fail",
 		time.Minute*5,
-		settings.NonNegativeDuration,
 		settings.WithPublic)
 
 	preSplitExports = settings.RegisterBoolSetting(

pkg/ccl/changefeedccl/cdcevent/rowfetcher_cache.go

@@ -38,7 +38,6 @@ var traceKVLogFrequency = settings.RegisterDurationSetting(
 	"changefeed.cdcevent.trace_kv.log_frequency",
 	"controls how frequently KVs are logged when KV tracing is enabled",
 	500*time.Millisecond,
-	settings.NonNegativeDuration,
 )
 
 // rowFetcherCache maintains a cache of single table row.Fetchers. Given a key

pkg/ccl/changefeedccl/changefeed_processors.go

@@ -694,7 +694,6 @@ var aggregatorHeartbeatFrequency = settings.RegisterDurationSetting(
 	"changefeed aggregator will emit a heartbeat message to the coordinator with this frequency; 0 disables. "+
 		"The setting value should be <=1/2 of server.shutdown.jobs.timeout period",
 	4*time.Second,
-	settings.NonNegativeDuration,
 )
 
 var aggregatorFlushJitter = settings.RegisterFloatSetting(

pkg/ccl/changefeedccl/changefeedbase/settings.go

@@ -23,7 +23,6 @@ var TableDescriptorPollInterval = settings.RegisterDurationSetting(
 	"changefeed.experimental_poll_interval",
 	"polling interval for the table descriptors",
 	1*time.Second,
-	settings.NonNegativeDuration,
 )
 
 // DefaultMinCheckpointFrequency is the default frequency to flush sink.
@@ -53,7 +52,6 @@ var SlowSpanLogThreshold = settings.RegisterDurationSetting(
 	"changefeed.slow_span_log_threshold",
 	"a changefeed will log spans with resolved timestamps this far behind the current wall-clock time; if 0, a default value is calculated based on other cluster settings",
 	0,
-	settings.NonNegativeDuration,
 )
 
 // IdleTimeout controls how long the changefeed will wait for a new KV being
@@ -63,7 +61,6 @@ var IdleTimeout = settings.RegisterDurationSetting(
 	"changefeed.idle_timeout",
 	"a changefeed will mark itself idle if no changes have been emitted for greater than this duration; if 0, the changefeed will never be marked idle",
 	10*time.Minute,
-	settings.NonNegativeDuration,
 	settings.WithName("changefeed.auto_idle.timeout"),
 )
 
@@ -75,7 +72,6 @@ var SpanCheckpointInterval = settings.RegisterDurationSetting(
 	"interval at which span-level checkpoints will be written; "+
 		"if 0, span-level checkpoints are disabled",
 	10*time.Minute,
-	settings.NonNegativeDuration,
 	settings.WithName("changefeed.span_checkpoint.interval"),
 )
 
@@ -90,7 +86,6 @@ var SpanCheckpointLagThreshold = settings.RegisterDurationSetting(
 		"to save leading span progress is written; if 0, span-level checkpoints "+
 		"due to lagging spans is disabled",
 	10*time.Minute,
-	settings.NonNegativeDuration,
 	settings.WithPublic,
 	settings.WithName("changefeed.span_checkpoint.lag_threshold"),
 )
@@ -185,7 +180,6 @@ var ResolvedTimestampMinUpdateInterval = settings.RegisterDurationSetting(
 		"updated again; default of 0 means no minimum interval is enforced but "+
 		"updating will still be limited by the average time it takes to checkpoint progress",
 	0,
-	settings.NonNegativeDuration,
 	settings.WithPublic,
 	settings.WithName("changefeed.resolved_timestamp.min_update_interval"),
 )
@@ -227,7 +221,6 @@ var MaxProtectedTimestampAge = settings.RegisterDurationSetting(
 	"changefeed.protect_timestamp.max_age",
 	"fail the changefeed if the protected timestamp age exceeds this threshold; 0 disables expiration",
 	4*24*time.Hour,
-	settings.NonNegativeDuration,
 	settings.WithPublic)
 
 // BatchReductionRetryEnabled enables the temporary reduction of batch sizes upon kafka message too large errors
@@ -328,7 +321,7 @@ var UsageMetricsReportingInterval = settings.RegisterDurationSetting(
 	"changefeed.usage.reporting_interval",
 	"the interval at which the changefeed calculates and updates its usage metric",
 	5*time.Minute,
-	settings.PositiveDuration, settings.DurationInRange(2*time.Minute, 50*time.Minute),
+	settings.DurationInRange(2*time.Minute, 50*time.Minute),
 )
 
 // UsageMetricsReportingTimeoutPercent is the percent of
@@ -354,5 +347,4 @@ var Quantize = settings.RegisterDurationSettingWithExplicitUnit(
 	"changefeed.resolved_timestamp.granularity",
 	"the granularity at which changefeed progress are quantized to make tracking more efficient",
 	0,
-	settings.NonNegativeDuration,
 )

pkg/ccl/changefeedccl/telemetry.go

@@ -181,5 +181,4 @@ var continuousTelemetryInterval = settings.RegisterDurationSetting(
 	"determines the interval at which each node emits continuous telemetry events"+
 		" during the lifespan of every changefeed; setting a zero value disables logging",
 	24*time.Hour,
-	settings.NonNegativeDuration,
 )

pkg/ccl/jwtauthccl/settings.go

@@ -142,7 +142,6 @@ var JWTAuthClientTimeout = settings.RegisterDurationSetting(
 	"sets the client timeout for external calls made during JWT authentication "+
 		"(e.g. fetching JWKS, etc.)",
 	15*time.Second,
-	settings.NonNegativeDuration,
 	settings.WithPublic,
 )
 

pkg/ccl/kvccl/kvfollowerreadsccl/followerreads.go

@@ -39,7 +39,6 @@ var ClosedTimestampPropagationSlack = settings.RegisterDurationSetting(
 		"propagate from a leaseholder to followers. This is taken into account by "+
 		"follower_read_timestamp().",
 	time.Second,
-	settings.NonNegativeDuration,
 )
 
 // getFollowerReadLag returns the (negative) offset duration from hlc.Now()

pkg/ccl/oidcccl/settings.go

@@ -81,7 +81,6 @@ var OIDCAuthClientTimeout = settings.RegisterDurationSetting(
 	"sets the client timeout for external calls made during OIDC authentication "+
 		"(e.g. authorization code flow, etc.)",
 	15*time.Second,
-	settings.NonNegativeDuration,
 	settings.WithPublic,
 )
 

pkg/crosscluster/logical/logical_replication_job.go

@@ -57,8 +57,7 @@ var (
 		settings.ApplicationLevel,
 		"logical_replication.consumer.job_checkpoint_frequency",
 		"controls the frequency with which the job updates their progress; if 0, disabled",
-		10*time.Second,
-		settings.NonNegativeDuration)
+		10*time.Second)
 
 	// heartbeatFrequency controls frequency the stream replication
 	// destination cluster sends heartbeat to the source cluster to keep
@@ -69,7 +68,6 @@ var (
 		"controls frequency the stream replication destination cluster sends heartbeat "+
 			"to the source cluster to keep the stream alive",
 		30*time.Second,
-		settings.NonNegativeDuration,
 	)
 )
 

pkg/crosscluster/physical/stream_ingestion_processor.go

@@ -88,7 +88,6 @@ var cutoverSignalPollInterval = settings.RegisterDurationSetting(
 	"bulkio.stream_ingestion.failover_signal_poll_interval",
 	"the interval at which the stream ingestion job checks if it has been signaled to cutover",
 	10*time.Second,
-	settings.NonNegativeDuration,
 	settings.WithName("physical_replication.consumer.failover_signal_poll_interval"),
 )