Skip to content

Commit b06ad91

Browse files
mason-sharpclaude
andcommitted
Address CodeRabbit review: validate negative max_connections on all paths
- Let negative values pass through resolvers and config fallbacks instead of silently treating them as unset, so Validate() rejects them consistently. - Add max_connections validation to RepsetDiffCmd.Validate() and SchemaDiffCmd.Validate() to fail fast before per-table fan-out. - Simplify jobs config assignments to unconditional intArg calls. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 1b0b97c commit b06ad91

5 files changed

Lines changed: 14 additions & 10 deletions

File tree

internal/api/http/handler.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,9 @@ func (s *APIServer) resolveMaxConnections(cfg *config.Config, requested int) int
299299
if requested > 0 {
300300
return requested
301301
}
302+
if requested < 0 {
303+
return requested // rejected by Validate()
304+
}
302305
if cfg != nil && cfg.TableDiff.MaxConnections > 0 {
303306
return cfg.TableDiff.MaxConnections
304307
}

internal/consistency/diff/repset_diff.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,9 @@ func (c *RepsetDiffCmd) Validate() error {
231231
if c.RepsetName == "" {
232232
return fmt.Errorf("repset name is required")
233233
}
234+
if c.MaxConnections < 0 {
235+
return fmt.Errorf("max_connections must be >= 1 (or 0 to derive from concurrency factor)")
236+
}
234237
return nil
235238
}
236239

internal/consistency/diff/schema_diff.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,10 @@ func (c *SchemaDiffCmd) Validate() error {
178178
return fmt.Errorf("schema-diff needs at least two nodes to compare")
179179
}
180180

181+
if c.MaxConnections < 0 {
182+
return fmt.Errorf("max_connections must be >= 1 (or 0 to derive from concurrency factor)")
183+
}
184+
181185
return nil
182186
}
183187

internal/consistency/diff/table_diff.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -729,7 +729,7 @@ func (t *TableDiffTask) Validate() error {
729729
}
730730

731731
cfg := config.Get()
732-
if t.MaxConnections == 0 && cfg.TableDiff.MaxConnections > 0 {
732+
if t.MaxConnections == 0 {
733733
t.MaxConnections = cfg.TableDiff.MaxConnections
734734
}
735735
if t.MaxConnections < 0 {

internal/jobs/config.go

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,7 @@ func buildTableDiffJob(cfg *config.Config, def config.JobDef, spec scheduleSpec)
109109
if v := intArg(def.Args, "max_diff_rows", 0); v > 0 {
110110
base.MaxDiffRows = int64(v)
111111
}
112-
if v := intArg(def.Args, "max_connections", 0); v > 0 {
113-
base.MaxConnections = v
114-
}
112+
base.MaxConnections = intArg(def.Args, "max_connections", 0)
115113
if out := stringArg(def.Args, "output"); out != "" {
116114
base.Output = out
117115
}
@@ -187,9 +185,7 @@ func buildSchemaDiffJob(cfg *config.Config, def config.JobDef, spec scheduleSpec
187185
if v := intArg(def.Args, "compare_unit_size", 0); v > 0 {
188186
base.CompareUnitSize = v
189187
}
190-
if v := intArg(def.Args, "max_connections", 0); v > 0 {
191-
base.MaxConnections = v
192-
}
188+
base.MaxConnections = intArg(def.Args, "max_connections", 0)
193189
if out := stringArg(def.Args, "output"); out != "" {
194190
base.Output = out
195191
}
@@ -264,9 +260,7 @@ func buildRepsetDiffJob(cfg *config.Config, def config.JobDef, spec scheduleSpec
264260
if out := stringArg(def.Args, "output"); out != "" {
265261
base.Output = out
266262
}
267-
if v := intArg(def.Args, "max_connections", 0); v > 0 {
268-
base.MaxConnections = v
269-
}
263+
base.MaxConnections = intArg(def.Args, "max_connections", 0)
270264
base.TableFilter = stringArg(def.Args, "table_filter")
271265
base.OverrideBlockSize = boolArg(def.Args, "override_block_size", base.OverrideBlockSize)
272266
base.SkipDBUpdate = boolArg(def.Args, "skip_db_update", base.SkipDBUpdate)

0 commit comments

Comments
 (0)