Add KV Direct Binding compatibility flag.

[mar-cf]

Add KV Direct Binding compatibility flag.

EW-8494

[kentonv]

Why does this need a compatibility flag? We expect this change to be fully backwards-compatible, don't we? If you want to limit it to apply only to certain workers during testing, could the config service simply special-case a certain account / worker name for now?

I am wary of proliferating compatibility flags as a new tool for testing internal features that aren't actually breaking changes.

[kentonv]

I am wary of proliferating compatibility flags as a new tool for testing internal features that aren't actually breaking changes.

To elaborate a little bit, there are broadly two use cases where we've used compat flags so far:

1. Behavior changes that need to be controllable by the user, i.e. changes that are actually backwards-incompatible and so need to be able to be turned on/off by users. The behavior might not be in the runtime itself -- it may be a control plane change -- but it's still convenient to control using compat flags so that we have a unified public-facing API for such things. This is the intended use case for compat flags.
2. Experimental changes to runtime behavior, even if not backwards-compatible, flagged only for early testing purposes. Here, users will never use the flag; only people developing on the runtime itself will. This is not really the intended use case for compat flags, but it's convenient, because the compat flag framework is easy to access throughout the runtime.

Here you are introducing a flag under the second use case, except that it's not controlling runtime behavior. The feature is implemented in the control plane. We've never used compat flags for such things before. Is there a good reason why other methods aren't a good fit here?

[aplunk]

I am wary of proliferating compatibility flags as a new tool for testing internal features that aren't actually breaking changes.

To elaborate a little bit, there are broadly two use cases where we've used compat flags so far:

1. Behavior changes that need to be controllable by the user, i.e. changes that are actually backwards-incompatible and so need to be able to be turned on/off by users. The behavior might not be in the runtime itself -- it may be a control plane change -- but it's still convenient to control using compat flags so that we have a unified public-facing API for such things. This is the intended use case for compat flags.
2. Experimental changes to runtime behavior, even if not backwards-compatible, flagged only for early testing purposes. Here, users will never use the flag; only people developing on the runtime itself will. This is not really the intended use case for compat flags, but it's convenient, because the compat flag framework is easy to access throughout the runtime.

Here you are introducing a flag under the second use case, except that it's not controlling runtime behavior. The feature is implemented in the control plane. We've never used compat flags for such things before. Is there a good reason why other methods aren't a good fit here?

The idea was that the runtime would optionally render kv tunnels as subpipelines for accounts that opted into the new behavior for testing. If compat flags aren't the right approach, then how can we release this feature for a subset of accounts?

The end goal is to remove the flag entirely and the new behavior will be default for all accounts.

[mar-cf]

I am wary of proliferating compatibility flags as a new tool for testing internal features that aren't actually breaking changes.

To elaborate a little bit, there are broadly two use cases where we've used compat flags so far:

1. Behavior changes that need to be controllable by the user, i.e. changes that are actually backwards-incompatible and so need to be able to be turned on/off by users. The behavior might not be in the runtime itself -- it may be a control plane change -- but it's still convenient to control using compat flags so that we have a unified public-facing API for such things. This is the intended use case for compat flags.
2. Experimental changes to runtime behavior, even if not backwards-compatible, flagged only for early testing purposes. Here, users will never use the flag; only people developing on the runtime itself will. This is not really the intended use case for compat flags, but it's convenient, because the compat flag framework is easy to access throughout the runtime.

Here you are introducing a flag under the second use case, except that it's not controlling runtime behavior. The feature is implemented in the control plane. We've never used compat flags for such things before. Is there a good reason why other methods aren't a good fit here?

Would rewriting a pipeline definition be considered a runtime or control plane change?

If I'm understanding correctly, the alternative you're suggesting to using a feature flag is hardcoding a customer/worker allowlist in code? If so, I think the benefit to using feature flags would be a clean separation of code and configuration. Enabling a feature rollout should be independent from the binary release process.

[a-robinson]

The feature is implemented in the control plane.

Does the runtime on-the-fly rewriting pipelines containing KV bindings count as control plane? I wouldn't have thought of it that way, but I guess so.

Is there a good reason why other methods aren't a good fit here?

Eh, not really. We could always stick an allowlist into the process config file instead if we want to avoid creating additional compat flags.

[mar-cf]

@kflansburg thoughts?

[kentonv]

Oh, is this a rewrite that occurs in the runtime, not in the config service? I guess that could change my mind...

[kentonv]

The end goal is to remove the flag entirely and the new behavior will be default for all accounts.

Well, the issue is that we actually cannot ever remove a compat flag completely. We can only mark it obsolete. It's a little sad to fill up this file with obsolete flags. Admittedly the cost in production is pretty mild (it's just a bit).

[mar-cf]

The end goal is to remove the flag entirely and the new behavior will be default for all accounts.

Well, the issue is that we actually cannot ever remove a compat flag completely. We can only mark it obsolete. It's a little sad to fill up this file with obsolete flags. Admittedly the cost in production is pretty mild (it's just a bit).

What prevents us from removing obsolete flags?

[kentonv]

What prevents us from removing obsolete flags?

In production the flags are encoded into a bitfield. Once a flag is defined, someone might deploy workers that use it. Even if you mark the flag obsolete, those workers may continue to exist in production forever, so you can't ever reuse the bit to mean something else, because the system will incorrectly think those old workers want the new thing.

[mar-cf]

@kentonv @mikea
Is the latest plan to move forward with this change, support compat flags in pipeline and update EWC to set the value?

Are there any changes requested?

[kentonv]

Yeah, on further discussion I think this makes sense after all and am happy to approve it.

However, there's a procedural issue at the moment: #2432 reverted a change that introduced compat flag @53, which now makes this PR in conflict. But we need to re-introduce flag 53 as it was defined before, rather than replace it with your flag -- it's possible someone deployed with that flag while it was defined, and we wouldn't want such workers to be interpreted as having your flag instead.

I asked @AdityaAtulTewari to make a PR re-introducing the compat flag alone, but you could also just re-add it in your PR (so your PR restores flag @53 as cache_option_enabled, and then adds your flag still as @54).

[kentonv]

#2435 restores the @53 flag.

[mar-cf]

I created a commit to do that here. Thanks for taking care of that. I'll remove the commit to readd.