Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update from 8.3.1 to 9.1.0 changes variant on existing features flags #232

Closed
hcgoranson opened this issue Nov 30, 2023 · 7 comments
Closed
Labels

Comments

@hcgoranson
Copy link

hcgoranson commented Nov 30, 2023

Describe the bug

After updating the Unleash-client-java from 8.3.1 to 9.1.0 we observed that the variant suddenly started to change for the variant of the gradual rollout strategy for existing feature flags.

Example: let's say a user has a feature flag enabled at version 8.3.1 and the variant set to Test. After the update to 9.1.0 the variant is set to Baseline and hence the feature flag doesn't work as expected anymore.

My best guess of the root cause of this is that the fix: change seed for variantutils to ensure fair distribution caused it since it changed the seed of the hash calculation in the getNormalizedNumber method from 0 to 86028157 and hence change the target so the variant might be different.

Steps to reproduce the bug

  1. Create a feature new feature flag with Gradual rollout strategy and a 50/50 split variant Baseline/Test.
  2. Use an application with the Unleash-java-client version v8.3.1 to fetch the flag and observe which variant, so Baseline or Test
  3. Do step 2) at least 15 times for different users (the rest of the context shall remain the same)
  4. Update the application to v9.1.0 and fetch the flags for all users again
  5. Compare the variant form the different Unleash-java-client versions

Expected behavior

I expect the variant to be the same for an existing feature flag for a given user regardless if it is Unleash-java-client version v8.3.1 or v9.1.0. The consequence otherwise is that features can suddenly be switched on in an inconsistent state or skew the analysis of an experiment.

Logs, error output, etc.

No response

Screenshots

image image

Additional context

I think this change might break all our feature flags since the variant is recalculated

Unleash version

Unleash enterprise 5.6.0+2921.375b1898 (Pro)

Subscription type

Pro

Hosting type

Hosted by Unleash

SDK information (language and version)

Java SDK - v8.3.1 (works fine) v9.1.0 has the problem

@FredrikOseberg
Copy link
Contributor

@hcgoranson

First of all, you have my deepest apologies that this occurred. We try our best to avoid situations like this, but sometimes we make mistakes. In this case we found a latent bug in how we performed the gradual rollout hashing in combination with variants. Long story short: the initial hashing of the gradual rollout created a bias that affected the variant distribution. You can read the full story here: https://www.getunleash.io/blog/hashing-it-right-solving-a-gradual-rollout-puzzle

Unfortunately, there was no way to fix this without causing a breaking change. In order to fix the variant distribution, we were forced to make a major version update of the SDK that signified the breaking change. To keep the same distribution, there is no other path than to stay on java 8.3.1 until and upgrade when it's more convenient.

Again, we apologise immensely for the inconvenience.

@hcgoranson
Copy link
Author

hcgoranson commented Dec 6, 2023

Hi, thanks for the reply. I had a read at the blog and understand the rationale now.

Would it be possible to add switch in the java SDK so we can handle the compatibility issue on the client side our self. I'm thinking of that if we can control on our side if we use the old or the new hash when calling isEnabled. In that way we can gradually phase out the features already running with the old seed and at the same time upgrade the SDK.

We have quite a few feature flags with different lifespan and nbr of users so I'm a bit doubtful that we will have a good time to make the upgrade without too much damage.

@FredrikOseberg
Copy link
Contributor

FredrikOseberg commented Dec 6, 2023 via email

@FredrikOseberg
Copy link
Contributor

Understood. Let me raise this internally, I will get back to you as soon as I know more. ons. 6. des. 2023 kl. 07:51 skrev Hans-Christian Göranson < @.>:

Hi, thanks for the reply. I had a read at the blog and understand the rationale. Would it be possible to add switch in the java SDK so we can handle the compatibility issue on the client side our self. I'm thinking of that if we can control on our side if we use the old or the new hash when calling isEnabled. In that way we can gradually phase out the features already running with the old seed and at the same time upgrade the SDK. We have quite a few feature flags with different lifespan and nbr of users so I'm a bit doubtful that we will have a good time to make the upgrade without too much damage. — Reply to this email directly, view it on GitHub <#232 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD2WIPV7UXMN364L36ANQBDYIAIXZAVCNFSM6AAAAABABI4HAKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBSGE4TCNRSGI . You are receiving this because you commented.Message ID: @.
>

@hcgoranson

I've discussed this with the team internally and we have the following suggestion. We can introduce a new method called deprecatedGetVariant that will use the old flawed hashing algorithm but be clearly marked in the API surface as a method that will be discontinued in the future. You can use this method to keep your current experiments the same while you transition to the new version, but at some point in the future this method will be removed in a major version upgrade. Would that work for you?

@hcgoranson
Copy link
Author

That would be great.
It think we can then upgrade to a 9.x.x version and keep but gradually phase out the existing flags (hopefully not within a too long time period) and for all new flags we will use the new hashing algorithm

@FredrikOseberg
Copy link
Contributor

That would be great. It think we can then upgrade to a 9.x.x version and keep but gradually phase out the existing flags (hopefully not within a too long time period) and for all new flags we will use the new hashing algorithm

@hcgoranson We've released version 9.2.0 of the Java SDK that includes #233 restoring the old version of doing variant hashing and selection using the deprecatedGetVariant method. It will allow you to migrate to the new feature flag variant hashing over time while still keeping the old feature flags variant distribution.

We've taken on some significant technical debt to do so, so we would appreciate if you could keep us in the loop on your migration status as we would prefer to get rid of this debt as soon as possible.

Thanks!

@hcgoranson
Copy link
Author

Thank you very much, I will initiate the work on our side

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants