Skip to content

Commit f13456f

Browse files
sighphyresighphyre
authored
fix: use sync pool for strategy randomness (#212)
* fix: use a sync pool for strategy randomness, which offers slightly better perf under load * chore: rework metrics to use less locking on the hotpath * fix: variant strategies now correctly inherit stickiness from their strategy * chore: make metric count channels best effort send
1 parent 19e25a8 commit f13456f

11 files changed

Lines changed: 253 additions & 94 deletions

‎api/feature.go‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,12 +86,12 @@ func (fr FeatureResponse) SegmentsMap() map[int][]Constraint {
8686
}
8787

8888
// Get variant for a given feature which is considered as enabled
89-
func (vc VariantCollection) GetVariant(ctx *context.Context) *Variant {
89+
func (vc VariantCollection) GetVariant(ctx *context.Context, stickinessOverride *string) *Variant {
9090
if len(vc.Variants) > 0 {
9191
v := vc.getOverrideVariant(ctx)
9292
var variant *Variant
9393
if v == nil {
94-
variant = vc.getVariantFromWeights(ctx)
94+
variant = vc.getVariantFromWeights(ctx, stickinessOverride)
9595
} else {
9696
variant = &v.Variant
9797
}
@@ -102,15 +102,19 @@ func (vc VariantCollection) GetVariant(ctx *context.Context) *Variant {
102102
return DISABLED_VARIANT
103103
}
104104

105-
func (vc VariantCollection) getVariantFromWeights(ctx *context.Context) *Variant {
105+
func (vc VariantCollection) getVariantFromWeights(ctx *context.Context, stickinessOverride *string) *Variant {
106106
totalWeight := 0
107107
for _, variant := range vc.Variants {
108108
totalWeight += variant.Weight
109109
}
110110
if totalWeight == 0 {
111111
return DISABLED_VARIANT
112112
}
113+
113114
stickiness := vc.Variants[0].Stickiness
115+
if stickinessOverride != nil {
116+
stickiness = *stickinessOverride
117+
}
114118

115119
target := strategies.NormalizedVariantValue(getSeed(ctx, stickiness), vc.GroupId, totalWeight, strategies.VariantNormalizationSeed)
116120
counter := uint32(0)

‎api/variant_test.go‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ func (suite *VariantTestSuite) TestGetVariantWhenFeatureHasNoVariant() {
132132
variantSetup := VariantCollection{
133133
GroupId: mockFeature.Name,
134134
Variants: mockFeature.Variants,
135-
}.GetVariant(mockContext)
135+
}.GetVariant(mockContext, nil)
136136

137137
suite.Equal(DISABLED_VARIANT, variantSetup, "Should return default variant")
138138
}
@@ -155,7 +155,7 @@ func (suite *VariantTestSuite) TestGetVariant_OverrideOnUserId() {
155155
variantSetup := VariantCollection{
156156
GroupId: mockFeature.Name,
157157
Variants: mockFeature.Variants,
158-
}.GetVariant(mockContext)
158+
}.GetVariant(mockContext, nil)
159159
suite.Equal("VarA", variantSetup.Name, "Should return VarA")
160160
suite.Equal(true, variantSetup.Enabled, "Should be equal")
161161
suite.Equal(expectedPayload, variantSetup.Payload, "Should be equal")
@@ -178,7 +178,7 @@ func (suite *VariantTestSuite) TestGetVariant_OverrideOnRemoteAddress() {
178178
variantSetup := VariantCollection{
179179
GroupId: mockFeature.Name,
180180
Variants: mockFeature.Variants,
181-
}.GetVariant(mockContext)
181+
}.GetVariant(mockContext, nil)
182182
suite.Equal("VarB", variantSetup.Name, "Should return VarB")
183183
suite.Equal(true, variantSetup.Enabled, "Should be equal")
184184
suite.Equal(expectedPayload, variantSetup.Payload, "Should be equal")
@@ -202,7 +202,7 @@ func (suite *VariantTestSuite) TestGetVariant_OverrideOnSessionId() {
202202
variantSetup := VariantCollection{
203203
GroupId: mockFeature.Name,
204204
Variants: mockFeature.Variants,
205-
}.GetVariant(mockContext)
205+
}.GetVariant(mockContext, nil)
206206
suite.Equal("VarA", variantSetup.Name, "Should return VarA")
207207
suite.Equal(true, variantSetup.Enabled, "Should be equal")
208208
suite.Equal(expectedPayload, variantSetup.Payload, "Should be equal")
@@ -226,7 +226,7 @@ func (suite *VariantTestSuite) TestGetVariant_OverrideOnCustomProperties() {
226226
variantSetup := VariantCollection{
227227
GroupId: mockFeature.Name,
228228
Variants: mockFeature.Variants,
229-
}.GetVariant(mockContext)
229+
}.GetVariant(mockContext, nil)
230230
suite.Equal("VarC", variantSetup.Name, "Should return VarC")
231231
suite.Equal(true, variantSetup.Enabled, "Should be equal")
232232
suite.Equal(expectedPayload, variantSetup.Payload, "Should be equal")
@@ -244,7 +244,7 @@ func (suite *VariantTestSuite) TestGetVariant_ShouldReturnVarD() {
244244
variantSetup := VariantCollection{
245245
GroupId: mockFeature.Name,
246246
Variants: mockFeature.Variants,
247-
}.GetVariant(mockContext)
247+
}.GetVariant(mockContext, nil)
248248
suite.Equal("VarE", variantSetup.Name, "Should return VarE")
249249
suite.Equal(true, variantSetup.Enabled, "Should be equal")
250250
}
@@ -261,7 +261,7 @@ func (suite *VariantTestSuite) TestGetVariant_ShouldReturnVarE() {
261261
variantSetup := VariantCollection{
262262
GroupId: mockFeature.Name,
263263
Variants: mockFeature.Variants,
264-
}.GetVariant(mockContext)
264+
}.GetVariant(mockContext, nil)
265265
suite.Equal("VarF", variantSetup.Name, "Should return VarF")
266266
suite.Equal(true, variantSetup.Enabled, "Should be equal")
267267
}
@@ -278,7 +278,7 @@ func (suite *VariantTestSuite) TestGetVariant_ShouldReturnVarF() {
278278
variantSetup := VariantCollection{
279279
GroupId: mockFeature.Name,
280280
Variants: mockFeature.Variants,
281-
}.GetVariant(mockContext)
281+
}.GetVariant(mockContext, nil)
282282
suite.Equal("VarE", variantSetup.Name, "Should return VarE")
283283
suite.Equal(true, variantSetup.Enabled, "Should be equal")
284284
}
@@ -304,7 +304,7 @@ func (suite *VariantTestSuite) TestGetVariant_OverrideOnAppName() {
304304
variantSetup := VariantCollection{
305305
GroupId: mockFeature.Name,
306306
Variants: mockFeature.Variants,
307-
}.GetVariant(mockContext)
307+
}.GetVariant(mockContext, nil)
308308
suite.Equal("VarG", variantSetup.Name, "Should return VarG")
309309
suite.Equal(true, variantSetup.Enabled, "Should be equal")
310310
suite.Equal(expectedPayload, variantSetup.Payload, "Should be equal")
@@ -326,7 +326,7 @@ func (suite *VariantTestSuite) TestGetVariant_OverrideOnEnvironment() {
326326
variantSetup := VariantCollection{
327327
GroupId: mockFeature.Name,
328328
Variants: mockFeature.Variants,
329-
}.GetVariant(mockContext)
329+
}.GetVariant(mockContext, nil)
330330
suite.Equal("VarG", variantSetup.Name, "Should return VarG")
331331
suite.Equal(true, variantSetup.Enabled, "Should be equal")
332332
suite.Equal(expectedPayload, variantSetup.Payload, "Should be equal")

‎client.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,7 @@ func (uc *Client) getVariantWithoutMetrics(feature string, snapshot *FeatureMemo
431431
return api.VariantCollection{
432432
GroupId: f.Name,
433433
Variants: f.Variants,
434-
}.GetVariant(ctx)
434+
}.GetVariant(ctx, nil)
435435
}
436436

437437
// Close stops the client from syncing data from the server.

‎client_test.go‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ func TestClient_WithFallbackFunc(t *testing.T) {
5959
mockListener := &MockedListener{}
6060
mockListener.On("OnReady").Return()
6161
mockListener.On("OnRegistered", mock.AnythingOfType("ClientData"))
62-
mockListener.On("OnCount", feature, true).Return()
62+
mockListener.On("OnCount", feature, true).Maybe()
6363
mockListener.On("OnError").Return()
6464

6565
client, err := NewClient(
@@ -103,7 +103,7 @@ func TestClient_WithResolver(t *testing.T) {
103103
mockListener := &MockedListener{}
104104
mockListener.On("OnReady").Return()
105105
mockListener.On("OnRegistered", mock.AnythingOfType("ClientData"))
106-
mockListener.On("OnCount", feature, true).Return()
106+
mockListener.On("OnCount", feature, true).Maybe()
107107
mockListener.On("OnError").Return()
108108

109109
client, err := NewClient(
@@ -335,7 +335,7 @@ func TestClientWithVariantContext(t *testing.T) {
335335

336336
mockListener := &MockedListener{}
337337
mockListener.On("OnReady").Return()
338-
mockListener.On("OnCount", mock.AnythingOfType("string"), mock.AnythingOfType("bool")).Return()
338+
mockListener.On("OnCount", mock.AnythingOfType("string"), mock.AnythingOfType("bool")).Maybe()
339339
mockListener.On("OnRegistered", mock.AnythingOfType("ClientData"))
340340
mockListener.On("OnError", mock.AnythingOfType("*errors.errorString"))
341341

@@ -424,7 +424,7 @@ func TestClient_WithSegment(t *testing.T) {
424424
mockListener := &MockedListener{}
425425
mockListener.On("OnReady").Return()
426426
mockListener.On("OnRegistered", mock.AnythingOfType("ClientData"))
427-
mockListener.On("OnCount", feature, true).Return()
427+
mockListener.On("OnCount", feature, true).Maybe()
428428
mockListener.On("OnError").Return()
429429

430430
client, err := NewClient(
@@ -502,7 +502,7 @@ func TestClient_WithNonExistingSegment(t *testing.T) {
502502
mockListener := &MockedListener{}
503503
mockListener.On("OnReady").Return()
504504
mockListener.On("OnRegistered", mock.AnythingOfType("ClientData"))
505-
mockListener.On("OnCount", feature, false).Return()
505+
mockListener.On("OnCount", feature, false).Maybe()
506506
mockListener.On("OnError", mock.AnythingOfType("*errors.errorString"))
507507

508508
client, err := NewClient(
@@ -606,7 +606,7 @@ func TestClient_WithMultipleSegments(t *testing.T) {
606606
mockListener := &MockedListener{}
607607
mockListener.On("OnReady").Return()
608608
mockListener.On("OnRegistered", mock.AnythingOfType("ClientData"))
609-
mockListener.On("OnCount", feature, true).Return()
609+
mockListener.On("OnCount", feature, true).Maybe()
610610
mockListener.On("OnError").Return()
611611

612612
client, err := NewClient(
@@ -721,7 +721,7 @@ func TestClient_VariantShouldRespectConstraint(t *testing.T) {
721721
mockListener := &MockedListener{}
722722
mockListener.On("OnReady").Return()
723723
mockListener.On("OnRegistered", mock.AnythingOfType("ClientData"))
724-
mockListener.On("OnCount", feature, true).Return()
724+
mockListener.On("OnCount", feature, true).Maybe()
725725
mockListener.On("OnError").Return()
726726

727727
client, err := NewClient(
@@ -839,7 +839,7 @@ func TestClient_VariantShouldFailWhenSegmentConstraintsDontMatch(t *testing.T) {
839839

840840
mockListener := &MockedListener{}
841841
mockListener.On("OnReady").Return()
842-
mockListener.On("OnCount", mock.AnythingOfType("string"), mock.AnythingOfType("bool")).Return()
842+
mockListener.On("OnCount", mock.AnythingOfType("string"), mock.AnythingOfType("bool")).Maybe()
843843
mockListener.On("OnRegistered", mock.AnythingOfType("ClientData"))
844844
mockListener.On("OnError").Return()
845845

@@ -942,7 +942,7 @@ func TestClient_ShouldFavorStrategyVariantOverFeatureVariant(t *testing.T) {
942942

943943
mockListener := &MockedListener{}
944944
mockListener.On("OnReady").Return()
945-
mockListener.On("OnCount", mock.AnythingOfType("string"), mock.AnythingOfType("bool")).Return()
945+
mockListener.On("OnCount", mock.AnythingOfType("string"), mock.AnythingOfType("bool")).Maybe()
946946
mockListener.On("OnRegistered", mock.AnythingOfType("ClientData"))
947947
mockListener.On("OnError", mock.AnythingOfType("*errors.errorString"))
948948

@@ -1042,7 +1042,7 @@ func TestClient_ShouldReturnOldVariantForNonMatchingStrategyVariant(t *testing.T
10421042

10431043
mockListener := &MockedListener{}
10441044
mockListener.On("OnReady").Return()
1045-
mockListener.On("OnCount", mock.AnythingOfType("string"), mock.AnythingOfType("bool")).Return()
1045+
mockListener.On("OnCount", mock.AnythingOfType("string"), mock.AnythingOfType("bool")).Maybe()
10461046
mockListener.On("OnRegistered", mock.AnythingOfType("ClientData"))
10471047
mockListener.On("OnError", mock.AnythingOfType("*errors.errorString"))
10481048

@@ -1106,7 +1106,7 @@ func TestClient_VariantFromEnabledFeatureWithNoVariants(t *testing.T) {
11061106
mockListener := &MockedListener{}
11071107
mockListener.On("OnReady").Return()
11081108
mockListener.On("OnRegistered", mock.AnythingOfType("ClientData"))
1109-
mockListener.On("OnCount", feature, true).Return()
1109+
mockListener.On("OnCount", feature, true).Maybe()
11101110
mockListener.On("OnError").Return()
11111111

11121112
client, err := NewClient(

‎feature_state.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,12 +81,20 @@ func (s *FeatureMemoryState) evaluateFeature(
8181
}, fmt.Errorf("invalid groupId type for feature %s", f.Name)
8282
}
8383

84+
var stickiness *string
85+
86+
if v, ok := stCfg.Parameters[strategy.ParamStickiness]; ok {
87+
if s, ok := v.(string); ok {
88+
stickiness = &s
89+
}
90+
}
91+
8492
return api.StrategyResult{
8593
Enabled: true,
8694
Variant: api.VariantCollection{
8795
GroupId: groupId,
8896
Variants: stCfg.Variants,
89-
}.GetVariant(ctx),
97+
}.GetVariant(ctx, stickiness),
9098
}, nil
9199
}
92100

‎impression_test.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func TestImpression_Off(t *testing.T) {
5858
mockListener := &MockedListener{}
5959
mockListener.On("OnReady").Return()
6060
mockListener.On("OnRegistered", mock.AnythingOfType("ClientData"))
61-
mockListener.On("OnCount", feature, true).Return()
61+
mockListener.On("OnCount", feature, true).Maybe()
6262
mockListener.On("OnImpression", mock.Anything).Maybe()
6363

6464
client := setupClient(t, mockListener)
@@ -107,7 +107,7 @@ func TestImpression_IsEnabled(t *testing.T) {
107107
mockListener := &MockedListener{}
108108
mockListener.On("OnReady").Return()
109109
mockListener.On("OnRegistered", mock.AnythingOfType("ClientData"))
110-
mockListener.On("OnCount", feature, true).Return()
110+
mockListener.On("OnCount", feature, true).Maybe()
111111

112112
mockListener.On("OnImpression", mock.MatchedBy(func(e ImpressionEvent) bool {
113113
return e.FeatureName == feature &&
@@ -178,7 +178,7 @@ func TestImpression_GetVariant(t *testing.T) {
178178
mockListener := &MockedListener{}
179179
mockListener.On("OnReady").Return()
180180
mockListener.On("OnRegistered", mock.AnythingOfType("ClientData"))
181-
mockListener.On("OnCount", feature, true).Return()
181+
mockListener.On("OnCount", feature, true).Maybe()
182182

183183
mockListener.On("OnImpression", mock.MatchedBy(func(e ImpressionEvent) bool {
184184
return e.FeatureName == feature &&
@@ -247,7 +247,7 @@ func TestImpression_WithContext(t *testing.T) {
247247
mockListener := &MockedListener{}
248248
mockListener.On("OnReady").Return()
249249
mockListener.On("OnRegistered", mock.AnythingOfType("ClientData"))
250-
mockListener.On("OnCount", feature, true).Return()
250+
mockListener.On("OnCount", feature, true).Maybe()
251251

252252
userCtx := context.Context{
253253
UserId: ctxUserId,
@@ -411,7 +411,7 @@ func TestImpression_GetChannelMethod(t *testing.T) {
411411
mockListener := &MockedListener{}
412412
mockListener.On("OnReady").Return()
413413
mockListener.On("OnRegistered", mock.AnythingOfType("ClientData"))
414-
mockListener.On("OnCount", feature, true).Return()
414+
mockListener.On("OnCount", feature, true).Maybe()
415415
mockListener.On("OnImpression", mock.AnythingOfType("ImpressionEvent")).Maybe()
416416
mockListener.On("OnError", mock.Anything).Maybe()
417417

‎internal/strategies/flexible_rollout.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,9 @@ func (s flexibleRolloutStrategy) Name() string {
3131
func (s flexibleRolloutStrategy) resolveStickiness(st stickiness, ctx context.Context) string {
3232
switch st {
3333
case defaultStickiness:
34-
return coalesce(ctx.UserId, ctx.SessionId, s.random.string())
34+
return coalesce(ctx.UserId, ctx.SessionId, randomString())
3535
case randomStickiness:
36-
return s.random.string()
36+
return randomString()
3737
default:
3838
return ctx.Field(string(st))
3939
}

‎internal/strategies/helpers.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,3 +89,17 @@ func (r *rng) string() string {
8989
func newRng() *rng {
9090
return &rng{random: rand.New(rand.NewPCG(uint64(time.Now().UnixNano()), uint64(os.Getpid())))}
9191
}
92+
93+
var rngPool = sync.Pool{
94+
New: func() any {
95+
return rand.New(rand.NewPCG(uint64(time.Now().UnixNano()), uint64(os.Getpid())))
96+
},
97+
}
98+
99+
func randomString() string {
100+
r := rngPool.Get().(*rand.Rand)
101+
n := r.IntN(10000) + 1
102+
rngPool.Put(r)
103+
104+
return strconv.Itoa(n)
105+
}

0 commit comments

Comments
 (0)