Spike: investigate how to resolve consistency issues when using REPLICA_DATABASE_URLS

[matthewelwell]

See parent ticket for context.

The goal of this spike is to come up with a solution in the ORM that allows us to achieve some kind of explicit selection of the databases that should be used for a given queryset, or set of querysets (e.g. with a context manager).

[emyller]

Given the context (thanks), ideally we should find a solution that also works for on-premise setups regardless of database platform.

Contributing to the context, here's the sequence of requests the frontend makes. The GET request to fetch the updated feature flag will, sometimes, fetch an outdated state from a replica.

Safest options to work around replica lag within the ORM realm are:

1. Modify the router to redirect reads to the writer database only for a given set of models.
   Though I think it's not the optimal solution given the possibly high activity of models such as FeatureState, pointed out as the motivator for this spike. Also it removes, by default, the usefulness of replication for the associated table.
2. Resort to .using("default") in scenarios where it's useful to guarantee consistency.
   I like this one because it's the cheapest. There's no need to implement anything new. Also, because fetching from /api/v1/projects/1/features/1/ isn't the most common use case to fetch feature flag states, we could perhaps easily modify the request handler to use the writer database and fix this one problem.
3. Implement a context manager that wraps code around Postgres' synchronous_commit.
   This one approaches the problem at write time. In short: we can make the ORM wait for all replicas to finish updating before reporting a successful transaction commit.
   Although this one presents an elegant and tempting solution, IMO it also sounds like overkill — unless we find several other use cases that could benefit from the new machinery. It also creates yet another dependency on Postgres, and requires changing Postgres settings in order to enable using the parameter at transaction level. This article shared by @matthewelwell contains an excellent explanation and example to help if we end up taking this route.

I would experiment with option (2) above, but fallback to (3) if necessary.

Alternatively: if the frontend is the only client affected by this issue, perhaps the cleanest solution will be in the frontend. Perhaps:

1. Optimistically PUT and immediately display the new state in the UI.
2. Wait a few secs to fetch and verify the change, or revert UI state and display an error message.

I like this approach because I'm still not sure if this is a problem worth solving on the backend. Additionally, this change would improve the sense of swiftness in the UI.

[emyller]

@matthewelwell (and team) let's discuss this next week and make a decision before closing this issue, if that makes sense.

[matthewelwell]

@emyller based on your investigation, perhaps there was some context missing somewhere. While there are perhaps similar issues in the FE <-> API interactions, the real nasty part of the consistency issue actually lies within synchronous logic flows in the API where read queries are made shortly after writes. Most recently, it occurred in the logic in this module, which has a lot of the responsibility for creating new environment feature versions. The API creates the new version here, which triggers a creation of a new set of FeatureState objects associated with the version. Then, the query here tries to retrieve those feature states (using a SELECT query, which hits a replica), which fails because the data hasn't yet been replicated to the read replica DBs.

I was really hoping that there would be an option where we explicitly define where we want to use replica tables on a per queryset basis. Did you look into whether this might be possible?

My concern with the option 2 that you suggested is that it requires us to always consider it in situations where we might not necessarily know we need to consider it such as the one that I mentioned above. I would much rather that the default is such that all queries go to the primary DB, and we can explicitly distribute certain requests to the replicas.

Perhaps we can discuss this further synchronously but I wanted to list out my thoughts here first.

[emyller]

It looks like I've really missed some context. 🤔 Thanks for the example.

I was really hoping that there would be an option where we explicitly define where we want to use replica tables on a per queryset basis. Did you look into whether this might be possible?

The temptation of writing a context manager comes to mind but please help me make sure we're on the same page: we want to default all functionality to the writer instance by default, and choose code to use replicas on a need basis? If that leads to "yes", then I think Django has that built-in.

For example, you can explicitly point to a database at the manager level, similarly to how we do when selecting tasks for processing from either the default, or task_processor.

I guess we'd only need a wrapper around this utility to 1. help detect whether replicas are set up for use, and 2. select which one to use — likely moving the existing code from PrimaryReplicaRouter, which should cease to exist once we'll only use replicas explicitly.

So, here's a quick draft surfacing SDKFeatureStates:

class SDKFeatureStates(...):

    def get(self, request, identifier=None, ...):
        if identifier:
            flags = maybe_use_replica(Identity.objects).get(...).get_all_feature_states()
            return stuff
        flags = maybe_use_replica(Environment.objects).get(...).get_all_feature_states()  # Currently get_environment_flags_list
        return other_stuff

This comes with minor challenges such as other Manager instances accessed or created within those key functions (get_all_feature_states) not using the replicas, but it should be easy to address. I'm ready to write a Story if the idea makes sense!