Skip to content

Commit

Permalink
Merge #142108
Browse files Browse the repository at this point in the history
142108: testserver: set default rf to 1 for single node servers r=rafiss a=msbutler

We already set the default rf to 1 for non system ranges. This patch skips a schema change run to set num replicas for system ranges to 1 and sets the system ranges rf to 1 by default instead. This patch reduces the test runtime of server startup by abour 15%, post initialization:

```
❯ benchdiff --old  281a729 --new c14e2c1 ./pkg/server -r BenchmarkTestServerStartup  -c 15 --preview -b
old:  281a729 Merge #142062 #142079
new:  c14e2c1 testserver: set default rf to 1 for single node se
args: benchdiff "--old" "281a7294cf1360446177ceed1e25e141d19e0c6e" "--new" "c14e2c15b8697e62a8916cd5c60a1be43c54fdf1" "./pkg/server" "-r" "BenchmarkTestServerStartup" "-c" "15" "--preview" "-b"

name                                   old time/op    new time/op    delta
TestServerStartup/ExternalTenant-24       390ms ±10%     320ms ±26%  -18.06%  (p=0.000 n=15+15)
TestServerStartup/SharedTenant-24         390ms ±16%     328ms ± 7%  -15.85%  (p=0.000 n=15+14)
TestServerStartup/SystemTenantOnly-24     386ms ±13%     336ms ±12%  -13.01%  (p=0.000 n=15+15)

name                                   old alloc/op   new alloc/op   delta
TestServerStartup/SystemTenantOnly-24     133MB ± 0%     119MB ± 0%  -10.21%  (p=0.000 n=15+15)
TestServerStartup/ExternalTenant-24       133MB ± 0%     119MB ± 1%  -10.19%  (p=0.000 n=12+15)
TestServerStartup/SharedTenant-24         133MB ± 1%     119MB ± 1%  -10.18%  (p=0.000 n=15+14)

name                                   old allocs/op  new allocs/op  delta
TestServerStartup/SystemTenantOnly-24      799k ± 0%      701k ± 0%  -12.34%  (p=0.000 n=15+15)
TestServerStartup/SharedTenant-24          799k ± 0%      701k ± 0%  -12.33%  (p=0.000 n=15+15)
TestServerStartup/ExternalTenant-24        799k ± 0%      701k ± 0%  -12.29%  (p=0.000 n=15+15)
```

Epic: none

Release note: none

Co-authored-by: Michael Butler <[email protected]>
  • Loading branch information
craig[bot] and msbutler committed Feb 28, 2025
2 parents 1fe3c90 + b30f967 commit 72dd5b2
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 1 deletion.
2 changes: 1 addition & 1 deletion pkg/server/initial_sql.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func (s *topLevelServer) RunInitialSQL(
return nil
}

if startSingleNode {
if startSingleNode && (*s.cfg.DefaultSystemZoneConfig.NumReplicas != 1 || *s.cfg.DefaultZoneConfig.NumReplicas != 1) {
// For start-single-node, set the default replication factor to
// 1 so as to avoid warning messages and unnecessary rebalance
// churn.
Expand Down
1 change: 1 addition & 0 deletions pkg/server/testserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -2496,6 +2496,7 @@ func (testServerFactoryImpl) New(params base.TestServerArgs) (interface{}, error

if !params.PartOfCluster {
ts.Cfg.DefaultZoneConfig.NumReplicas = proto.Int32(1)
ts.Cfg.DefaultSystemZoneConfig.NumReplicas = proto.Int32(1)
}

// Needs to be called before NewServer to ensure resolvers are initialized.
Expand Down
14 changes: 14 additions & 0 deletions pkg/server/testserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/log"
)
Expand Down Expand Up @@ -85,3 +86,16 @@ func TestServerStartup(t *testing.T) {
})
}
}

func TestServerProperties(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)

s, sql, _ := serverutils.StartServer(t, base.TestServerArgs{})
defer s.Stopper().Stop(context.Background())

sqlDB := sqlutils.MakeSQLRunner(sql)

// Ensure all ranges have 1 replica
sqlDB.CheckQueryResults(t, "select count(*) from [show cluster ranges] where replicas = '{1}'", sqlDB.QueryStr(t, "select count(*) from [show cluster ranges]"))
}

0 comments on commit 72dd5b2

Please sign in to comment.