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

V2 Adapters #163

Open
2 tasks
jnunemaker opened this issue Sep 29, 2016 · 5 comments
Open
2 tasks

V2 Adapters #163

jnunemaker opened this issue Sep 29, 2016 · 5 comments
Assignees
Labels

Comments

@jnunemaker
Copy link
Collaborator

jnunemaker commented Sep 29, 2016

Tasks

  • Storage#get_multi needs to be efficient; need to add get_multi to v2 adapters and use it in Storage
  • Add a migration path

Original Write Up

While thinking about or working on description for features, custom gates for features and runtime controls, it became apparent that the current adapter style of a method per thing we want to store isn't going to scale to more functionality well.

The Beginning

Initially when I started with adapters for flipper (way back in the day), the interface was more key/value based (get, set, delete). All the information for a feature was stored in one key (other rollout tools do similar).

The Benefits of Key/Value

  • flexibility - we can store a feature in one key and serialize whatever data makes sense for the feature, e. g. a description for the feature, a collection of disabled gates, or even use the key/value style for new features (think runtime controls). Several new features (just listed and also listed above) are kind of stuck because of the amount of work needed not having this flexibility is annoying.
  • simplicity - Making an adapter becomes dramatically easier. All adapter creators would need is get, set and delete.

The Problem (read then write race)

The main problem with key/value and storing all information in a feature (to make reads faster) is that without some form of locking, race conditions exist. Imagine the following scenario:

  • developer 1 views "profile_next" feature
  • developer 2 views "profile_next" feature
  • developer 1 adds "fred" to enabled actors and developer 2 removes "barney" from enabled actors at the same time, flipper does a get for the feature, modifies the actor set and then does a set for the feature
  • whichever set happens last nullifies the other developers change, this would be frustrating/confusing to not see a change you just made that appeared to be successful

A Solution (to read then write race)

The solution is to either lock around the read/write operation or use datastore functionality that allows adding or removing a thing from a set (sadd/srem in redis, insert/delete in mysql with unique index, etc.). For this reason, I went with making it so adapters could take advantage of data store functionality to avoid this read then write race. That is why adapters have an enable and disable method, to allow for adapter creators to customize the operation of enabling or disabling something to be atomic for the backend they are adapting to.

The Reality

The reality is that features are enabled and disabled very rarely compared to how often they are checked. Write throughput (enable/disable) is usually so low that the chance of this race happening is extremely low.

For times where flipper is used with lots of writes are happening (thus making the race more likely), it would be pretty simple to use some form of locking (optimistic/pessimistic) to get around the issue. I really don't think it will be a problem in practice though. If it becomes a problem, flipper could even provide a lock adapter that does nothing by default (simplest/fastest), but allows users to customize for their needs.

The Future

Since key/value has a big upside and, in reality, very little downside, my goal is to move to key/value for adapters. A ton of people rely on flipper to control functionality on their sites, so my top goals are:

  • safety - flipper should support both adapter versions for a period of time, v1 is the current adapter interface and it will be deprecated, v2 is the new adapter interface and all new functionality will be built on it
  • performance - flipper should perform the same or better with the new adapter interface
  • migrateability - it should be extremely easy to upgrade flipper from v1 to v2. My current plan is to provide a bridge adapter that can do a one time migration of all data (ensure no one is changing features, run script, profit) or on the fly (something like read from v2, fallback to v1, migrate v1 data to v2 format, and then delete v1)

Status

I've got a branch started that is getting pretty close (already supports v1 and v2 and should perform the same, needs migration path still) and a project where you can track the status. Currently the project just has a few notes, but I'll try to flesh it out soon.

@jnunemaker jnunemaker self-assigned this Sep 29, 2016
@AlexWheeler
Copy link
Collaborator

AlexWheeler commented Sep 29, 2016

this sounds really cool. So just to make sure I'm understanding this - if my team, for example, wants a description field on features then we'd add a column to our features table (i.e. flipper_features) and then just flipper[feature].set("description", "some description")

Curious how this would tie in with the UI.

My team adds
flippper[feature].set("description", "foo")
but the UI expects a "desc" field.

# i.e. ui.erb
features.each do |feature|
  <h1><%= feature.desc %></h1>
end

Am I thinking about this right?

@jnunemaker
Copy link
Collaborator Author

@AlexWheeler I don't think I'm picturing quite that, for the reasons you mention. Standardizing is important or how will all the tools built on flipper know what to do. What I'm picturing is that this opens up the internal storage, so we can say, "hey we want to allow people to store a description" and it just works. We add description to the UI/API/Feature and it just gets serialized with the other Feature meta data like the gates and is available to any tools on top of Flipper.

The get/set/delete would be at the adapter level, not the feature level. Check out the v2 adapter docs or the docs for one of the datastores, like redis, which shows the difference between setting up a v1 or v2 adapter. Flipper itself and the Feature code stay the same. Does that clarify?

@AlexWheeler
Copy link
Collaborator

totally clarifies everything thanks

@st33b
Copy link

st33b commented Apr 7, 2017

Hi there! Love the ideas you've put forth for v2. I'm curious to see how close you've gotten to other folks being able to try it out.

@jnunemaker
Copy link
Collaborator Author

@st33b I'm really torn on v2. At first I was really excited, but I'm starting to think new adapter methods instead of even more dumbed down adapters isn't that bad of an idea. I would just keep rolling with current flipper as I may try implementing a few of the features that were blocked by v2 with v1 to see how they turn out.

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