From 99ff314e561843ab458b795b863d8f5836b9f81f Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Mon, 13 Oct 2025 14:17:57 -0700 Subject: [PATCH 1/3] Fix float32 precision bug causing bucket values to exceed 9999 for users with hash values near max uint32 --- pkg/decision/bucketer/murmurhashbucketer.go | 4 +- .../bucketer/murmurhashbucketer_test.go | 82 +++++++++++++++++++ 2 files changed, 84 insertions(+), 2 deletions(-) diff --git a/pkg/decision/bucketer/murmurhashbucketer.go b/pkg/decision/bucketer/murmurhashbucketer.go index c2667ff3..92ab095a 100644 --- a/pkg/decision/bucketer/murmurhashbucketer.go +++ b/pkg/decision/bucketer/murmurhashbucketer.go @@ -27,7 +27,7 @@ import ( "github.com/twmb/murmur3" ) -var maxHashValue = float32(math.Pow(2, 32)) +var maxHashValue = float64(math.Pow(2, 32)) // DefaultHashSeed is the hash seed to use for murmurhash const DefaultHashSeed = 1 @@ -60,7 +60,7 @@ func (b MurmurhashBucketer) Generate(bucketingKey string) int { b.logger.Error(fmt.Sprintf("Unable to generate a hash for the bucketing key=%s", bucketingKey), err) } hashCode := hasher.Sum32() - ratio := float32(hashCode) / maxHashValue + ratio := float64(hashCode) / maxHashValue return int(ratio * maxTrafficValue) } diff --git a/pkg/decision/bucketer/murmurhashbucketer_test.go b/pkg/decision/bucketer/murmurhashbucketer_test.go index 273aaa79..a75ccc4e 100644 --- a/pkg/decision/bucketer/murmurhashbucketer_test.go +++ b/pkg/decision/bucketer/murmurhashbucketer_test.go @@ -68,3 +68,85 @@ func TestGenerateBucketValue(t *testing.T) { assert.Equal(t, 2434, bucketer.Generate(bucketingKey3)) assert.Equal(t, 5439, bucketer.Generate(bucketingKey4)) } + +// TestFloat64PrecisionEdgeCase tests the fix for FSSDK-11917 +// This test ensures that users with hash values near the maximum uint32 value +// are bucketed correctly (bucket value 9999) instead of getting bucket value 10000 +// which would exclude them from the "Everyone Else" rollout rule. +func TestFloat64PrecisionEdgeCase(t *testing.T) { + bucketer := NewMurmurhashBucketer(logging.GetLogger("", "TestFloat64PrecisionEdgeCase"), DefaultHashSeed) + + // The main test: ensure all bucket values are in valid range [0, 9999] + // This catches any case where float32 precision would cause bucket value 10000 + for i := 0; i < 10000; i++ { + bucketingKey := fmt.Sprintf("user_%d_rule_12345", i) + bucket := bucketer.Generate(bucketingKey) + assert.GreaterOrEqual(t, bucket, 0, "Bucket value should be >= 0 for key %s", bucketingKey) + assert.LessOrEqual(t, bucket, 9999, "Bucket value should be <= 9999 for key %s (float32 bug would produce 10000)", bucketingKey) + } + + // Test with a bucketing key that Mark Biesheuvel identified + // The issue occurs with specific user ID + rule ID combinations + // Test a range of user IDs to find edge cases + for userID := 15580841; userID <= 15580851; userID++ { + t.Run(fmt.Sprintf("UserID_%d", userID), func(t *testing.T) { + // Test with different rule IDs that might trigger the edge case + ruleIDs := []string{"default-rollout-386456-20914452332", "rule_123", "everyone_else"} + for _, ruleID := range ruleIDs { + bucketingKey := fmt.Sprintf("%d%s", userID, ruleID) + bucket := bucketer.Generate(bucketingKey) + assert.GreaterOrEqual(t, bucket, 0, "Bucket should be >= 0") + assert.LessOrEqual(t, bucket, 9999, "Bucket should be <= 9999 (float32 would produce 10000 for some edge cases)") + } + }) + } +} + +// TestBucketingWithHighHashValues tests that users with hash values close to max uint32 +// are correctly bucketed into rollout rules +func TestBucketingWithHighHashValues(t *testing.T) { + bucketer := NewMurmurhashBucketer(logging.GetLogger("", "TestBucketingWithHighHashValues"), DefaultHashSeed) + + // Create a traffic allocation for "Everyone Else" (0-10000 range) + everyoneElseAllocation := []entities.Range{ + { + EntityID: "variation_1", + EndOfRange: 10000, + }, + } + + // Test various users to ensure they are properly bucketed + // The bucketing key needs to match the format used in actual bucketing (userID + experimentID or ruleID) + testBucketingKey := "15580846default-rollout-386456-20914452332" + bucketValue := bucketer.Generate(testBucketingKey) + + // The important test: bucket value should be in valid range [0, 9999] + assert.GreaterOrEqual(t, bucketValue, 0, "Bucket value should be >= 0") + assert.LessOrEqual(t, bucketValue, 9999, "Bucket value should be <= 9999") + + // Test that this user is bucketed into the "Everyone Else" variation + entityID := bucketer.BucketToEntity(testBucketingKey, everyoneElseAllocation) + assert.Equal(t, "variation_1", entityID, "User should be bucketed into 'Everyone Else' variation") + + // Test that the fix doesn't break normal bucketing behavior + // Create a more complex traffic allocation + complexAllocation := []entities.Range{ + { + EntityID: "", + EndOfRange: 5000, // 50% get no variation + }, + { + EntityID: "variation_a", + EndOfRange: 10000, // 50% get variation_a + }, + } + + // Test with the specific bucketing key + entityID = bucketer.BucketToEntity(testBucketingKey, complexAllocation) + // The bucket value will determine which variation is returned + if bucketValue < 5000 { + assert.Equal(t, "", entityID, "User with bucket < 5000 should get no variation") + } else { + assert.Equal(t, "variation_a", entityID, "User with bucket >= 5000 should get variation_a") + } +} From 8df5894cb6a540abe03249aef5a5afd3b224b867 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Tue, 14 Oct 2025 11:20:25 -0700 Subject: [PATCH 2/3] Fix float32 precision bug causing bucket values to exceed 9999 for users with hash values near max uint32 --- pkg/decision/bucketer/murmurhashbucketer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/decision/bucketer/murmurhashbucketer.go b/pkg/decision/bucketer/murmurhashbucketer.go index 92ab095a..ef9a9958 100644 --- a/pkg/decision/bucketer/murmurhashbucketer.go +++ b/pkg/decision/bucketer/murmurhashbucketer.go @@ -27,7 +27,7 @@ import ( "github.com/twmb/murmur3" ) -var maxHashValue = float64(math.Pow(2, 32)) +var maxHashValue = math.Pow(2, 32) // DefaultHashSeed is the hash seed to use for murmurhash const DefaultHashSeed = 1 From 6ea84ed54eb72590545925c3a5eeeb28b94f9ad3 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Tue, 21 Oct 2025 17:02:09 -0700 Subject: [PATCH 3/3] Trigger CI