airbytehq / airbyte

The leading data integration platform for ETL / ELT data pipelines from APIs, databases & files to data warehouses, data lakes & data lakehouses. Both self-hosted and Cloud-hosted.
https://airbyte.com
Other
15.55k stars 4.01k forks source link

Fail syncs if LaunchDarkly has not been properly initialized #26498

Closed pedroslopez closed 7 months ago

pedroslopez commented 1 year ago

What

When using LaunchDarkly as the feature flag client, the current behavior if the LaunchDarkly service happens to be down is the following:

  1. If LaunchDarkly is down upon airbyte startup, before any feature flags have been loaded, then default values for all feature flags will be returned.
  2. If LaunchDarkly goes down after services are started up and an initial set of flags has been loaded, the last known value of the flags will be returned.

Using latest known values is fine (2), but the failure mode for (1) is problematic, specially as we get into using LaunchDarkly for overriding connector versions. Reverting to default values can cause problems in the case of going back (or forwards) across major breaking versions of connectors, and can cause unintended consequences. We will also soon have users that are always locked to specific versions of connectors.

We need to guarantee that we won't unintentionally start running a different version of a connector. Following our product principles and that no data is better than wrong data, we should fail syncs if the feature flag client has not been initialized.

Alternatives considered

A more long-term solution that doesn't involve failing syncs would be to utilize LaunchDarkly's persistent data stores feature, which would store the flags in something like Redis that we control. See https://github.com/airbytehq/airbyte-internal-issues/issues/4413 to track this.

evantahler commented 1 year ago

It's been a while now... so... I'm moving this to the icebox

gosusnp commented 7 months ago

This feels like a symptom of using launch darkly for the wrong purpose. I'd rather avoid a situation where we cannot run syncs during a launch darkly outage. In most cases, using the default should still give us a correct behavior. Flags where we require a consistent uptime should be moved to our internal configuration rather than enforcing this type of dependency.

pedroslopez commented 7 months ago

Agree!

This is not necessary as we move connector version pinning to our database

evantahler commented 7 months ago

Let's close this then!