feat(feature-flags): generalize driver-side cache by cathleeny · Pull Request #496 · databricks/databricks-sql-go

cathleeny · 2026-10-06T23:05:32Z

Summary

Move and generalize the existing telemetry feature-flag cache into internal/featureflags; retain all returned values and add six typed getters.
Share values by workspace ID, with normalized host as a fallback, and use authenticated transport supplied by each caller.
Honor the connector-service TTL, falling back to 15 minutes when it is missing or invalid.

Validation

Full unit suite: go test ./... passed.
Feature-flag and telemetry tests passed with the race detector.
Cold-load/workspace and stale-reader concurrency tests passed for 30 repeated race-detector runs.
The new stale-reader regression test failed before the fix and passed afterward.
go vet ./... and golangci-lint passed (0 issues).

Signed-off-by: Cathleen Yan <58714163+cathleeny@users.noreply.github.com>

github-actions · 2026-10-07T17:32:36Z

Go integration tests triggered (sea / passthrough). View workflow runs.

peco-review-bot

Verdict: 1 Low

Clean, well-tested migration of the telemetry feature-flag cache into internal/featureflags. No dangling references to the deleted telemetry/featureflag.go symbols remain in code; the Acquire/Release ref-count stays balanced (telemetry init only runs on the Thrift path, which implies !UseKernel, which guarantees a prior Acquire); and the stale-reader concurrency (whole-map replacement under lock + singleflight cold path + TryLock refresh) is race-safe. Two minor notes: (1) one Low finding on the unrelated KERNEL_REV bump; (2) telemetry/ADDING_FEATURE_FLAGS.md (not in this diff) is now stale — it instructs developers to edit the deleted featureflag.go and references removed getFeatureFlagCache()/getAllFeatureFlags()/isTelemetryEnabled APIs, so it should be rewritten for the new Cache/typed-getter surface in a follow-up.

Signed-off-by: Cathleen Yan <58714163+cathleeny@users.noreply.github.com>

github-actions · 2026-10-08T21:29:23Z

Integration test approval reset.

New commits were pushed to this PR. Label(s) integration-test-kernel were removed for security.

A maintainer must re-review and re-add a label to preview tests again. (The real gate runs in the merge queue.)

Latest commit: fe36481

peco-review-bot

Verdict: 1 Low

Looks good — a clean, well-tested refactor that generalizes the feature-flag cache and correctly moves refcount ownership to the connector/conn lifecycle (Acquire in Connect, Release on error and in Close, balanced across kernel and Reyden-recovery paths). The go.mod/depguard changes properly accompany the new singleflight import, and the copy-on-write map replacement keeps the lock-free reads race-free. One low-severity note on cold-path singleflight context sharing.

Signed-off-by: Cathleen Yan <58714163+cathleeny@users.noreply.github.com>

peco-review-bot

Verdict: 2 Low

Looks good — a clean, well-tested generalization of the telemetry flag cache into internal/featureflags. Acquire/Release reference counting is balanced across all connect/close paths (including Reyden→kernel recovery and kernel-skip), and the lock-free reads are safe because flags maps are published immutably. Two low-severity behavioral notes inline (stale-refresh context asymmetry; JSON-literal requirement for the new string getters) — neither blocks merge.

Signed-off-by: Cathleen Yan <58714163+cathleeny@users.noreply.github.com>

peco-review-bot

Verdict: 1 Low

Looks good — the feature-flags cache refactor is solid and well-tested (ref-count balance across Connect error paths and Close, cold-fetch singleflight, stale-reader non-blocking refresh, typed getters), with the depguard allowlist and go.mod direct-dep promotion correctly handled and no dangling references to removed symbols. One low-severity scope concern: an unrelated kernel C-ABI header change is bundled in.

Signed-off-by: Cathleen Yan <58714163+cathleeny@users.noreply.github.com>

peco-review-bot

✅ No issues identified by the review bot.

Signed-off-by: Cathleen Yan <58714163+cathleeny@users.noreply.github.com>

peco-review-bot

Verdict: 2 Low

Looks good — a clean, well-tested generalization of the telemetry feature-flag cache into internal/featureflags. The refcount lifecycle is correctly moved to connection ownership (balanced Acquire/Release across failed-open and Close, with a nil-guard against double release), the map-swap + singleflight concurrency design is sound and race-tested, and the depguard allowlist / go.mod direct-vs-indirect moves properly accompany the new singleflight dependency. Two low-severity notes only: a potential refresh-starvation edge under short-deadline readers, and a slightly misleading GetStringList error message.

jay-xiao446

lgtm! thanks for addressing my comments

cathleeny added 2 commits October 6, 2026 21:29

Generalize driver feature flag cache and honor server TTL

3e1f6ee

Signed-off-by: Cathleen Yan <58714163+cathleeny@users.noreply.github.com>

fix(feature-flags): keep stale readers non-blocking during refresh

ef1968b

Signed-off-by: Cathleen Yan <58714163+cathleeny@users.noreply.github.com>

cathleeny mentioned this pull request Oct 6, 2026

feat(feature-flags): generalize driver-side cache #495

Closed

cathleeny added 2 commits October 7, 2026 00:30

fix(feature-flags): decouple cache lifetime from telemetry

e75abf3

Signed-off-by: Cathleen Yan <58714163+cathleeny@users.noreply.github.com>

chore(kernel): bump to merged feature flag cache

ac375f5

Signed-off-by: Cathleen Yan <58714163+cathleeny@users.noreply.github.com>

cathleeny added the integration-test-kernel Preview the full passthrough kernel integration suite (live warehouse) label Oct 7, 2026

cathleeny marked this pull request as ready for review October 7, 2026 17:38

peco-review-bot Bot approved these changes Oct 7, 2026

View reviewed changes

Comment thread KERNEL_REV Outdated

docs(feature-flags): update shared cache usage guide

fe36481

Signed-off-by: Cathleen Yan <58714163+cathleeny@users.noreply.github.com>

github-actions Bot removed the integration-test-kernel Preview the full passthrough kernel integration suite (live warehouse) label Oct 8, 2026

peco-review-bot Bot approved these changes Oct 8, 2026

View reviewed changes

Comment thread internal/featureflags/cache.go Outdated

cathleeny requested a review from jay-xiao446 October 12, 2026 18:48

cathleeny added 2 commits October 12, 2026 18:54

Merge main and retain merged kernel telemetry revision

f5f7b45

Signed-off-by: Cathleen Yan <58714163+cathleeny@users.noreply.github.com>

Fix shared flag-fetch cancellation and sync pinned kernel header

b188769

Signed-off-by: Cathleen Yan <58714163+cathleeny@users.noreply.github.com>

peco-review-bot Bot approved these changes Oct 12, 2026

View reviewed changes

Comment thread internal/featureflags/cache.go

Comment thread internal/featureflags/cache.go

docs(featureflags): clarify value encoding and refresh cancellation

310037b

Signed-off-by: Cathleen Yan <58714163+cathleeny@users.noreply.github.com>

peco-review-bot Bot approved these changes Oct 12, 2026

View reviewed changes

Comment thread internal/backend/kernel/include/databricks_kernel.h Outdated

jay-xiao446 reviewed Oct 12, 2026

View reviewed changes

Comment thread internal/backend/kernel/include/databricks_kernel.h Outdated

chore: drop unrelated kernel header sync

537eee4

Signed-off-by: Cathleen Yan <58714163+cathleeny@users.noreply.github.com>

jay-xiao446 reviewed Oct 12, 2026

View reviewed changes

Comment thread connector.go Outdated

Comment thread connector.go Outdated

peco-review-bot Bot approved these changes Oct 12, 2026

View reviewed changes

fix(feature-flags): share lazy cache across driver backends

1bf0d6b

Signed-off-by: Cathleen Yan <58714163+cathleeny@users.noreply.github.com>

peco-review-bot Bot approved these changes Oct 13, 2026

View reviewed changes

Comment thread internal/featureflags/cache.go

Comment thread internal/featureflags/cache.go

jay-xiao446 approved these changes Oct 13, 2026

View reviewed changes

cathleeny enabled auto-merge October 13, 2026 23:42

cathleeny added this pull request to the merge queue Oct 13, 2026

Merged via the queue into main with commit 59f8bae Oct 13, 2026
17 checks passed

cathleeny deleted the cathleeny/general-driver-flags branch October 13, 2026 23:49

This was referenced Oct 14, 2026

[coverage] Conformance findings: FEATUREFLAG-001 adbc-drivers/databricks#707

Open

[coverage] Conformance findings: FEATUREFLAG-001 #499

Open

[coverage] Conformance findings: FEATUREFLAG-001 databricks/databricks-sql-python#984

Open

Conversation

cathleeny commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Validation

Uh oh!

github-actions Bot commented Oct 7, 2026

Uh oh!

peco-review-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Oct 8, 2026

Uh oh!

peco-review-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

peco-review-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

peco-review-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

peco-review-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

peco-review-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jay-xiao446 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cathleeny commented Oct 6, 2026 •

edited

Loading