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

feat(flags): finer grained concurrency control #29328

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

haacked
Copy link
Contributor

@haacked haacked commented Feb 27, 2025

Problem

fine-grained-concurrency-demo.mp4

Follow up to: #29268

#29268 introduces concurrency controls to feature flag editing. If another user saves a change, any change, while you're editing, we block your save from stomping over their changes.

However, the implementation to do that was very broad. For example, if two people change different fields, it would block. It would block even if a user made no changes, but clicked the save button as we'd save it and increment the version (perhaps we should fix that too, but I leave it as an exercise for the reader).

Changes

Now, when we detect a possible conflict, we look deeper and compare what fields have changed. If the current save attempts to overwrite any fields that have changed while they were editing, we block the save. If there are no conflicts, for example, the current user changes the description while another user changed the filters, then we allow the save to go through.

CAVEAT: This PR doesn't do anything fancy in regards to filters. So if one user edits the variants, while another edits the conditions, we will block because they get saved to the same filters JSON blob. I think this is satisfactory for now.

Does this work well for both Cloud and self-hosted?

Yes

How did you test this code?

Manual testing and also unit tests. See the demo video.

Test Plan

  • Test a scheduled change. Make sure it does not fail. It is allowed to stomp on any other changes.
  • Edit the same feature flag in two browsers: Change description in one, a release condition in the other. expected: No conflict.
  • Edit the same feature flag in two browsers: Change description in one, and description in the other. expected: conflict.

@haacked haacked requested a review from a team February 27, 2025 22:51
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

This PR implements finer-grained concurrency control for feature flags, allowing simultaneous edits to different fields without conflicts.

  • Added _get_conflicting_changes method in feature_flag.py that compares specific fields between original flag and current database state
  • Modified update process to only block saves when the same fields were changed by different users
  • Added originalFeatureFlag reducer in featureFlagLogic.ts to track the initial state for accurate conflict detection
  • Added cleanFlag helper function to remove metadata fields before comparison
  • Added comprehensive test cases in test_feature_flag.py to verify concurrent edits to different fields succeed

3 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile

Copy link
Contributor

github-actions bot commented Feb 27, 2025

Size Change: 0 B

Total Size: 9.73 MB

ℹ️ View Unchanged
Filename Size
frontend/dist/toolbar.js 9.73 MB

compressed-size-action

@haacked haacked force-pushed the haacked/improved-concurrency branch from 0566700 to 06c8f2c Compare February 28, 2025 04:36
haacked and others added 3 commits February 27, 2025 20:43
Allows for updates where the fields the current user is changing haven't been changed by another user, even if the flag has been changed.
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@haacked haacked force-pushed the haacked/improved-concurrency branch from 06c8f2c to d79b4c4 Compare February 28, 2025 04:43
While editing an experiment, you can edit release conditions by opening a modal. We want that modal to have the latest conditions in case someone else is editing the flag conditions at the same time.
Also enforces concurrency when saving the rollout conditions for the feature flag.
@haacked
Copy link
Contributor Author

haacked commented Feb 28, 2025

Demo of concurrency support when editing rollout conditions for an experiment.

experiment-editor.mp4

@haacked
Copy link
Contributor Author

haacked commented Feb 28, 2025

Early access feature
As far as I can tell, you can't directly edit a feature flag from Early access feature. Instead, it links to the connected flag:

image

Surveys

I believe there is a way to connect surveys to feature flags, but I didn't figure it out from the UI. Looking at the code, it uses FeatureFlagReleaseConditions so I think it'll support my changes. I'll ask around.

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

Successfully merging this pull request may close these issues.

1 participant