Add redis as storage backend

[slavabobik]

Is your feature request related to a problem? Please describe.
Hi, we actively used redis as storage, and I want to add this feature. I've found this issue #174
but it was closes because author inactivity, so can I add?

Describe the solution you'd like
I would like to add Redis as storage

[markphelps]

@slavabobik yes definitely would love any help adding more supported backends! Redis seems like a good candidate, it would be the first non-SQL backend though so that might be challenging, but I am looking forward to seeing your implementation

[markphelps]

I also re-opened #174 because I still think that would be nice to have as well

[slavabobik]

Thank you 🙏 could you please describe in brief(or may be you have article/wiki) flipt architecture? Because Redis first non-SQL backend, I would like to discuss about design first

[markphelps]

Sure I'll try to write something up this afternoon.

I also need to do a bit of package renaming/cleanup (like #634) since I realize now that the code is mostly coupled to the backend being a SQL backend

[slavabobik]

Thanks, should I wait for your changes or I can begin in parallel?

[markphelps]

Feel free to begin whenever. I merged #364 yesterday.

Here is a quick overview of the storage architecture:

Current Storage Structure

Overview

To support a new storage backend, we'll need to implement the Store interface.

It is a combination of 4 other interfaces: EvaluationStore, FlagStore, RuleStore, and SegmentStore.

The purpose of the storage layer is simply to store/retrieve the data. The actual evaluation actually occurs at the server layer for example.

Validation

That said, validation of the data is somewhat mixed unfortunately between the 'server' layer and the 'storage' layer.

For example, there is a validation.go file that contains all of the validation for all of the *Request entities. This enforces non-empty values, etc so we won't have to worry about that for a new store as it happens 'above' the storage layer.

BUT we do currently rely on the storage layer for "NotFound" type errors when querying data, such as this.

Code Layout

├── storage
│   ├── cache
│   ├── sql
│   │   ├── common
│   │   ├── mysql
│   │   ├── postgres
│   │   └── sqlite

The current storage layer can be broken down as follows:

• storage - contains the Store interface mentioned above as well as the sql and caching packages.
• storage/cache - Currently the only cache available is in memory, but maybe this would be a good place to add in Redis first without actually making Redis the full backend store? It uses the github.com/patrickmn/go-cache lib which I believe has a Redis implementation?
• storage/sql - contains all code related to SQL, including a common package which contains SQL code that can be shared between Postgres, MySQL and SQLite. It also contains things such as metrics and the migrator code which is responsible for migrating the DB when there are schema changes (something the Redis layer won't have to worry about).
• storage/sql/common - described above
• storage/sql/mysql - contains ONLY the MySQL specific code to handle errors for specific MySQL error codes such as unique constraint violations or foreign key constraint errors.
• storage/sql/postgres - same as above but for Postgres.
• storage/sql/sqlite - same as above but for SQLite.

Required Redis Changes

Store

This structure will likely be different for creating a new Redis storage backend. I can see simply a storage/redis package that contains code to implement the above Store interface. This will live alongside the existing storage/sql package.

We'll also need to make sure that the Redis layer handles things like the SQL layer does like invalid relationships, unique constraints etc, which will probably depend on how the data is actually stored in Redis.

Config

The final area I think that will need change is to introduce a way to configure Redis within the application (like connection strings, timeouts, etc). This will likely require changes to the Config struct which handles unmarshalling configuration data from YAML. User-facing docs can be found here.

We'll likely need to introduce a new 'top level' key such as Storage to replace Database, and then allow new config values for redis. I can handle updating this config structure and YAML once we have a working example of the new Redis storage layer.

Sorry, I know this is a lot of info, I just wanted to get as much as I could out of my head and into this issue. Feel free to ask more questions on this issue, or I can turn this into a discussion or document any more things that aren't clear.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[ZombieMInd]

I'm trying to make Redis cache implementation, which @markphelps mentioned, but I run into a problem with conversion interface from cacher get method. It asserts interface from get to concrete model type, but it doesn't work with Redis implementation with json encoding, as json unmarshals in map[string]interface{}

I assume to move unmarshalling to a higher level, i.e to cached store implementation such as GetFlag
Or I can try to change the encoding package in Redis cache implementation

[markphelps]

@ZombieMInd thanks for taking this on!

It might require a bit of cleanup in the cache package first to make implementing the Redis cache easier. I made an attempt in #825 . I'll likely merge this if I can finish/answer the TODOs in that PR.

I also tried a quick spike on implementing a Redis cache here based on #825. It uses the https://github.com/go-redis/cache package. I havent actually tried using it with a real Redis impl, and theres still plenty of work to do, but I think shows an example of how implementing the cache.Cacher interface could work for Redis.

Theres also some modifications we could make to the cache.Cacher interface to make it more applicable to external caches like:

1. Taking a context.Context parameter for all methods
2. Returning error where applicable
3. Likely getting rid of the Flush method as this will probably be difficult for distributed caches to implement

[ZombieMInd]

Thanks for the reply!
I absolutely agree with those improvements, it would be really great! Maybe I can help you somehow with those tasks?
I have pretty much the same implementation as this with the same package, but here it begs to add context and errors

[markphelps]

Sorry for the delay @ZombieMInd.

I should have the Redis PR done by end of the week. I would really love your help testing if possible. I can create a snapshot release for you to try that has the Redis implementation once available.

Or if you are able to get your PR up before I do we can go off yours.

[markphelps]

@ZombieMInd I still haven't forgotten about this. I am making steady progress in my limited spare time. I chose to go with a different approach by just caching the response from the server layer instead of caching at the storage layer and then going through the computation for evaluation each time.

My branch that I'm working on is wip-redis. I plan to devote some more time to it this week. Thanks for your patience!

[markphelps]

@ZombieMInd I know its been forever but I finally have a PR up #954 !! Would love it if you are able to take a look as well

[ZombieMInd]

@ZombieMInd Sorry for my disappearing. You did great work! I love it! Looking forward to new features in this project

[markphelps]

Thanks @ZombieMInd and no worries. v1.10.0 is out with Redis cache support. https://flipt.io/docs/configuration#cache

LMK what you think!