mikebridge commented on PR #42760: URL: https://github.com/apache/superset/pull/42760#issuecomment-5184683182
This replacement keeps the containment-reuse idea from #40221, but changes its safety and architecture substantially: - **Explicit opt-in:** the original PR automatically wrapped semantic queries once `SEMANTIC_LAYERS` was enabled. This PR adds a separate default-off `SEMANTIC_LAYER_CONTAINMENT_CACHE` rollout flag, and requires each provider to explicitly delegate cache responsibility to Superset. - **Provider contract:** providers declare whether reuse is global or execution-context scoped, supply secret-free result-affecting identity material, and opt into only the comparison, membership, NULL, and pattern semantics they can guarantee. - **Security-aware identity:** cache identity includes the semantic definition and provider material; context-scoped identity also incorporates a hash of the principal, roles, guest-token claims, and RLS cache key. - **Smaller architecture:** the original cache implementation was a large multipurpose class/module. This version separates application orchestration, host adaptation, identity, containment policy, result transformation, repository storage, shared types, and distributed coordination. - **Distributed mutation safety:** descriptor updates use bounded Redis owner-token leases and atomic compare-and-delete release, with direct Redis and Sentinel coverage. This replaces the original non-atomic descriptor-index updates. - **Failure boundaries:** expected lookup, storage, and coordination failures leave provider queries available. Invalid or unsupported deployment configuration disables containment rather than semantic queries. - **Forced refresh:** `force_query` bypasses containment reads without changing ordinary result identity, and a successful forced provider result is stored under the canonical identity. - **Result provenance:** semantic containment hits are propagated separately from ordinary query-cache hits so the UI can explain which cache supplied the result. - **Rollout and observability:** requested/effective state, fixed-name metrics, unsupported-backend warnings, canary guidance, restart convergence, and rollback instructions are included. - **Verification:** the replacement adds deterministic concurrency and negative-control tests, a pinned real Redis/Sentinel CI path, and 100% statement/branch coverage for the semantic-layer package. The implementation also incorporates the still-valid fixes from #41824, #41825, #41826, and #41856 while avoiding unrelated files from the original branch. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
