From be6d297aeffdc1391e9e8e70d9cb8cdde370c3a7 Mon Sep 17 00:00:00 2001 From: Ahmed Darwich Date: Thu, 10 Sep 2026 11:29:25 -0700 Subject: [PATCH 1/4] Add keyspace rollout concurrency settings to the CLI --- AGENTS.md | 18 ++ internal/cmd/keyspace/keyspace.go | 1 + internal/cmd/keyspace/settings.go | 6 +- internal/cmd/keyspace/settings_test.go | 18 ++ internal/cmd/keyspace/update_settings.go | 87 ++++---- internal/cmd/keyspace/update_settings_test.go | 188 ++++++++++++++++-- internal/planetscale/keyspaces.go | 4 + internal/planetscale/keyspaces_test.go | 61 ++++++ 8 files changed, 331 insertions(+), 52 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index cf5e825b..e556bd54 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -419,6 +419,24 @@ pscale database aggressive-cutover disable --org --format json Vitess only. See https://planetscale.com/docs/vitess/schema-changes/aggressive-cutover +## Vitess keyspace rollout concurrency + +Configure how many shard rollouts may run concurrently for a keyspace: + +```bash +pscale keyspace settings --org --format json +pscale keyspace update-settings --org --format json --max-rollout 8 +pscale keyspace update-settings --org --format json --reset-max-rollout +``` + +`--max-rollout` accepts 1–32. Resetting removes the configured value and uses +the default of 1. In JSON, `max_rollout` is the stored configured value and is +`null` when unset; it is not a computed effective concurrency value. The +service caps effective rollout concurrency at 32. Values above 32 may appear +when an administrator has stored an override, but customer updates remain +limited to 32. An administrator's force override can also supersede the +configured value for the next rollout. + ## Vitess deploy requests (inspect + throttler) Core lifecycle is already covered (`list/create/show/diff/review/deploy/apply/unblock/update/cancel/close/revert/skip-revert`). `update` (`edit` is an alias) sets auto-apply and auto-delete-branch. `unblock` clears the queue after a failed deploy or revert (dashboard “Unblock deploy queue”); it is not `apply`. These inspect commands are read-only: diff --git a/internal/cmd/keyspace/keyspace.go b/internal/cmd/keyspace/keyspace.go index fb11e622..8cfa4dca 100644 --- a/internal/cmd/keyspace/keyspace.go +++ b/internal/cmd/keyspace/keyspace.go @@ -55,6 +55,7 @@ type Keyspace struct { type KeyspaceSettings struct { ReplicationDurabilityConstraintStrategy string `header:"replication durability constraint strategy" json:"replication_durability_constraint"` VReplicationFlags VReplicationFlags `header:"inline" json:"vreplication_flags"` + MaxRollout int `header:"max rollout" json:"max_rollout"` orig *ps.Keyspace } diff --git a/internal/cmd/keyspace/settings.go b/internal/cmd/keyspace/settings.go index 11a9b4d8..7c122d0e 100644 --- a/internal/cmd/keyspace/settings.go +++ b/internal/cmd/keyspace/settings.go @@ -53,7 +53,11 @@ func SettingsCmd(ch *cmdutil.Helper) *cobra.Command { // toKeyspaceSettings converts a Keyspace API response to a KeyspaceSettings object for display func toKeyspaceSettings(ks *ps.Keyspace) *KeyspaceSettings { settings := &KeyspaceSettings{ - orig: ks, + MaxRollout: 1, + orig: ks, + } + if ks.MaxRollout != nil { + settings.MaxRollout = *ks.MaxRollout } // Set replication durability constraints if available diff --git a/internal/cmd/keyspace/settings_test.go b/internal/cmd/keyspace/settings_test.go index f71ccb55..cbfa3320 100644 --- a/internal/cmd/keyspace/settings_test.go +++ b/internal/cmd/keyspace/settings_test.go @@ -3,6 +3,7 @@ package keyspace import ( "bytes" "context" + "encoding/json" "errors" "testing" "time" @@ -183,6 +184,7 @@ func TestBuildKeyspaceSettings(t *testing.T) { c := qt.New(t) ts := time.Now() + maxRollout := 64 // Test with all settings populated fullKs := &ps.Keyspace{ @@ -198,6 +200,7 @@ func TestBuildKeyspaceSettings(t *testing.T) { AllowNoBlobBinlogRowImage: true, VPlayerBatching: false, }, + MaxRollout: &maxRollout, } settings := toKeyspaceSettings(fullKs) @@ -205,6 +208,8 @@ func TestBuildKeyspaceSettings(t *testing.T) { c.Assert(settings.VReplicationFlags.OptimizeInserts, qt.Equals, true) c.Assert(settings.VReplicationFlags.AllowNoBlobBinlogRowImage, qt.Equals, true) c.Assert(settings.VReplicationFlags.VPlayerBatching, qt.Equals, false) + c.Assert(settings.MaxRollout, qt.Equals, 64) + assertMaxRolloutJSON(t, settings, "64") // Test with nil settings nilKs := &ps.Keyspace{ @@ -221,4 +226,17 @@ func TestBuildKeyspaceSettings(t *testing.T) { c.Assert(nilSettings.VReplicationFlags.OptimizeInserts, qt.Equals, false) // Default values c.Assert(nilSettings.VReplicationFlags.AllowNoBlobBinlogRowImage, qt.Equals, false) c.Assert(nilSettings.VReplicationFlags.VPlayerBatching, qt.Equals, false) + c.Assert(nilSettings.MaxRollout, qt.Equals, 1) + assertMaxRolloutJSON(t, nilSettings, "null") +} + +func assertMaxRolloutJSON(t *testing.T, settings *KeyspaceSettings, want string) { + t.Helper() + c := qt.New(t) + encoded, err := json.Marshal(settings) + c.Assert(err, qt.IsNil) + + var object map[string]json.RawMessage + c.Assert(json.Unmarshal(encoded, &object), qt.IsNil) + c.Assert(string(object["max_rollout"]), qt.Equals, want) } diff --git a/internal/cmd/keyspace/update_settings.go b/internal/cmd/keyspace/update_settings.go index e92bef54..c89ce040 100644 --- a/internal/cmd/keyspace/update_settings.go +++ b/internal/cmd/keyspace/update_settings.go @@ -12,11 +12,11 @@ import ( ) func UpdateSettingsCmd(ch *cmdutil.Helper) *cobra.Command { - updateReq := &ps.UpdateKeyspaceSettingsRequest{} - var flags struct { replicationDurabilityConstraints *ps.ReplicationDurabilityConstraints vreplicationFlags *ps.VReplicationFlags + maxRollout int + resetMaxRollout bool interactive bool } @@ -30,16 +30,43 @@ func UpdateSettingsCmd(ch *cmdutil.Helper) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() database, branch, keyspace := args[0], args[1], args[2] + maxRolloutChanged := cmd.Flags().Changed("max-rollout") + resetMaxRolloutChanged := cmd.Flags().Changed("reset-max-rollout") + resetMaxRolloutRequested := resetMaxRolloutChanged && flags.resetMaxRollout + + if maxRolloutChanged && resetMaxRolloutRequested { + return fmt.Errorf("--max-rollout and --reset-max-rollout are mutually exclusive") + } + if flags.interactive && (maxRolloutChanged || resetMaxRolloutChanged) { + return fmt.Errorf("--max-rollout and --reset-max-rollout cannot be used with --interactive") + } + if maxRolloutChanged && (flags.maxRollout < 1 || flags.maxRollout > 32) { + return fmt.Errorf("--max-rollout must be between 1 and 32") + } - updateReq.Organization = ch.Config.Organization - updateReq.Database = database - updateReq.Branch = branch - updateReq.Keyspace = keyspace + updateReq := &ps.UpdateKeyspaceSettingsRequest{ + Organization: ch.Config.Organization, + Database: database, + Branch: branch, + Keyspace: keyspace, + } if flags.interactive { return updateInteractive(ctx, ch, updateReq) } + // Only nested VReplication updates need a read before the PATCH so + // unspecified flags in that group can be preserved. + rdcChanged := cmd.Flags().Changed("replication-durability-constraints-strategy") + vrfChanged := cmd.Flags().Changed("vreplication-optimize-inserts") || + cmd.Flags().Changed("vreplication-enable-noblob-binlog-mode") || + cmd.Flags().Changed("vreplication-batch-replication-events") + + if !rdcChanged && !vrfChanged && !maxRolloutChanged && !resetMaxRolloutRequested { + ch.Printer.Println("No changes were requested. No update performed.") + return nil + } + client, err := ch.Client() if err != nil { return err @@ -48,25 +75,16 @@ func UpdateSettingsCmd(ch *cmdutil.Helper) *cobra.Command { end := ch.Printer.PrintProgress(fmt.Sprintf("Updating settings for keyspace %s in %s/%s", printer.BoldBlue(keyspace), printer.BoldBlue(database), printer.BoldBlue(branch))) defer end() - if err := setInitialSettings(ctx, ch, updateReq); err != nil { - return err - } - - // Check if any relevant flags are changing replication durability constraints - rdcChanged := cmd.Flags().Changed("replication-durability-constraints-strategy") if rdcChanged { - if updateReq.ReplicationDurabilityConstraints == nil { - updateReq.ReplicationDurabilityConstraints = &ps.ReplicationDurabilityConstraints{} + updateReq.ReplicationDurabilityConstraints = &ps.ReplicationDurabilityConstraints{ + Strategy: constraintsToStrategy(flags.replicationDurabilityConstraints.Strategy), } - updateReq.ReplicationDurabilityConstraints.Strategy = constraintsToStrategy(flags.replicationDurabilityConstraints.Strategy) } - // Check if any relevant flags are changing VReplication flags - vrfChanged := cmd.Flags().Changed("vreplication-optimize-inserts") || - cmd.Flags().Changed("vreplication-enable-noblob-binlog-mode") || - cmd.Flags().Changed("vreplication-batch-replication-events") - if vrfChanged { + if err := setInitialSettings(ctx, client, updateReq, false, true); err != nil { + return err + } if updateReq.VReplicationFlags == nil { updateReq.VReplicationFlags = &ps.VReplicationFlags{} } @@ -84,10 +102,12 @@ func UpdateSettingsCmd(ch *cmdutil.Helper) *cobra.Command { } } - if !rdcChanged && !vrfChanged { - end() - ch.Printer.Println("No changes were requested. No update performed.") - return nil + if maxRolloutChanged { + maxRollout := &flags.maxRollout + updateReq.MaxRollout = &maxRollout + } else if resetMaxRolloutRequested { + var maxRollout *int + updateReq.MaxRollout = &maxRollout } k, err := updateKeyspaceSettings(ctx, client, updateReq) @@ -105,17 +125,14 @@ func UpdateSettingsCmd(ch *cmdutil.Helper) *cobra.Command { cmd.Flags().BoolVar(&flags.vreplicationFlags.OptimizeInserts, "vreplication-optimize-inserts", true, "When enabled, skips sending INSERT events for rows that have yet to be replicated.") cmd.Flags().BoolVar(&flags.vreplicationFlags.AllowNoBlobBinlogRowImage, "vreplication-enable-noblob-binlog-mode", true, "When enabled, omits changed BLOB and TEXT columns from replication events, which reduces binlog sizes.") cmd.Flags().BoolVar(&flags.vreplicationFlags.VPlayerBatching, "vreplication-batch-replication-events", false, "When enabled, sends fewer queries to MySQL to improve performance.") + cmd.Flags().IntVar(&flags.maxRollout, "max-rollout", 0, "Maximum number of concurrent shard rollouts (1-32). The effective service cap is 32.") + cmd.Flags().BoolVar(&flags.resetMaxRollout, "reset-max-rollout", false, "Reset the configured maximum concurrent shard rollouts to the default (1).") cmd.Flags().BoolVarP(&flags.interactive, "interactive", "i", false, "Run the command in interactive mode") return cmd } -func setInitialSettings(ctx context.Context, ch *cmdutil.Helper, req *ps.UpdateKeyspaceSettingsRequest) error { - client, err := ch.Client() - if err != nil { - return err - } - +func setInitialSettings(ctx context.Context, client *ps.Client, req *ps.UpdateKeyspaceSettingsRequest, includeDurability, includeVReplication bool) error { organization := req.Organization database := req.Database branch := req.Branch @@ -136,13 +153,13 @@ func setInitialSettings(ctx context.Context, ch *cmdutil.Helper, req *ps.UpdateK } } - // Get initial defaults from the API - if ks.ReplicationDurabilityConstraints != nil { + if includeDurability && ks.ReplicationDurabilityConstraints != nil { req.ReplicationDurabilityConstraints = ks.ReplicationDurabilityConstraints } - if ks.VReplicationFlags != nil { - req.VReplicationFlags = ks.VReplicationFlags + if includeVReplication && ks.VReplicationFlags != nil { + vreplicationFlags := *ks.VReplicationFlags + req.VReplicationFlags = &vreplicationFlags } return nil @@ -154,7 +171,7 @@ func updateInteractive(ctx context.Context, ch *cmdutil.Helper, updateReq *ps.Up return err } - if err := setInitialSettings(ctx, ch, updateReq); err != nil { + if err := setInitialSettings(ctx, client, updateReq, true, true); err != nil { return err } diff --git a/internal/cmd/keyspace/update_settings_test.go b/internal/cmd/keyspace/update_settings_test.go index fe2cccbf..951d0341 100644 --- a/internal/cmd/keyspace/update_settings_test.go +++ b/internal/cmd/keyspace/update_settings_test.go @@ -79,7 +79,7 @@ func TestKeyspace_UpdateSettingsCmd_OnlyVReplicationFlags(t *testing.T) { c.Assert(req.Organization, qt.Equals, org) c.Assert(req.Branch, qt.Equals, branch) c.Assert(req.Keyspace, qt.Equals, keyspace) - c.Assert(req.ReplicationDurabilityConstraints.Strategy, qt.Equals, rdcStrategy) + c.Assert(req.ReplicationDurabilityConstraints, qt.IsNil) c.Assert(req.VReplicationFlags.OptimizeInserts, qt.Equals, false) c.Assert(req.VReplicationFlags.AllowNoBlobBinlogRowImage, qt.Equals, false) c.Assert(req.VReplicationFlags.VPlayerBatching, qt.Equals, true) @@ -289,9 +289,7 @@ func TestKeyspace_UpdateSettingsCmd_OnlyDurabilityConstraints(t *testing.T) { c.Assert(req.Branch, qt.Equals, branch) c.Assert(req.Keyspace, qt.Equals, keyspace) c.Assert(req.ReplicationDurabilityConstraints.Strategy, qt.Equals, updatedRdcStrategy) - c.Assert(req.VReplicationFlags.OptimizeInserts, qt.Equals, true) - c.Assert(req.VReplicationFlags.AllowNoBlobBinlogRowImage, qt.Equals, true) - c.Assert(req.VReplicationFlags.VPlayerBatching, qt.Equals, false) + c.Assert(req.VReplicationFlags, qt.IsNil) return updatedKs, nil }, @@ -318,7 +316,7 @@ func TestKeyspace_UpdateSettingsCmd_OnlyDurabilityConstraints(t *testing.T) { }) err := cmd.Execute() c.Assert(err, qt.IsNil) - c.Assert(svc.GetFnInvoked, qt.IsTrue) + c.Assert(svc.GetFnInvoked, qt.IsFalse) c.Assert(svc.UpdateSettingsFnInvoked, qt.IsTrue) c.Assert(buf.String(), qt.JSONEquals, updatedKs) } @@ -384,9 +382,7 @@ func TestKeyspace_UpdateSettingsCmd_NilVReplicationFlags(t *testing.T) { c.Assert(req.Branch, qt.Equals, branch) c.Assert(req.Keyspace, qt.Equals, keyspace) - // Check that ReplicationDurabilityConstraints is unchanged and not nil - c.Assert(req.ReplicationDurabilityConstraints, qt.Not(qt.IsNil)) - c.Assert(req.ReplicationDurabilityConstraints.Strategy, qt.Equals, rdcStrategy) + c.Assert(req.ReplicationDurabilityConstraints, qt.IsNil) // Check that VReplication flags are initialized (since flags were provided) c.Assert(req.VReplicationFlags, qt.Not(qt.IsNil)) @@ -493,11 +489,7 @@ func TestKeyspace_UpdateSettingsCmd_NilReplicationDurabilityConstraints(t *testi c.Assert(req.ReplicationDurabilityConstraints, qt.Not(qt.IsNil)) c.Assert(req.ReplicationDurabilityConstraints.Strategy, qt.Equals, updatedRdcStrategy) - // VReplication flags should be maintained and not nil - c.Assert(req.VReplicationFlags, qt.Not(qt.IsNil)) - c.Assert(req.VReplicationFlags.OptimizeInserts, qt.Equals, true) - c.Assert(req.VReplicationFlags.AllowNoBlobBinlogRowImage, qt.Equals, true) - c.Assert(req.VReplicationFlags.VPlayerBatching, qt.Equals, false) + c.Assert(req.VReplicationFlags, qt.IsNil) return updatedKs, nil }, @@ -524,7 +516,7 @@ func TestKeyspace_UpdateSettingsCmd_NilReplicationDurabilityConstraints(t *testi }) err := cmd.Execute() c.Assert(err, qt.IsNil) - c.Assert(svc.GetFnInvoked, qt.IsTrue) + c.Assert(svc.GetFnInvoked, qt.IsFalse) c.Assert(svc.UpdateSettingsFnInvoked, qt.IsTrue) c.Assert(buf.String(), qt.JSONEquals, updatedKs) } @@ -614,7 +606,7 @@ func TestKeyspace_UpdateSettingsCmd_PreserveNilValues(t *testing.T) { }) err := cmd.Execute() c.Assert(err, qt.IsNil) - c.Assert(svc.GetFnInvoked, qt.IsTrue) + c.Assert(svc.GetFnInvoked, qt.IsFalse) c.Assert(svc.UpdateSettingsFnInvoked, qt.IsTrue) c.Assert(buf.String(), qt.JSONEquals, updatedKs) } @@ -629,6 +621,170 @@ func TestKeyspace_ConstraintsToStrategy(t *testing.T) { c.Assert(constraintsToStrategy("unknown"), qt.Equals, "unknown") } +func TestKeyspace_UpdateSettingsCmd_MaxRollout(t *testing.T) { + c := qt.New(t) + var buf bytes.Buffer + format := printer.JSON + maxRollout := 8 + + svc := &mock.KeyspacesService{ + UpdateSettingsFn: func(_ context.Context, req *ps.UpdateKeyspaceSettingsRequest) (*ps.Keyspace, error) { + c.Assert(req.ReplicationDurabilityConstraints, qt.IsNil) + c.Assert(req.VReplicationFlags, qt.IsNil) + c.Assert(req.MaxRollout, qt.Not(qt.IsNil)) + c.Assert(*req.MaxRollout, qt.Not(qt.IsNil)) + c.Assert(**req.MaxRollout, qt.Equals, maxRollout) + return &ps.Keyspace{MaxRollout: &maxRollout}, nil + }, + } + p := printer.NewPrinter(&format) + p.SetResourceOutput(&buf) + ch := &cmdutil.Helper{ + Printer: p, + Config: &config.Config{Organization: "planetscale"}, + Client: func() (*ps.Client, error) { + return &ps.Client{Keyspaces: svc}, nil + }, + } + + cmd := UpdateSettingsCmd(ch) + cmd.SetArgs([]string{"database", "main", "keyspace", "--max-rollout=8", "--reset-max-rollout=false"}) + c.Assert(cmd.Execute(), qt.IsNil) + c.Assert(svc.GetFnInvoked, qt.IsFalse) + c.Assert(svc.UpdateSettingsFnInvoked, qt.IsTrue) + c.Assert(buf.String(), qt.Contains, `"max_rollout": 8`) +} + +func TestKeyspace_UpdateSettingsCmd_ResetMaxRollout(t *testing.T) { + c := qt.New(t) + var buf bytes.Buffer + format := printer.JSON + + svc := &mock.KeyspacesService{ + UpdateSettingsFn: func(_ context.Context, req *ps.UpdateKeyspaceSettingsRequest) (*ps.Keyspace, error) { + c.Assert(req.ReplicationDurabilityConstraints, qt.IsNil) + c.Assert(req.VReplicationFlags, qt.IsNil) + c.Assert(req.MaxRollout, qt.Not(qt.IsNil)) + c.Assert(*req.MaxRollout, qt.IsNil) + return &ps.Keyspace{}, nil + }, + } + p := printer.NewPrinter(&format) + p.SetResourceOutput(&buf) + ch := &cmdutil.Helper{ + Printer: p, + Config: &config.Config{Organization: "planetscale"}, + Client: func() (*ps.Client, error) { + return &ps.Client{Keyspaces: svc}, nil + }, + } + + cmd := UpdateSettingsCmd(ch) + cmd.SetArgs([]string{"database", "main", "keyspace", "--reset-max-rollout"}) + c.Assert(cmd.Execute(), qt.IsNil) + c.Assert(svc.GetFnInvoked, qt.IsFalse) + c.Assert(svc.UpdateSettingsFnInvoked, qt.IsTrue) + c.Assert(buf.String(), qt.Contains, `"max_rollout": null`) +} + +func TestKeyspace_UpdateSettingsCmd_MaxRolloutValidationBeforeAPI(t *testing.T) { + for _, tt := range []struct { + name string + args []string + wantError string + }{ + {name: "too low", args: []string{"--max-rollout=0"}, wantError: `--max-rollout must be between 1 and 32`}, + {name: "too high", args: []string{"--max-rollout=33"}, wantError: `--max-rollout must be between 1 and 32`}, + {name: "set and reset", args: []string{"--max-rollout=8", "--reset-max-rollout"}, wantError: `--max-rollout and --reset-max-rollout are mutually exclusive`}, + {name: "set interactively", args: []string{"--interactive", "--max-rollout=8"}, wantError: `--max-rollout and --reset-max-rollout cannot be used with --interactive`}, + {name: "reset interactively", args: []string{"--interactive", "--reset-max-rollout"}, wantError: `--max-rollout and --reset-max-rollout cannot be used with --interactive`}, + {name: "explicit false reset interactively", args: []string{"--interactive", "--reset-max-rollout=false"}, wantError: `--max-rollout and --reset-max-rollout cannot be used with --interactive`}, + } { + t.Run(tt.name, func(t *testing.T) { + c := qt.New(t) + format := printer.Human + clientCalled := false + ch := &cmdutil.Helper{ + Printer: printer.NewPrinter(&format), + Config: &config.Config{Organization: "planetscale"}, + Client: func() (*ps.Client, error) { + clientCalled = true + return nil, errors.New("unexpected API client call") + }, + } + cmd := UpdateSettingsCmd(ch) + cmd.SetArgs(append([]string{"database", "main", "keyspace"}, tt.args...)) + c.Assert(cmd.Execute(), qt.ErrorMatches, tt.wantError) + c.Assert(clientCalled, qt.IsFalse) + }) + } +} + +func TestKeyspace_UpdateSettingsCmd_ResetMaxRolloutFalseIsNoOp(t *testing.T) { + c := qt.New(t) + format := printer.Human + clientCalled := false + ch := &cmdutil.Helper{ + Printer: printer.NewPrinter(&format), + Config: &config.Config{Organization: "planetscale"}, + Client: func() (*ps.Client, error) { + clientCalled = true + return nil, errors.New("unexpected API client call") + }, + } + + cmd := UpdateSettingsCmd(ch) + cmd.SetArgs([]string{"database", "main", "keyspace", "--reset-max-rollout=false"}) + c.Assert(cmd.Execute(), qt.IsNil) + c.Assert(clientCalled, qt.IsFalse) +} + +func TestKeyspace_UpdateSettingsCmd_PreservesUnspecifiedVReplicationFlags(t *testing.T) { + c := qt.New(t) + var buf bytes.Buffer + format := printer.JSON + initial := &ps.Keyspace{ + VReplicationFlags: &ps.VReplicationFlags{ + OptimizeInserts: true, + AllowNoBlobBinlogRowImage: true, + VPlayerBatching: false, + }, + } + updated := &ps.Keyspace{ + VReplicationFlags: &ps.VReplicationFlags{ + OptimizeInserts: false, + AllowNoBlobBinlogRowImage: true, + VPlayerBatching: false, + }, + } + svc := &mock.KeyspacesService{ + GetFn: func(_ context.Context, _ *ps.GetKeyspaceRequest) (*ps.Keyspace, error) { + return initial, nil + }, + UpdateSettingsFn: func(_ context.Context, req *ps.UpdateKeyspaceSettingsRequest) (*ps.Keyspace, error) { + c.Assert(req.ReplicationDurabilityConstraints, qt.IsNil) + c.Assert(req.MaxRollout, qt.IsNil) + c.Assert(req.VReplicationFlags, qt.DeepEquals, updated.VReplicationFlags) + return updated, nil + }, + } + p := printer.NewPrinter(&format) + p.SetResourceOutput(&buf) + ch := &cmdutil.Helper{ + Printer: p, + Config: &config.Config{Organization: "planetscale"}, + Client: func() (*ps.Client, error) { + return &ps.Client{Keyspaces: svc}, nil + }, + } + + cmd := UpdateSettingsCmd(ch) + cmd.SetArgs([]string{"database", "main", "keyspace", "--vreplication-optimize-inserts=false"}) + c.Assert(cmd.Execute(), qt.IsNil) + c.Assert(svc.GetFnInvoked, qt.IsTrue) + c.Assert(svc.UpdateSettingsFnInvoked, qt.IsTrue) +} + func TestKeyspace_UpdateSettingsCmd_ErrorNotFound(t *testing.T) { c := qt.New(t) @@ -664,7 +820,7 @@ func TestKeyspace_UpdateSettingsCmd_ErrorNotFound(t *testing.T) { } cmd := UpdateSettingsCmd(ch) - cmd.SetArgs([]string{db, branch, keyspace}) + cmd.SetArgs([]string{db, branch, keyspace, "--vreplication-optimize-inserts=false"}) err := cmd.Execute() c.Assert(err, qt.Not(qt.IsNil)) // Just check that there is an error c.Assert(svc.GetFnInvoked, qt.IsTrue) diff --git a/internal/planetscale/keyspaces.go b/internal/planetscale/keyspaces.go index 1b78de6b..41362bcb 100644 --- a/internal/planetscale/keyspaces.go +++ b/internal/planetscale/keyspaces.go @@ -24,6 +24,7 @@ type Keyspace struct { UpdatedAt time.Time `json:"updated_at"` VReplicationFlags *VReplicationFlags `json:"vreplication_flags"` ReplicationDurabilityConstraints *ReplicationDurabilityConstraints `json:"replication_durability_constraints"` + MaxRollout *int `json:"max_rollout"` ReadOnlyRegions []*ReadOnlyRegionKeyspace `json:"read_only_regions"` } @@ -174,6 +175,9 @@ type UpdateKeyspaceSettingsRequest struct { Keyspace string `json:"-"` ReplicationDurabilityConstraints *ReplicationDurabilityConstraints `json:"replication_durability_constraints,omitempty"` VReplicationFlags *VReplicationFlags `json:"vreplication_flags,omitempty"` + // MaxRollout is a tri-state PATCH field: nil omits max_rollout, a pointer + // to an integer sets it, and a pointer to nil sends JSON null to reset it. + MaxRollout **int `json:"max_rollout,omitempty"` } type ReplicationDurabilityConstraints struct { diff --git a/internal/planetscale/keyspaces_test.go b/internal/planetscale/keyspaces_test.go index 792ec397..4a8fdf82 100644 --- a/internal/planetscale/keyspaces_test.go +++ b/internal/planetscale/keyspaces_test.go @@ -3,6 +3,7 @@ package planetscale import ( "context" "encoding/json" + "io" "net/http" "net/http/httptest" "testing" @@ -479,3 +480,63 @@ func TestKeyspaces_UpdateSettings(t *testing.T) { c.Assert(keyspace.VReplicationFlags.VPlayerBatching, qt.Equals, true) c.Assert(keyspace.ReplicationDurabilityConstraints.Strategy, qt.Equals, "maximum") } + +func TestKeyspaces_UpdateSettingsMaxRolloutPayload(t *testing.T) { + for _, tt := range []struct { + name string + maxRollout func() **int + wantBody string + }{ + { + name: "omitted", + maxRollout: func() **int { + return nil + }, + wantBody: `{}`, + }, + { + name: "integer", + maxRollout: func() **int { + value := 8 + valuePointer := &value + return &valuePointer + }, + wantBody: `{"max_rollout":8}`, + }, + { + name: "null", + maxRollout: func() **int { + var value *int + return &value + }, + wantBody: `{"max_rollout":null}`, + }, + } { + t.Run(tt.name, func(t *testing.T) { + c := qt.New(t) + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, err := io.ReadAll(r.Body) + c.Assert(err, qt.IsNil) + c.Assert(r.Method, qt.Equals, http.MethodPatch) + c.Assert(string(body), qt.JSONEquals, json.RawMessage(tt.wantBody)) + _, err = w.Write([]byte(`{"max_rollout":64}`)) + c.Assert(err, qt.IsNil) + })) + defer ts.Close() + + client, err := NewClient(WithBaseURL(ts.URL)) + c.Assert(err, qt.IsNil) + + keyspace, err := client.Keyspaces.UpdateSettings(context.Background(), &UpdateKeyspaceSettingsRequest{ + Organization: "foo", + Database: "bar", + Branch: "baz", + Keyspace: "qux", + MaxRollout: tt.maxRollout(), + }) + c.Assert(err, qt.IsNil) + c.Assert(keyspace.MaxRollout, qt.Not(qt.IsNil)) + c.Assert(*keyspace.MaxRollout, qt.Equals, 64) + }) + } +} From bf04f28dcdf6391c6b0314315d8de967a20afd68 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 15 Sep 2026 15:58:04 +0000 Subject: [PATCH 2/4] Fetch nested keyspace settings once when combining flag groups. Validate throttler threshold before any API call, assert max-rollout updates leave throttler unset, and drop internal admin-override wording from public AGENTS.md. --- AGENTS.md | 5 +- internal/cmd/keyspace/update_settings.go | 16 ++--- internal/cmd/keyspace/update_settings_test.go | 65 +++++++++++++++++++ 3 files changed, 74 insertions(+), 12 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e556bd54..eb225155 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -432,10 +432,7 @@ pscale keyspace update-settings --org --for `--max-rollout` accepts 1–32. Resetting removes the configured value and uses the default of 1. In JSON, `max_rollout` is the stored configured value and is `null` when unset; it is not a computed effective concurrency value. The -service caps effective rollout concurrency at 32. Values above 32 may appear -when an administrator has stored an override, but customer updates remain -limited to 32. An administrator's force override can also supersede the -configured value for the next rollout. +service caps effective rollout concurrency at 32. ## Vitess deploy requests (inspect + throttler) diff --git a/internal/cmd/keyspace/update_settings.go b/internal/cmd/keyspace/update_settings.go index e2e6c572..4e10c5bd 100644 --- a/internal/cmd/keyspace/update_settings.go +++ b/internal/cmd/keyspace/update_settings.go @@ -47,6 +47,9 @@ func UpdateSettingsCmd(ch *cmdutil.Helper) *cobra.Command { if maxRolloutChanged && (flags.maxRollout < 1 || flags.maxRollout > 32) { return fmt.Errorf("--max-rollout must be between 1 and 32") } + if cmd.Flags().Changed("throttler-threshold") && flags.throttlerThreshold < 0 { + return errors.New("--throttler-threshold must be greater than or equal to 0") + } updateReq := &ps.UpdateKeyspaceSettingsRequest{ Organization: ch.Config.Organization, @@ -87,10 +90,13 @@ func UpdateSettingsCmd(ch *cmdutil.Helper) *cobra.Command { } } - if vrfChanged { - if err := setInitialSettings(ctx, client, updateReq, false, true, false); err != nil { + if vrfChanged || throttlerChanged { + if err := setInitialSettings(ctx, client, updateReq, false, vrfChanged, throttlerChanged); err != nil { return err } + } + + if vrfChanged { if updateReq.VReplicationFlags == nil { updateReq.VReplicationFlags = &ps.VReplicationFlags{} } @@ -109,9 +115,6 @@ func UpdateSettingsCmd(ch *cmdutil.Helper) *cobra.Command { } if throttlerChanged { - if err := setInitialSettings(ctx, client, updateReq, false, false, true); err != nil { - return err - } if updateReq.Throttler == nil { updateReq.Throttler = &ps.KeyspaceThrottler{} } @@ -121,9 +124,6 @@ func UpdateSettingsCmd(ch *cmdutil.Helper) *cobra.Command { } if cmd.Flags().Changed("throttler-threshold") { - if flags.throttlerThreshold < 0 { - return errors.New("--throttler-threshold must be greater than or equal to 0") - } updateReq.Throttler.Threshold = &flags.throttlerThreshold } } diff --git a/internal/cmd/keyspace/update_settings_test.go b/internal/cmd/keyspace/update_settings_test.go index 2296cefa..c8b2644a 100644 --- a/internal/cmd/keyspace/update_settings_test.go +++ b/internal/cmd/keyspace/update_settings_test.go @@ -835,6 +835,7 @@ func TestKeyspace_UpdateSettingsCmd_RejectsNegativeThrottlerThreshold(t *testing cmd.SetArgs([]string{db, branch, keyspace, "--throttler-threshold=-1"}) err := cmd.Execute() c.Assert(err, qt.ErrorMatches, ".*throttler-threshold must be greater than or equal to 0") + c.Assert(svc.GetFnInvoked, qt.IsFalse) c.Assert(svc.UpdateSettingsFnInvoked, qt.IsFalse) } @@ -858,6 +859,7 @@ func TestKeyspace_UpdateSettingsCmd_MaxRollout(t *testing.T) { UpdateSettingsFn: func(_ context.Context, req *ps.UpdateKeyspaceSettingsRequest) (*ps.Keyspace, error) { c.Assert(req.ReplicationDurabilityConstraints, qt.IsNil) c.Assert(req.VReplicationFlags, qt.IsNil) + c.Assert(req.Throttler, qt.IsNil) c.Assert(req.MaxRollout, qt.Not(qt.IsNil)) c.Assert(*req.MaxRollout, qt.Not(qt.IsNil)) c.Assert(**req.MaxRollout, qt.Equals, maxRollout) @@ -882,6 +884,68 @@ func TestKeyspace_UpdateSettingsCmd_MaxRollout(t *testing.T) { c.Assert(buf.String(), qt.Contains, `"max_rollout": 8`) } +func TestKeyspace_UpdateSettingsCmd_MaxRolloutWithNestedSettings(t *testing.T) { + c := qt.New(t) + format := printer.JSON + maxRollout := 8 + throttlerEnabled := true + throttlerThreshold := 5.0 + getCalls := 0 + initial := &ps.Keyspace{ + VReplicationFlags: &ps.VReplicationFlags{ + OptimizeInserts: true, + AllowNoBlobBinlogRowImage: true, + VPlayerBatching: false, + }, + Throttler: &ps.KeyspaceThrottler{ + Enabled: &throttlerEnabled, + Threshold: &throttlerThreshold, + }, + } + + svc := &mock.KeyspacesService{ + GetFn: func(_ context.Context, _ *ps.GetKeyspaceRequest) (*ps.Keyspace, error) { + getCalls++ + return initial, nil + }, + UpdateSettingsFn: func(_ context.Context, req *ps.UpdateKeyspaceSettingsRequest) (*ps.Keyspace, error) { + c.Assert(req.ReplicationDurabilityConstraints, qt.IsNil) + c.Assert(req.VReplicationFlags.OptimizeInserts, qt.IsFalse) + c.Assert(req.VReplicationFlags.AllowNoBlobBinlogRowImage, qt.IsTrue) + c.Assert(req.VReplicationFlags.VPlayerBatching, qt.IsFalse) + c.Assert(req.Throttler.Enabled, qt.Not(qt.IsNil)) + c.Assert(*req.Throttler.Enabled, qt.IsTrue) + c.Assert(req.Throttler.Threshold, qt.Not(qt.IsNil)) + c.Assert(*req.Throttler.Threshold, qt.Equals, 10.0) + c.Assert(req.MaxRollout, qt.Not(qt.IsNil)) + c.Assert(*req.MaxRollout, qt.Not(qt.IsNil)) + c.Assert(**req.MaxRollout, qt.Equals, maxRollout) + return &ps.Keyspace{MaxRollout: &maxRollout}, nil + }, + } + ch := &cmdutil.Helper{ + Printer: printer.NewPrinter(&format), + Config: &config.Config{Organization: "planetscale"}, + Client: func() (*ps.Client, error) { + return &ps.Client{Keyspaces: svc}, nil + }, + } + + cmd := UpdateSettingsCmd(ch) + cmd.SetArgs([]string{ + "database", + "main", + "keyspace", + "--max-rollout=8", + "--vreplication-optimize-inserts=false", + "--throttler-threshold=10", + }) + + c.Assert(cmd.Execute(), qt.IsNil) + c.Assert(getCalls, qt.Equals, 1) + c.Assert(svc.UpdateSettingsFnInvoked, qt.IsTrue) +} + func TestKeyspace_UpdateSettingsCmd_ResetMaxRollout(t *testing.T) { c := qt.New(t) var buf bytes.Buffer @@ -891,6 +955,7 @@ func TestKeyspace_UpdateSettingsCmd_ResetMaxRollout(t *testing.T) { UpdateSettingsFn: func(_ context.Context, req *ps.UpdateKeyspaceSettingsRequest) (*ps.Keyspace, error) { c.Assert(req.ReplicationDurabilityConstraints, qt.IsNil) c.Assert(req.VReplicationFlags, qt.IsNil) + c.Assert(req.Throttler, qt.IsNil) c.Assert(req.MaxRollout, qt.Not(qt.IsNil)) c.Assert(*req.MaxRollout, qt.IsNil) return &ps.Keyspace{}, nil From a75b666edb42cf9b10768c15afdaff0d8d843766 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 15 Sep 2026 17:52:37 +0000 Subject: [PATCH 3/4] Show --max-rollout default as 1 in help. Use the same Cobra default as the other update-settings flags so help prints (default 1). Omitting the flag still does not PATCH; Changed() gates the request. --- internal/cmd/keyspace/update_settings.go | 2 +- internal/cmd/keyspace/update_settings_test.go | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/internal/cmd/keyspace/update_settings.go b/internal/cmd/keyspace/update_settings.go index 4e10c5bd..e2f154c5 100644 --- a/internal/cmd/keyspace/update_settings.go +++ b/internal/cmd/keyspace/update_settings.go @@ -151,7 +151,7 @@ func UpdateSettingsCmd(ch *cmdutil.Helper) *cobra.Command { cmd.Flags().BoolVar(&flags.vreplicationFlags.OptimizeInserts, "vreplication-optimize-inserts", true, "When enabled, skips sending INSERT events for rows that have yet to be replicated.") cmd.Flags().BoolVar(&flags.vreplicationFlags.AllowNoBlobBinlogRowImage, "vreplication-enable-noblob-binlog-mode", true, "When enabled, omits changed BLOB and TEXT columns from replication events, which reduces binlog sizes.") cmd.Flags().BoolVar(&flags.vreplicationFlags.VPlayerBatching, "vreplication-batch-replication-events", false, "When enabled, sends fewer queries to MySQL to improve performance.") - cmd.Flags().IntVar(&flags.maxRollout, "max-rollout", 0, "Maximum number of concurrent shard rollouts (1-32). The effective service cap is 32.") + cmd.Flags().IntVar(&flags.maxRollout, "max-rollout", 1, "Maximum number of concurrent shard rollouts (1-32). The effective service cap is 32.") cmd.Flags().BoolVar(&flags.resetMaxRollout, "reset-max-rollout", false, "Reset the configured maximum concurrent shard rollouts to the default (1).") cmd.Flags().BoolVar(&flags.throttlerEnabled, "throttler-enabled", true, "Pause schema migrations and VReplication workflows when replication lag rises above the threshold.") cmd.Flags().Float64Var(&flags.throttlerThreshold, "throttler-threshold", 5, "Replication lag in seconds above which migrations and workflows are paused.") diff --git a/internal/cmd/keyspace/update_settings_test.go b/internal/cmd/keyspace/update_settings_test.go index c8b2644a..032bcf2f 100644 --- a/internal/cmd/keyspace/update_settings_test.go +++ b/internal/cmd/keyspace/update_settings_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "errors" + "strings" "testing" "time" @@ -1012,6 +1013,26 @@ func TestKeyspace_UpdateSettingsCmd_MaxRolloutValidationBeforeAPI(t *testing.T) } } +func TestKeyspace_UpdateSettingsCmd_MaxRolloutHelpDefault(t *testing.T) { + c := qt.New(t) + format := printer.Human + ch := &cmdutil.Helper{ + Printer: printer.NewPrinter(&format), + Config: &config.Config{Organization: "planetscale"}, + } + + cmd := UpdateSettingsCmd(ch) + var maxRolloutUsage string + for _, line := range strings.Split(cmd.Flags().FlagUsages(), "\n") { + if strings.Contains(line, "--max-rollout") { + maxRolloutUsage = line + break + } + } + c.Assert(maxRolloutUsage, qt.Not(qt.Equals), "") + c.Assert(maxRolloutUsage, qt.Contains, "(default 1)") +} + func TestKeyspace_UpdateSettingsCmd_ResetMaxRolloutFalseIsNoOp(t *testing.T) { c := qt.New(t) format := printer.Human From 98a4b76340e850163e12104256d5b48cc86884f1 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 16 Sep 2026 15:09:08 +0000 Subject: [PATCH 4/4] Slim --max-rollout to the existing update-settings pattern. Keep a pointer field, omitempty, Flags().Changed(), and a 1-32 check. Show "not set" when the API returns null. Drop GET-skip, JSON null reset, and the extra AGENTS.md section. --- AGENTS.md | 15 - internal/cmd/keyspace/keyspace.go | 2 +- internal/cmd/keyspace/settings.go | 6 +- internal/cmd/keyspace/settings_test.go | 23 +- internal/cmd/keyspace/update_settings.go | 113 +++--- internal/cmd/keyspace/update_settings_test.go | 323 ++++++------------ internal/planetscale/keyspaces.go | 4 +- internal/planetscale/keyspaces_test.go | 95 ++---- 8 files changed, 212 insertions(+), 369 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index eb225155..cf5e825b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -419,21 +419,6 @@ pscale database aggressive-cutover disable --org --format json Vitess only. See https://planetscale.com/docs/vitess/schema-changes/aggressive-cutover -## Vitess keyspace rollout concurrency - -Configure how many shard rollouts may run concurrently for a keyspace: - -```bash -pscale keyspace settings --org --format json -pscale keyspace update-settings --org --format json --max-rollout 8 -pscale keyspace update-settings --org --format json --reset-max-rollout -``` - -`--max-rollout` accepts 1–32. Resetting removes the configured value and uses -the default of 1. In JSON, `max_rollout` is the stored configured value and is -`null` when unset; it is not a computed effective concurrency value. The -service caps effective rollout concurrency at 32. - ## Vitess deploy requests (inspect + throttler) Core lifecycle is already covered (`list/create/show/diff/review/deploy/apply/unblock/update/cancel/close/revert/skip-revert`). `update` (`edit` is an alias) sets auto-apply and auto-delete-branch. `unblock` clears the queue after a failed deploy or revert (dashboard “Unblock deploy queue”); it is not `apply`. These inspect commands are read-only: diff --git a/internal/cmd/keyspace/keyspace.go b/internal/cmd/keyspace/keyspace.go index 4a036227..7a59d599 100644 --- a/internal/cmd/keyspace/keyspace.go +++ b/internal/cmd/keyspace/keyspace.go @@ -55,7 +55,7 @@ type Keyspace struct { type KeyspaceSettings struct { ReplicationDurabilityConstraintStrategy string `header:"replication durability constraint strategy" json:"replication_durability_constraint"` VReplicationFlags VReplicationFlags `header:"inline" json:"vreplication_flags"` - MaxRollout int `header:"max rollout" json:"max_rollout"` + MaxRollout string `header:"max rollout" json:"max_rollout"` Throttler Throttler `header:"inline" json:"throttler"` orig *ps.Keyspace diff --git a/internal/cmd/keyspace/settings.go b/internal/cmd/keyspace/settings.go index fdd2dc1d..233fe6d6 100644 --- a/internal/cmd/keyspace/settings.go +++ b/internal/cmd/keyspace/settings.go @@ -2,6 +2,7 @@ package keyspace import ( "fmt" + "strconv" "github.com/planetscale/cli/internal/cmdutil" ps "github.com/planetscale/cli/internal/planetscale" @@ -53,11 +54,12 @@ func SettingsCmd(ch *cmdutil.Helper) *cobra.Command { // toKeyspaceSettings converts a Keyspace API response to a KeyspaceSettings object for display func toKeyspaceSettings(ks *ps.Keyspace) *KeyspaceSettings { settings := &KeyspaceSettings{ - MaxRollout: 1, + MaxRollout: "not set", orig: ks, } + if ks.MaxRollout != nil { - settings.MaxRollout = *ks.MaxRollout + settings.MaxRollout = strconv.Itoa(*ks.MaxRollout) } // Set replication durability constraints if available diff --git a/internal/cmd/keyspace/settings_test.go b/internal/cmd/keyspace/settings_test.go index cbfa3320..07cb5f44 100644 --- a/internal/cmd/keyspace/settings_test.go +++ b/internal/cmd/keyspace/settings_test.go @@ -3,7 +3,6 @@ package keyspace import ( "bytes" "context" - "encoding/json" "errors" "testing" "time" @@ -184,7 +183,6 @@ func TestBuildKeyspaceSettings(t *testing.T) { c := qt.New(t) ts := time.Now() - maxRollout := 64 // Test with all settings populated fullKs := &ps.Keyspace{ @@ -200,16 +198,17 @@ func TestBuildKeyspaceSettings(t *testing.T) { AllowNoBlobBinlogRowImage: true, VPlayerBatching: false, }, - MaxRollout: &maxRollout, } + maxRollout := 8 + fullKs.MaxRollout = &maxRollout + settings := toKeyspaceSettings(fullKs) c.Assert(settings.ReplicationDurabilityConstraintStrategy, qt.Equals, "maximum") // Should be translated + c.Assert(settings.MaxRollout, qt.Equals, "8") c.Assert(settings.VReplicationFlags.OptimizeInserts, qt.Equals, true) c.Assert(settings.VReplicationFlags.AllowNoBlobBinlogRowImage, qt.Equals, true) c.Assert(settings.VReplicationFlags.VPlayerBatching, qt.Equals, false) - c.Assert(settings.MaxRollout, qt.Equals, 64) - assertMaxRolloutJSON(t, settings, "64") // Test with nil settings nilKs := &ps.Keyspace{ @@ -223,20 +222,8 @@ func TestBuildKeyspaceSettings(t *testing.T) { nilSettings := toKeyspaceSettings(nilKs) c.Assert(nilSettings.ReplicationDurabilityConstraintStrategy, qt.Equals, "not set") + c.Assert(nilSettings.MaxRollout, qt.Equals, "not set") c.Assert(nilSettings.VReplicationFlags.OptimizeInserts, qt.Equals, false) // Default values c.Assert(nilSettings.VReplicationFlags.AllowNoBlobBinlogRowImage, qt.Equals, false) c.Assert(nilSettings.VReplicationFlags.VPlayerBatching, qt.Equals, false) - c.Assert(nilSettings.MaxRollout, qt.Equals, 1) - assertMaxRolloutJSON(t, nilSettings, "null") -} - -func assertMaxRolloutJSON(t *testing.T, settings *KeyspaceSettings, want string) { - t.Helper() - c := qt.New(t) - encoded, err := json.Marshal(settings) - c.Assert(err, qt.IsNil) - - var object map[string]json.RawMessage - c.Assert(json.Unmarshal(encoded, &object), qt.IsNil) - c.Assert(string(object["max_rollout"]), qt.Equals, want) } diff --git a/internal/cmd/keyspace/update_settings.go b/internal/cmd/keyspace/update_settings.go index e2f154c5..69156f65 100644 --- a/internal/cmd/keyspace/update_settings.go +++ b/internal/cmd/keyspace/update_settings.go @@ -14,13 +14,14 @@ import ( ) func UpdateSettingsCmd(ch *cmdutil.Helper) *cobra.Command { + updateReq := &ps.UpdateKeyspaceSettingsRequest{} + var flags struct { replicationDurabilityConstraints *ps.ReplicationDurabilityConstraints vreplicationFlags *ps.VReplicationFlags - maxRollout int - resetMaxRollout bool throttlerEnabled bool throttlerThreshold float64 + maxRollout int interactive bool } @@ -34,48 +35,16 @@ func UpdateSettingsCmd(ch *cmdutil.Helper) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() database, branch, keyspace := args[0], args[1], args[2] - maxRolloutChanged := cmd.Flags().Changed("max-rollout") - resetMaxRolloutChanged := cmd.Flags().Changed("reset-max-rollout") - resetMaxRolloutRequested := resetMaxRolloutChanged && flags.resetMaxRollout - - if maxRolloutChanged && resetMaxRolloutRequested { - return fmt.Errorf("--max-rollout and --reset-max-rollout are mutually exclusive") - } - if flags.interactive && (maxRolloutChanged || resetMaxRolloutChanged) { - return fmt.Errorf("--max-rollout and --reset-max-rollout cannot be used with --interactive") - } - if maxRolloutChanged && (flags.maxRollout < 1 || flags.maxRollout > 32) { - return fmt.Errorf("--max-rollout must be between 1 and 32") - } - if cmd.Flags().Changed("throttler-threshold") && flags.throttlerThreshold < 0 { - return errors.New("--throttler-threshold must be greater than or equal to 0") - } - updateReq := &ps.UpdateKeyspaceSettingsRequest{ - Organization: ch.Config.Organization, - Database: database, - Branch: branch, - Keyspace: keyspace, - } + updateReq.Organization = ch.Config.Organization + updateReq.Database = database + updateReq.Branch = branch + updateReq.Keyspace = keyspace if flags.interactive { return updateInteractive(ctx, ch, updateReq) } - // Nested VReplication and throttler updates read current settings - // first so unspecified flags in that group can be preserved. - rdcChanged := cmd.Flags().Changed("replication-durability-constraints-strategy") - vrfChanged := cmd.Flags().Changed("vreplication-optimize-inserts") || - cmd.Flags().Changed("vreplication-enable-noblob-binlog-mode") || - cmd.Flags().Changed("vreplication-batch-replication-events") - throttlerChanged := cmd.Flags().Changed("throttler-enabled") || - cmd.Flags().Changed("throttler-threshold") - - if !rdcChanged && !vrfChanged && !throttlerChanged && !maxRolloutChanged && !resetMaxRolloutRequested { - ch.Printer.Println("No changes were requested. No update performed.") - return nil - } - client, err := ch.Client() if err != nil { return err @@ -84,18 +53,24 @@ func UpdateSettingsCmd(ch *cmdutil.Helper) *cobra.Command { end := ch.Printer.PrintProgress(fmt.Sprintf("Updating settings for keyspace %s in %s/%s", printer.BoldBlue(keyspace), printer.BoldBlue(database), printer.BoldBlue(branch))) defer end() - if rdcChanged { - updateReq.ReplicationDurabilityConstraints = &ps.ReplicationDurabilityConstraints{ - Strategy: constraintsToStrategy(flags.replicationDurabilityConstraints.Strategy), - } + if err := setInitialSettings(ctx, ch, updateReq); err != nil { + return err } - if vrfChanged || throttlerChanged { - if err := setInitialSettings(ctx, client, updateReq, false, vrfChanged, throttlerChanged); err != nil { - return err + // Check if any relevant flags are changing replication durability constraints + rdcChanged := cmd.Flags().Changed("replication-durability-constraints-strategy") + if rdcChanged { + if updateReq.ReplicationDurabilityConstraints == nil { + updateReq.ReplicationDurabilityConstraints = &ps.ReplicationDurabilityConstraints{} } + updateReq.ReplicationDurabilityConstraints.Strategy = constraintsToStrategy(flags.replicationDurabilityConstraints.Strategy) } + // Check if any relevant flags are changing VReplication flags + vrfChanged := cmd.Flags().Changed("vreplication-optimize-inserts") || + cmd.Flags().Changed("vreplication-enable-noblob-binlog-mode") || + cmd.Flags().Changed("vreplication-batch-replication-events") + if vrfChanged { if updateReq.VReplicationFlags == nil { updateReq.VReplicationFlags = &ps.VReplicationFlags{} @@ -114,6 +89,9 @@ func UpdateSettingsCmd(ch *cmdutil.Helper) *cobra.Command { } } + throttlerChanged := cmd.Flags().Changed("throttler-enabled") || + cmd.Flags().Changed("throttler-threshold") + if throttlerChanged { if updateReq.Throttler == nil { updateReq.Throttler = &ps.KeyspaceThrottler{} @@ -124,16 +102,26 @@ func UpdateSettingsCmd(ch *cmdutil.Helper) *cobra.Command { } if cmd.Flags().Changed("throttler-threshold") { + if flags.throttlerThreshold < 0 { + return errors.New("--throttler-threshold must be greater than or equal to 0") + } updateReq.Throttler.Threshold = &flags.throttlerThreshold } } + maxRolloutChanged := cmd.Flags().Changed("max-rollout") + if maxRolloutChanged { - maxRollout := &flags.maxRollout - updateReq.MaxRollout = &maxRollout - } else if resetMaxRolloutRequested { - var maxRollout *int - updateReq.MaxRollout = &maxRollout + if flags.maxRollout < 1 || flags.maxRollout > 32 { + return errors.New("--max-rollout must be between 1 and 32") + } + updateReq.MaxRollout = &flags.maxRollout + } + + if !rdcChanged && !vrfChanged && !throttlerChanged && !maxRolloutChanged { + end() + ch.Printer.Println("No changes were requested. No update performed.") + return nil } k, err := updateKeyspaceSettings(ctx, client, updateReq) @@ -151,16 +139,20 @@ func UpdateSettingsCmd(ch *cmdutil.Helper) *cobra.Command { cmd.Flags().BoolVar(&flags.vreplicationFlags.OptimizeInserts, "vreplication-optimize-inserts", true, "When enabled, skips sending INSERT events for rows that have yet to be replicated.") cmd.Flags().BoolVar(&flags.vreplicationFlags.AllowNoBlobBinlogRowImage, "vreplication-enable-noblob-binlog-mode", true, "When enabled, omits changed BLOB and TEXT columns from replication events, which reduces binlog sizes.") cmd.Flags().BoolVar(&flags.vreplicationFlags.VPlayerBatching, "vreplication-batch-replication-events", false, "When enabled, sends fewer queries to MySQL to improve performance.") - cmd.Flags().IntVar(&flags.maxRollout, "max-rollout", 1, "Maximum number of concurrent shard rollouts (1-32). The effective service cap is 32.") - cmd.Flags().BoolVar(&flags.resetMaxRollout, "reset-max-rollout", false, "Reset the configured maximum concurrent shard rollouts to the default (1).") cmd.Flags().BoolVar(&flags.throttlerEnabled, "throttler-enabled", true, "Pause schema migrations and VReplication workflows when replication lag rises above the threshold.") cmd.Flags().Float64Var(&flags.throttlerThreshold, "throttler-threshold", 5, "Replication lag in seconds above which migrations and workflows are paused.") + cmd.Flags().IntVar(&flags.maxRollout, "max-rollout", 1, "Maximum number of shards to roll out changes to concurrently (1-32).") cmd.Flags().BoolVarP(&flags.interactive, "interactive", "i", false, "Run the command in interactive mode") return cmd } -func setInitialSettings(ctx context.Context, client *ps.Client, req *ps.UpdateKeyspaceSettingsRequest, includeDurability, includeVReplication, includeThrottler bool) error { +func setInitialSettings(ctx context.Context, ch *cmdutil.Helper, req *ps.UpdateKeyspaceSettingsRequest) error { + client, err := ch.Client() + if err != nil { + return err + } + organization := req.Organization database := req.Database branch := req.Branch @@ -181,18 +173,17 @@ func setInitialSettings(ctx context.Context, client *ps.Client, req *ps.UpdateKe } } - if includeDurability && ks.ReplicationDurabilityConstraints != nil { + // Get initial defaults from the API + if ks.ReplicationDurabilityConstraints != nil { req.ReplicationDurabilityConstraints = ks.ReplicationDurabilityConstraints } - if includeVReplication && ks.VReplicationFlags != nil { - vreplicationFlags := *ks.VReplicationFlags - req.VReplicationFlags = &vreplicationFlags + if ks.VReplicationFlags != nil { + req.VReplicationFlags = ks.VReplicationFlags } - if includeThrottler && ks.Throttler != nil { - throttler := *ks.Throttler - req.Throttler = &throttler + if ks.Throttler != nil { + req.Throttler = ks.Throttler } return nil @@ -204,7 +195,7 @@ func updateInteractive(ctx context.Context, ch *cmdutil.Helper, updateReq *ps.Up return err } - if err := setInitialSettings(ctx, client, updateReq, true, true, true); err != nil { + if err := setInitialSettings(ctx, ch, updateReq); err != nil { return err } diff --git a/internal/cmd/keyspace/update_settings_test.go b/internal/cmd/keyspace/update_settings_test.go index 032bcf2f..80208167 100644 --- a/internal/cmd/keyspace/update_settings_test.go +++ b/internal/cmd/keyspace/update_settings_test.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "errors" - "strings" "testing" "time" @@ -80,7 +79,7 @@ func TestKeyspace_UpdateSettingsCmd_OnlyVReplicationFlags(t *testing.T) { c.Assert(req.Organization, qt.Equals, org) c.Assert(req.Branch, qt.Equals, branch) c.Assert(req.Keyspace, qt.Equals, keyspace) - c.Assert(req.ReplicationDurabilityConstraints, qt.IsNil) + c.Assert(req.ReplicationDurabilityConstraints.Strategy, qt.Equals, rdcStrategy) c.Assert(req.VReplicationFlags.OptimizeInserts, qt.Equals, false) c.Assert(req.VReplicationFlags.AllowNoBlobBinlogRowImage, qt.Equals, false) c.Assert(req.VReplicationFlags.VPlayerBatching, qt.Equals, true) @@ -290,7 +289,9 @@ func TestKeyspace_UpdateSettingsCmd_OnlyDurabilityConstraints(t *testing.T) { c.Assert(req.Branch, qt.Equals, branch) c.Assert(req.Keyspace, qt.Equals, keyspace) c.Assert(req.ReplicationDurabilityConstraints.Strategy, qt.Equals, updatedRdcStrategy) - c.Assert(req.VReplicationFlags, qt.IsNil) + c.Assert(req.VReplicationFlags.OptimizeInserts, qt.Equals, true) + c.Assert(req.VReplicationFlags.AllowNoBlobBinlogRowImage, qt.Equals, true) + c.Assert(req.VReplicationFlags.VPlayerBatching, qt.Equals, false) return updatedKs, nil }, @@ -317,7 +318,7 @@ func TestKeyspace_UpdateSettingsCmd_OnlyDurabilityConstraints(t *testing.T) { }) err := cmd.Execute() c.Assert(err, qt.IsNil) - c.Assert(svc.GetFnInvoked, qt.IsFalse) + c.Assert(svc.GetFnInvoked, qt.IsTrue) c.Assert(svc.UpdateSettingsFnInvoked, qt.IsTrue) c.Assert(buf.String(), qt.JSONEquals, updatedKs) } @@ -383,7 +384,9 @@ func TestKeyspace_UpdateSettingsCmd_NilVReplicationFlags(t *testing.T) { c.Assert(req.Branch, qt.Equals, branch) c.Assert(req.Keyspace, qt.Equals, keyspace) - c.Assert(req.ReplicationDurabilityConstraints, qt.IsNil) + // Check that ReplicationDurabilityConstraints is unchanged and not nil + c.Assert(req.ReplicationDurabilityConstraints, qt.Not(qt.IsNil)) + c.Assert(req.ReplicationDurabilityConstraints.Strategy, qt.Equals, rdcStrategy) // Check that VReplication flags are initialized (since flags were provided) c.Assert(req.VReplicationFlags, qt.Not(qt.IsNil)) @@ -490,7 +493,11 @@ func TestKeyspace_UpdateSettingsCmd_NilReplicationDurabilityConstraints(t *testi c.Assert(req.ReplicationDurabilityConstraints, qt.Not(qt.IsNil)) c.Assert(req.ReplicationDurabilityConstraints.Strategy, qt.Equals, updatedRdcStrategy) - c.Assert(req.VReplicationFlags, qt.IsNil) + // VReplication flags should be maintained and not nil + c.Assert(req.VReplicationFlags, qt.Not(qt.IsNil)) + c.Assert(req.VReplicationFlags.OptimizeInserts, qt.Equals, true) + c.Assert(req.VReplicationFlags.AllowNoBlobBinlogRowImage, qt.Equals, true) + c.Assert(req.VReplicationFlags.VPlayerBatching, qt.Equals, false) return updatedKs, nil }, @@ -517,7 +524,7 @@ func TestKeyspace_UpdateSettingsCmd_NilReplicationDurabilityConstraints(t *testi }) err := cmd.Execute() c.Assert(err, qt.IsNil) - c.Assert(svc.GetFnInvoked, qt.IsFalse) + c.Assert(svc.GetFnInvoked, qt.IsTrue) c.Assert(svc.UpdateSettingsFnInvoked, qt.IsTrue) c.Assert(buf.String(), qt.JSONEquals, updatedKs) } @@ -607,7 +614,7 @@ func TestKeyspace_UpdateSettingsCmd_PreserveNilValues(t *testing.T) { }) err := cmd.Execute() c.Assert(err, qt.IsNil) - c.Assert(svc.GetFnInvoked, qt.IsFalse) + c.Assert(svc.GetFnInvoked, qt.IsTrue) c.Assert(svc.UpdateSettingsFnInvoked, qt.IsTrue) c.Assert(buf.String(), qt.JSONEquals, updatedKs) } @@ -836,268 +843,162 @@ func TestKeyspace_UpdateSettingsCmd_RejectsNegativeThrottlerThreshold(t *testing cmd.SetArgs([]string{db, branch, keyspace, "--throttler-threshold=-1"}) err := cmd.Execute() c.Assert(err, qt.ErrorMatches, ".*throttler-threshold must be greater than or equal to 0") - c.Assert(svc.GetFnInvoked, qt.IsFalse) c.Assert(svc.UpdateSettingsFnInvoked, qt.IsFalse) } -func TestKeyspace_ConstraintsToStrategy(t *testing.T) { - c := qt.New(t) - - // Test the helper function for translating API values to semantic strings - c.Assert(constraintsToStrategy("maximum"), qt.Equals, "available") - c.Assert(constraintsToStrategy("minimum"), qt.Equals, "always") - c.Assert(constraintsToStrategy("dynamic"), qt.Equals, "lag") - c.Assert(constraintsToStrategy("unknown"), qt.Equals, "unknown") -} - func TestKeyspace_UpdateSettingsCmd_MaxRollout(t *testing.T) { c := qt.New(t) + var buf bytes.Buffer format := printer.JSON - maxRollout := 8 - svc := &mock.KeyspacesService{ - UpdateSettingsFn: func(_ context.Context, req *ps.UpdateKeyspaceSettingsRequest) (*ps.Keyspace, error) { - c.Assert(req.ReplicationDurabilityConstraints, qt.IsNil) - c.Assert(req.VReplicationFlags, qt.IsNil) - c.Assert(req.Throttler, qt.IsNil) - c.Assert(req.MaxRollout, qt.Not(qt.IsNil)) - c.Assert(*req.MaxRollout, qt.Not(qt.IsNil)) - c.Assert(**req.MaxRollout, qt.Equals, maxRollout) - return &ps.Keyspace{MaxRollout: &maxRollout}, nil - }, - } p := printer.NewPrinter(&format) p.SetResourceOutput(&buf) - ch := &cmdutil.Helper{ - Printer: p, - Config: &config.Config{Organization: "planetscale"}, - Client: func() (*ps.Client, error) { - return &ps.Client{Keyspaces: svc}, nil - }, - } - cmd := UpdateSettingsCmd(ch) - cmd.SetArgs([]string{"database", "main", "keyspace", "--max-rollout=8", "--reset-max-rollout=false"}) - c.Assert(cmd.Execute(), qt.IsNil) - c.Assert(svc.GetFnInvoked, qt.IsFalse) - c.Assert(svc.UpdateSettingsFnInvoked, qt.IsTrue) - c.Assert(buf.String(), qt.Contains, `"max_rollout": 8`) -} + org := "planetscale" + db := "planetscale" + branch := "main" + keyspace := "sharded" -func TestKeyspace_UpdateSettingsCmd_MaxRolloutWithNestedSettings(t *testing.T) { - c := qt.New(t) - format := printer.JSON + ts := time.Now() maxRollout := 8 - throttlerEnabled := true - throttlerThreshold := 5.0 - getCalls := 0 - initial := &ps.Keyspace{ - VReplicationFlags: &ps.VReplicationFlags{ - OptimizeInserts: true, - AllowNoBlobBinlogRowImage: true, - VPlayerBatching: false, - }, - Throttler: &ps.KeyspaceThrottler{ - Enabled: &throttlerEnabled, - Threshold: &throttlerThreshold, - }, + + updatedKs := &ps.Keyspace{ + ID: "ks1", + Name: keyspace, + CreatedAt: ts, + UpdatedAt: ts, + MaxRollout: &maxRollout, } svc := &mock.KeyspacesService{ - GetFn: func(_ context.Context, _ *ps.GetKeyspaceRequest) (*ps.Keyspace, error) { - getCalls++ - return initial, nil - }, - UpdateSettingsFn: func(_ context.Context, req *ps.UpdateKeyspaceSettingsRequest) (*ps.Keyspace, error) { - c.Assert(req.ReplicationDurabilityConstraints, qt.IsNil) - c.Assert(req.VReplicationFlags.OptimizeInserts, qt.IsFalse) - c.Assert(req.VReplicationFlags.AllowNoBlobBinlogRowImage, qt.IsTrue) - c.Assert(req.VReplicationFlags.VPlayerBatching, qt.IsFalse) - c.Assert(req.Throttler.Enabled, qt.Not(qt.IsNil)) - c.Assert(*req.Throttler.Enabled, qt.IsTrue) - c.Assert(req.Throttler.Threshold, qt.Not(qt.IsNil)) - c.Assert(*req.Throttler.Threshold, qt.Equals, 10.0) + GetFn: func(ctx context.Context, req *ps.GetKeyspaceRequest) (*ps.Keyspace, error) { + return &ps.Keyspace{ID: "ks1", Name: keyspace}, nil + }, + UpdateSettingsFn: func(ctx context.Context, req *ps.UpdateKeyspaceSettingsRequest) (*ps.Keyspace, error) { c.Assert(req.MaxRollout, qt.Not(qt.IsNil)) - c.Assert(*req.MaxRollout, qt.Not(qt.IsNil)) - c.Assert(**req.MaxRollout, qt.Equals, maxRollout) - return &ps.Keyspace{MaxRollout: &maxRollout}, nil + c.Assert(*req.MaxRollout, qt.Equals, 8) + c.Assert(req.Throttler, qt.IsNil) + + return updatedKs, nil }, } + ch := &cmdutil.Helper{ - Printer: printer.NewPrinter(&format), - Config: &config.Config{Organization: "planetscale"}, + Printer: p, + Config: &config.Config{ + Organization: org, + }, Client: func() (*ps.Client, error) { - return &ps.Client{Keyspaces: svc}, nil + return &ps.Client{ + Keyspaces: svc, + }, nil }, } cmd := UpdateSettingsCmd(ch) - cmd.SetArgs([]string{ - "database", - "main", - "keyspace", - "--max-rollout=8", - "--vreplication-optimize-inserts=false", - "--throttler-threshold=10", - }) - - c.Assert(cmd.Execute(), qt.IsNil) - c.Assert(getCalls, qt.Equals, 1) + cmd.SetArgs([]string{db, branch, keyspace, "--max-rollout=8"}) + err := cmd.Execute() + c.Assert(err, qt.IsNil) c.Assert(svc.UpdateSettingsFnInvoked, qt.IsTrue) + c.Assert(buf.String(), qt.JSONEquals, updatedKs) } -func TestKeyspace_UpdateSettingsCmd_ResetMaxRollout(t *testing.T) { +func TestKeyspace_UpdateSettingsCmd_RejectsOutOfRangeMaxRollout(t *testing.T) { c := qt.New(t) + var buf bytes.Buffer format := printer.JSON - svc := &mock.KeyspacesService{ - UpdateSettingsFn: func(_ context.Context, req *ps.UpdateKeyspaceSettingsRequest) (*ps.Keyspace, error) { - c.Assert(req.ReplicationDurabilityConstraints, qt.IsNil) - c.Assert(req.VReplicationFlags, qt.IsNil) - c.Assert(req.Throttler, qt.IsNil) - c.Assert(req.MaxRollout, qt.Not(qt.IsNil)) - c.Assert(*req.MaxRollout, qt.IsNil) - return &ps.Keyspace{}, nil - }, - } p := printer.NewPrinter(&format) p.SetResourceOutput(&buf) - ch := &cmdutil.Helper{ - Printer: p, - Config: &config.Config{Organization: "planetscale"}, - Client: func() (*ps.Client, error) { - return &ps.Client{Keyspaces: svc}, nil - }, - } - - cmd := UpdateSettingsCmd(ch) - cmd.SetArgs([]string{"database", "main", "keyspace", "--reset-max-rollout"}) - c.Assert(cmd.Execute(), qt.IsNil) - c.Assert(svc.GetFnInvoked, qt.IsFalse) - c.Assert(svc.UpdateSettingsFnInvoked, qt.IsTrue) - c.Assert(buf.String(), qt.Contains, `"max_rollout": null`) -} -func TestKeyspace_UpdateSettingsCmd_MaxRolloutValidationBeforeAPI(t *testing.T) { - for _, tt := range []struct { - name string - args []string - wantError string - }{ - {name: "too low", args: []string{"--max-rollout=0"}, wantError: `--max-rollout must be between 1 and 32`}, - {name: "too high", args: []string{"--max-rollout=33"}, wantError: `--max-rollout must be between 1 and 32`}, - {name: "set and reset", args: []string{"--max-rollout=8", "--reset-max-rollout"}, wantError: `--max-rollout and --reset-max-rollout are mutually exclusive`}, - {name: "set interactively", args: []string{"--interactive", "--max-rollout=8"}, wantError: `--max-rollout and --reset-max-rollout cannot be used with --interactive`}, - {name: "reset interactively", args: []string{"--interactive", "--reset-max-rollout"}, wantError: `--max-rollout and --reset-max-rollout cannot be used with --interactive`}, - {name: "explicit false reset interactively", args: []string{"--interactive", "--reset-max-rollout=false"}, wantError: `--max-rollout and --reset-max-rollout cannot be used with --interactive`}, - } { - t.Run(tt.name, func(t *testing.T) { - c := qt.New(t) - format := printer.Human - clientCalled := false - ch := &cmdutil.Helper{ - Printer: printer.NewPrinter(&format), - Config: &config.Config{Organization: "planetscale"}, - Client: func() (*ps.Client, error) { - clientCalled = true - return nil, errors.New("unexpected API client call") - }, - } - cmd := UpdateSettingsCmd(ch) - cmd.SetArgs(append([]string{"database", "main", "keyspace"}, tt.args...)) - c.Assert(cmd.Execute(), qt.ErrorMatches, tt.wantError) - c.Assert(clientCalled, qt.IsFalse) - }) - } -} + org := "planetscale" + db := "planetscale" + branch := "main" + keyspace := "sharded" -func TestKeyspace_UpdateSettingsCmd_MaxRolloutHelpDefault(t *testing.T) { - c := qt.New(t) - format := printer.Human - ch := &cmdutil.Helper{ - Printer: printer.NewPrinter(&format), - Config: &config.Config{Organization: "planetscale"}, - } + for _, arg := range []string{"--max-rollout=0", "--max-rollout=33"} { + svc := &mock.KeyspacesService{ + GetFn: func(ctx context.Context, req *ps.GetKeyspaceRequest) (*ps.Keyspace, error) { + return &ps.Keyspace{ID: "ks1", Name: keyspace}, nil + }, + } - cmd := UpdateSettingsCmd(ch) - var maxRolloutUsage string - for _, line := range strings.Split(cmd.Flags().FlagUsages(), "\n") { - if strings.Contains(line, "--max-rollout") { - maxRolloutUsage = line - break + ch := &cmdutil.Helper{ + Printer: p, + Config: &config.Config{ + Organization: org, + }, + Client: func() (*ps.Client, error) { + return &ps.Client{ + Keyspaces: svc, + }, nil + }, } - } - c.Assert(maxRolloutUsage, qt.Not(qt.Equals), "") - c.Assert(maxRolloutUsage, qt.Contains, "(default 1)") -} -func TestKeyspace_UpdateSettingsCmd_ResetMaxRolloutFalseIsNoOp(t *testing.T) { - c := qt.New(t) - format := printer.Human - clientCalled := false - ch := &cmdutil.Helper{ - Printer: printer.NewPrinter(&format), - Config: &config.Config{Organization: "planetscale"}, - Client: func() (*ps.Client, error) { - clientCalled = true - return nil, errors.New("unexpected API client call") - }, + cmd := UpdateSettingsCmd(ch) + cmd.SetArgs([]string{db, branch, keyspace, arg}) + err := cmd.Execute() + c.Assert(err, qt.ErrorMatches, ".*max-rollout must be between 1 and 32") + c.Assert(svc.UpdateSettingsFnInvoked, qt.IsFalse) } - - cmd := UpdateSettingsCmd(ch) - cmd.SetArgs([]string{"database", "main", "keyspace", "--reset-max-rollout=false"}) - c.Assert(cmd.Execute(), qt.IsNil) - c.Assert(clientCalled, qt.IsFalse) } -func TestKeyspace_UpdateSettingsCmd_PreservesUnspecifiedVReplicationFlags(t *testing.T) { +func TestKeyspace_UpdateSettingsCmd_OmittedMaxRolloutIsNotSent(t *testing.T) { c := qt.New(t) + var buf bytes.Buffer format := printer.JSON - initial := &ps.Keyspace{ - VReplicationFlags: &ps.VReplicationFlags{ - OptimizeInserts: true, - AllowNoBlobBinlogRowImage: true, - VPlayerBatching: false, - }, - } - updated := &ps.Keyspace{ - VReplicationFlags: &ps.VReplicationFlags{ - OptimizeInserts: false, - AllowNoBlobBinlogRowImage: true, - VPlayerBatching: false, - }, - } + + p := printer.NewPrinter(&format) + p.SetResourceOutput(&buf) + + org := "planetscale" + db := "planetscale" + branch := "main" + keyspace := "sharded" + svc := &mock.KeyspacesService{ - GetFn: func(_ context.Context, _ *ps.GetKeyspaceRequest) (*ps.Keyspace, error) { - return initial, nil + GetFn: func(ctx context.Context, req *ps.GetKeyspaceRequest) (*ps.Keyspace, error) { + return &ps.Keyspace{ID: "ks1", Name: keyspace}, nil }, - UpdateSettingsFn: func(_ context.Context, req *ps.UpdateKeyspaceSettingsRequest) (*ps.Keyspace, error) { - c.Assert(req.ReplicationDurabilityConstraints, qt.IsNil) + UpdateSettingsFn: func(ctx context.Context, req *ps.UpdateKeyspaceSettingsRequest) (*ps.Keyspace, error) { c.Assert(req.MaxRollout, qt.IsNil) - c.Assert(req.VReplicationFlags, qt.DeepEquals, updated.VReplicationFlags) - return updated, nil + + return &ps.Keyspace{ID: "ks1", Name: keyspace}, nil }, } - p := printer.NewPrinter(&format) - p.SetResourceOutput(&buf) + ch := &cmdutil.Helper{ Printer: p, - Config: &config.Config{Organization: "planetscale"}, + Config: &config.Config{ + Organization: org, + }, Client: func() (*ps.Client, error) { - return &ps.Client{Keyspaces: svc}, nil + return &ps.Client{ + Keyspaces: svc, + }, nil }, } cmd := UpdateSettingsCmd(ch) - cmd.SetArgs([]string{"database", "main", "keyspace", "--vreplication-optimize-inserts=false"}) - c.Assert(cmd.Execute(), qt.IsNil) - c.Assert(svc.GetFnInvoked, qt.IsTrue) + cmd.SetArgs([]string{db, branch, keyspace, "--throttler-threshold=10"}) + err := cmd.Execute() + c.Assert(err, qt.IsNil) c.Assert(svc.UpdateSettingsFnInvoked, qt.IsTrue) } +func TestKeyspace_ConstraintsToStrategy(t *testing.T) { + c := qt.New(t) + + // Test the helper function for translating API values to semantic strings + c.Assert(constraintsToStrategy("maximum"), qt.Equals, "available") + c.Assert(constraintsToStrategy("minimum"), qt.Equals, "always") + c.Assert(constraintsToStrategy("dynamic"), qt.Equals, "lag") + c.Assert(constraintsToStrategy("unknown"), qt.Equals, "unknown") +} + func TestKeyspace_UpdateSettingsCmd_ErrorNotFound(t *testing.T) { c := qt.New(t) @@ -1133,7 +1034,7 @@ func TestKeyspace_UpdateSettingsCmd_ErrorNotFound(t *testing.T) { } cmd := UpdateSettingsCmd(ch) - cmd.SetArgs([]string{db, branch, keyspace, "--vreplication-optimize-inserts=false"}) + cmd.SetArgs([]string{db, branch, keyspace}) err := cmd.Execute() c.Assert(err, qt.Not(qt.IsNil)) // Just check that there is an error c.Assert(svc.GetFnInvoked, qt.IsTrue) diff --git a/internal/planetscale/keyspaces.go b/internal/planetscale/keyspaces.go index 2d2d13cb..3d60c78b 100644 --- a/internal/planetscale/keyspaces.go +++ b/internal/planetscale/keyspaces.go @@ -177,9 +177,7 @@ type UpdateKeyspaceSettingsRequest struct { ReplicationDurabilityConstraints *ReplicationDurabilityConstraints `json:"replication_durability_constraints,omitempty"` VReplicationFlags *VReplicationFlags `json:"vreplication_flags,omitempty"` Throttler *KeyspaceThrottler `json:"throttler,omitempty"` - // MaxRollout is a tri-state PATCH field: nil omits max_rollout, a pointer - // to an integer sets it, and a pointer to nil sends JSON null to reset it. - MaxRollout **int `json:"max_rollout,omitempty"` + MaxRollout *int `json:"max_rollout,omitempty"` } type ReplicationDurabilityConstraints struct { diff --git a/internal/planetscale/keyspaces_test.go b/internal/planetscale/keyspaces_test.go index 4a8fdf82..993e983c 100644 --- a/internal/planetscale/keyspaces_test.go +++ b/internal/planetscale/keyspaces_test.go @@ -6,6 +6,7 @@ import ( "io" "net/http" "net/http/httptest" + "strings" "testing" qt "github.com/frankban/quicktest" @@ -481,62 +482,40 @@ func TestKeyspaces_UpdateSettings(t *testing.T) { c.Assert(keyspace.ReplicationDurabilityConstraints.Strategy, qt.Equals, "maximum") } -func TestKeyspaces_UpdateSettingsMaxRolloutPayload(t *testing.T) { - for _, tt := range []struct { - name string - maxRollout func() **int - wantBody string - }{ - { - name: "omitted", - maxRollout: func() **int { - return nil - }, - wantBody: `{}`, - }, - { - name: "integer", - maxRollout: func() **int { - value := 8 - valuePointer := &value - return &valuePointer - }, - wantBody: `{"max_rollout":8}`, - }, - { - name: "null", - maxRollout: func() **int { - var value *int - return &value - }, - wantBody: `{"max_rollout":null}`, - }, - } { - t.Run(tt.name, func(t *testing.T) { - c := qt.New(t) - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - body, err := io.ReadAll(r.Body) - c.Assert(err, qt.IsNil) - c.Assert(r.Method, qt.Equals, http.MethodPatch) - c.Assert(string(body), qt.JSONEquals, json.RawMessage(tt.wantBody)) - _, err = w.Write([]byte(`{"max_rollout":64}`)) - c.Assert(err, qt.IsNil) - })) - defer ts.Close() - - client, err := NewClient(WithBaseURL(ts.URL)) - c.Assert(err, qt.IsNil) - - keyspace, err := client.Keyspaces.UpdateSettings(context.Background(), &UpdateKeyspaceSettingsRequest{ - Organization: "foo", - Database: "bar", - Branch: "baz", - Keyspace: "qux", - MaxRollout: tt.maxRollout(), - }) - c.Assert(err, qt.IsNil) - c.Assert(keyspace.MaxRollout, qt.Not(qt.IsNil)) - c.Assert(*keyspace.MaxRollout, qt.Equals, 64) - }) - } +func TestKeyspaces_UpdateSettingsMaxRollout(t *testing.T) { + c := qt.New(t) + + var body string + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + c.Assert(r.Method, qt.Equals, http.MethodPatch) + + raw, err := io.ReadAll(r.Body) + c.Assert(err, qt.IsNil) + body = string(raw) + + w.WriteHeader(200) + out := `{"type":"Keyspace","id":"thisisanid","name":"planetscale","max_rollout":8}` + _, err = w.Write([]byte(out)) + c.Assert(err, qt.IsNil) + })) + + client, err := NewClient(WithBaseURL(ts.URL)) + c.Assert(err, qt.IsNil) + + ctx := context.Background() + maxRollout := 8 + + keyspace, err := client.Keyspaces.UpdateSettings(ctx, &UpdateKeyspaceSettingsRequest{ + Organization: "foo", + Database: "bar", + Branch: "baz", + Keyspace: "qux", + MaxRollout: &maxRollout, + }) + + c.Assert(err, qt.IsNil) + c.Assert(strings.TrimSpace(body), qt.Equals, `{"max_rollout":8}`) + c.Assert(keyspace.MaxRollout, qt.Not(qt.IsNil)) + c.Assert(*keyspace.MaxRollout, qt.Equals, 8) }