Skip to content

Commit 03b1d4c

Browse files
craig[bot]knzkvolijoshimhoff
committed
108100: server: init secondary tenant servers with proper clock parameters r=pavelkalinnikov a=knz We need to share the code path to initialize the HLC clock between all types of servers. Fixes cockroachdb#108098. Epic: CRDB-26687 108107: ccl/multiregionccl: skip TestMultiRegionDataDriven r=maryliag,chengxiong-ruan a=kvoli Failing consistently on nightly stress, skip. Informs: cockroachdb#98020 Epic: none Release note: None 108108: kvprober: remove unnecessary checks to avoid flakes r=joshimhoff a=joshimhoff Fixes cockroachdb#107873. **kvprober: remove unnecessary checks to avoid flakes** This commit removes two unnecessary checks, to avoid flakes. The removed checks can cause flakes, since they require all probes (of a certain type) to fail, even tho the test server is not configured to fail probes until after server startup. That is, there is a race condition. The checks are also unnecessary. What we really care about is the checks that follow the deleted ones: That the ten probes run within test all fail, since those happen after the test server is configured to fail probes. Release note: None. Epic: None. Co-authored-by: Raphael 'kena' Poss <[email protected]> Co-authored-by: Austen McClernon <[email protected]> Co-authored-by: Josh Imhoff <[email protected]>
4 parents ab13de5 + 3c12ccd + f8a4740 + 66be61f commit 03b1d4c

File tree

4 files changed

+29
-22
lines changed

4 files changed

+29
-22
lines changed

pkg/ccl/multiregionccl/datadriven_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ func TestMultiRegionDataDriven(t *testing.T) {
114114
defer log.Scope(t).Close(t)
115115

116116
skip.UnderRace(t, "flaky test")
117+
skip.WithIssue(t, 98020)
117118
ctx := context.Background()
118119
datadriven.Walk(t, datapathutils.TestDataPath(t), func(t *testing.T, path string) {
119120
ds := datadrivenTestState{}

pkg/kv/kvprober/kvprober_integration_test.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -171,8 +171,6 @@ func TestProberDoesReadsAndWrites(t *testing.T) {
171171
p.WriteProbe(ctx, s.DB())
172172
}
173173

174-
// Expect all read probes to fail but write probes & planning to succeed.
175-
require.Equal(t, p.Metrics().ReadProbeAttempts.Count(), p.Metrics().ReadProbeFailures.Count())
176174
// kvprober is running in background, so more than ten probes may be run.
177175
require.GreaterOrEqual(t, p.Metrics().ReadProbeFailures.Count(), int64(10))
178176

@@ -220,8 +218,6 @@ func TestProberDoesReadsAndWrites(t *testing.T) {
220218
p.WriteProbe(ctx, s.DB())
221219
}
222220

223-
// Expect all write probes to fail but read probes & planning to succeed.
224-
require.Equal(t, p.Metrics().WriteProbeAttempts.Count(), p.Metrics().WriteProbeFailures.Count())
225221
// kvprober is running in background, so more than ten probes may be run.
226222
require.GreaterOrEqual(t, p.Metrics().WriteProbeFailures.Count(), int64(10))
227223

pkg/server/server.go

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -239,21 +239,9 @@ func NewServer(cfg Config, stopper *stop.Stopper) (serverctl.ServerStartupInterf
239239
panic(errors.New("no tracer set in AmbientCtx"))
240240
}
241241

242-
maxOffset := time.Duration(cfg.MaxOffset)
243-
toleratedOffset := cfg.ToleratedOffset()
244-
var clock *hlc.Clock
245-
if cfg.ClockDevicePath != "" {
246-
ptpClock, err := ptp.MakeClock(ctx, cfg.ClockDevicePath)
247-
if err != nil {
248-
return nil, errors.Wrap(err, "instantiating clock source")
249-
}
250-
clock = hlc.NewClock(ptpClock, maxOffset, toleratedOffset)
251-
} else if cfg.TestingKnobs.Server != nil &&
252-
cfg.TestingKnobs.Server.(*TestingKnobs).WallClock != nil {
253-
clock = hlc.NewClock(cfg.TestingKnobs.Server.(*TestingKnobs).WallClock,
254-
maxOffset, toleratedOffset)
255-
} else {
256-
clock = hlc.NewClockWithSystemTimeSource(maxOffset, toleratedOffset)
242+
clock, err := newClockFromConfig(ctx, cfg.BaseConfig)
243+
if err != nil {
244+
return nil, err
257245
}
258246
registry := metric.NewRegistry()
259247
ruleRegistry := metric.NewRuleRegistry()
@@ -1319,6 +1307,27 @@ func NewServer(cfg Config, stopper *stop.Stopper) (serverctl.ServerStartupInterf
13191307
return lateBoundServer, err
13201308
}
13211309

1310+
// newClockFromConfig creates a HLC clock from the server configuration.
1311+
func newClockFromConfig(ctx context.Context, cfg BaseConfig) (*hlc.Clock, error) {
1312+
maxOffset := time.Duration(cfg.MaxOffset)
1313+
toleratedOffset := cfg.ToleratedOffset()
1314+
var clock *hlc.Clock
1315+
if cfg.ClockDevicePath != "" {
1316+
ptpClock, err := ptp.MakeClock(ctx, cfg.ClockDevicePath)
1317+
if err != nil {
1318+
return nil, errors.Wrap(err, "instantiating clock source")
1319+
}
1320+
clock = hlc.NewClock(ptpClock, maxOffset, toleratedOffset)
1321+
} else if cfg.TestingKnobs.Server != nil &&
1322+
cfg.TestingKnobs.Server.(*TestingKnobs).WallClock != nil {
1323+
clock = hlc.NewClock(cfg.TestingKnobs.Server.(*TestingKnobs).WallClock,
1324+
maxOffset, toleratedOffset)
1325+
} else {
1326+
clock = hlc.NewClockWithSystemTimeSource(maxOffset, toleratedOffset)
1327+
}
1328+
return clock, nil
1329+
}
1330+
13221331
// ClusterSettings returns the cluster settings.
13231332
func (s *topLevelServer) ClusterSettings() *cluster.Settings {
13241333
return s.st

pkg/server/tenant.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -978,9 +978,10 @@ func makeTenantSQLServerArgs(
978978
// the instance ID (once known) as a tag.
979979
startupCtx = baseCfg.AmbientCtx.AnnotateCtx(startupCtx)
980980

981-
maxOffset := time.Duration(baseCfg.MaxOffset)
982-
toleratedOffset := baseCfg.ToleratedOffset()
983-
clock := hlc.NewClockWithSystemTimeSource(maxOffset, toleratedOffset)
981+
clock, err := newClockFromConfig(startupCtx, baseCfg)
982+
if err != nil {
983+
return sqlServerArgs{}, err
984+
}
984985

985986
registry := metric.NewRegistry()
986987
ruleRegistry := metric.NewRuleRegistry()

0 commit comments

Comments
 (0)