Fixed EvaluatorBucketing Index Selection Bug #227
Conversation
|
We are currently encountering this bug in production with LaunchDarkly, whereby we have the following rollout variations:
We have had one instance of a user being allocated to rollout variation |
|
What is the purpose of the zero-weight buckets? |
They represent enum-backed in-code options. Currently we do not want to allocate users to The primary issue is that, when non-zero options are present, you should never expect a 0% option to be selected, and falling back on the last non-zero entry rather than the last bucket entry solves that. |
|
The rationale sounds reasonable, but we're going to have to look at all of the other SDK implementations— the evaluation logic is kept in sync between them, which is necessary in order to ensure that different apps connecting to the same environment get the same answers, so if we're going to treat this as a bug fix it'll have to be rolled out across all of them rather than only here. |
EvaluatorBucketing#variationIndexForUser can currently return a zero-weighted index even when non-zero weighted indices exist under the following circumstances: 1) The user's bucket evaluates to something larger than the weighted sum of all buckets. This can happen --always-- if the user's bucket is (by sheer luck) naturally 1.0, or if the sum of weights rounds in unfortunate ways. 2) There is a trailing zero-weighted bucket. Normally, the trailing zero-weighted bucket should never be selected, however because the code defaults to using the last bucket when 1) is true, the zero-weighted ends up being used. This change modifies variationIndexForUser to pick the last non-zero-weighted bucket instead.
becassel
force-pushed
the
bcassell/2021-02-23-fix-evaluator-bucketing
branch
from
61e0773 to
e1053ee
Compare
Yep no problem! I just pushed a quick refresh, realized I left two unused imports in by accident. |
(#6) implement data source status monitoring
|
Sorry this was left dangling - I'm closing it now because I believe it became obsolete a while ago due to a change in the LD service endpoints. It should no longer be possible for an SDK to receive flag data that contains a zero-weight bucket - the LD service should strip out any such buckets automatically when sending the data, so that even though they are still defined in the flag configuration on your dashboard, the SDK will not see them. |
No problem, good to hear it's been essentially fixed! |
Requirements
Describe the solution you've provided
This change modifies
variationIndexForUserto pick the last non-zero-weighted bucket instead of the last bucket.Describe alternatives you've considered
Not fixing the bug. That seems like a bad idea.
Additional context
EvaluatorBucketing#variationIndexForUsercan currently return a zero-weighted index even when non-zero weighted indices exist under the following circumstances:Normally, the trailing zero-weighted bucket should never be selected, however because the code defaults to using the last bucket when 1. is true, the zero-weighted bucket ends up being used.