Skip to content

Commit 16f1e79

Browse files
author
Jade Wang
committed
fix(csharp): apply cached feature flags on connect (warm cache)
PR #500 changed the connection-open feature-flag merge to fire-and-forget: `MergeWithEnvironmentConfigAndFeatureFlags` kicked off the async merge in a discarded `Task.Run` and returned the env-config-only properties, so server feature flags (e.g. `adbc.databricks.enable_fast_metadata_query`) were never applied to any connection. The synchronous applier existed but had no callers, so neither the first nor any subsequent connection ever received the flags. Add `FeatureFlagCache.TryMergeWarmFeatureFlags`: if the per-host cache is warm, merge the cached flags synchronously (local properties win); if cold, kick off a background warm-up and return null so `Connect()` proceeds unblocked and a subsequent connection to the same host picks up the flags. Never performs a blocking network fetch, and honors the negative-cache backoff. Co-authored-by: Isaac
1 parent 280b1bb commit 16f1e79

2 files changed

Lines changed: 59 additions & 9 deletions

File tree

csharp/src/DatabricksDatabase.cs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -162,22 +162,22 @@ public override AdbcConnection Connect(IReadOnlyDictionary<string, string>? opti
162162
}
163163

164164
/// <summary>
165-
/// Merges properties with environment config and fires a background feature flag warm-up.
165+
/// Merges properties with environment config and server feature flags.
166166
/// This is the single place where all property merging happens for both Thrift and REST connections.
167167
/// </summary>
168168
/// <param name="properties">Properties to merge.</param>
169-
/// <returns>Merged properties dictionary (feature flags applied on subsequent connections once cached).</returns>
169+
/// <returns>Merged properties dictionary with any warm-cache feature flags applied.</returns>
170170
private static IReadOnlyDictionary<string, string> MergeWithEnvironmentConfigAndFeatureFlags(IReadOnlyDictionary<string, string> properties)
171171
{
172172
var mergedWithEnvConfig = MergeWithDefaultEnvironmentConfig(properties);
173173

174-
// Fire-and-forget: warm the feature flag cache in the background.
175-
// On a cache hit the task completes immediately; on a miss it fetches and caches
176-
// so subsequent Connect() calls get flags applied synchronously (cache hit path).
177-
_ = Task.Run(() => FeatureFlagCache.GetInstance()
178-
.MergePropertiesWithFeatureFlagsAsync(mergedWithEnvConfig, s_assemblyVersion));
179-
180-
return mergedWithEnvConfig;
174+
// Apply server feature flags without ever blocking Connect(): if the per-host cache
175+
// is already warm, the flags are merged in synchronously (no network, local props win);
176+
// if it is cold, a background warm-up is kicked off so a subsequent connection to the
177+
// same host picks them up, and this connection proceeds with the unmodified properties.
178+
return FeatureFlagCache.GetInstance()
179+
.TryMergeWarmFeatureFlags(mergedWithEnvConfig, s_assemblyVersion)
180+
?? mergedWithEnvConfig;
181181
}
182182

183183
/// <summary>

csharp/src/FeatureFlagCache.cs

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,56 @@ public IReadOnlyDictionary<string, string> MergePropertiesWithFeatureFlags(
412412
.GetResult();
413413
}
414414

415+
/// <summary>
416+
/// Non-blocking feature-flag application for the connection-open path.
417+
/// <para>
418+
/// If flags for the host are already cached (warm), returns <paramref name="localProperties"/>
419+
/// merged with them (local properties win) so the connection sees the server flags immediately.
420+
/// If the cache is cold, this starts a background warm-up so a subsequent connection to the same
421+
/// host applies the flags, and returns <c>null</c> to signal the caller to proceed with the
422+
/// unmodified properties for this connection.
423+
/// </para>
424+
/// <para>
425+
/// Never performs a blocking network fetch, so it never stalls <c>Connect()</c>. Returns
426+
/// <c>null</c> when the cache is disabled (<see cref="DatabricksParameters.FeatureFlagCacheEnabled"/>
427+
/// explicitly <c>false</c>) or when no host can be determined.
428+
/// </para>
429+
/// </summary>
430+
/// <param name="localProperties">The connection properties (already merged with environment config).</param>
431+
/// <param name="assemblyVersion">Driver version, used when warming the cache.</param>
432+
/// <returns>The merged properties when the cache is warm; otherwise <c>null</c>.</returns>
433+
public IReadOnlyDictionary<string, string>? TryMergeWarmFeatureFlags(
434+
IReadOnlyDictionary<string, string> localProperties,
435+
string assemblyVersion)
436+
{
437+
// Respect the enable flag (default true; only an explicit, parseable "false" disables).
438+
if (localProperties.TryGetValue(DatabricksParameters.FeatureFlagCacheEnabled, out string? enabledStr) &&
439+
bool.TryParse(enabledStr, out bool parsedEnabled) && !parsedEnabled)
440+
{
441+
return null;
442+
}
443+
444+
var host = TryGetHost(localProperties);
445+
if (string.IsNullOrEmpty(host))
446+
{
447+
return null;
448+
}
449+
450+
// Warm cache (or negatively-cached failure): apply whatever flags are cached, synchronously.
451+
// A negative entry has no flags, so this is a no-op merge and — importantly — does NOT
452+
// trigger another fetch, honoring the negative-cache backoff.
453+
if (TryGetContext(host!, out var context) && context != null)
454+
{
455+
var flags = context.GetAllFlags();
456+
return flags.Count > 0 ? MergeProperties(flags, localProperties) : localProperties;
457+
}
458+
459+
// Cold cache: warm it in the background so the next connection to this host gets flags.
460+
// This connection proceeds without them (never blocks).
461+
_ = Task.Run(() => MergePropertiesWithFeatureFlagsAsync(localProperties, assemblyVersion));
462+
return null;
463+
}
464+
415465

416466
/// <summary>
417467
/// Tries to extract the host from properties without throwing.

0 commit comments

Comments
 (0)