sadpandajoe commented on code in PR #44851:
URL: https://github.com/apache/superset/pull/44851#discussion_r4191644703
##########
superset/semantic_layers/models.py:
##########
@@ -703,7 +721,18 @@ def data_for_slices(self, slices: list[Any]) ->
ExplorableData:
return self.data
def get_extra_cache_keys(self, query_obj: QueryObjectDict) ->
list[Hashable]:
Review Comment:
Result-cache keys now include the catalog token, but an async chart query is
cached by the worker under the token it captured while the browser's
synchronous read-back re-resolves the token. If the catalog naturally expires
while the query runs (e.g. 60s catalog lifetime, 90s query), the worker caches
under T0, the read-back resolves T1 (a fresh token is issued on expiry even
when definitions are unchanged), and the force-nonce marker does not match the
new cache key, so the same long query executes a second time inline. Is that
double execution on expiry intended, or should the read-back reuse the token
the task cached under?
##########
docs/developer_docs/semantic-metadata-store.md:
##########
@@ -0,0 +1,286 @@
+---
+title: Shared semantic metadata storage
+---
+
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+## Enablement and scope
+
+This host implementation supports the optional [SDK metadata
contract](./semantic-metadata-contract.md).
+It provides storage and cache identity; it does not add refresh endpoints or
UI.
+Both `SEMANTIC_LAYERS` and `SEMANTIC_LAYER_METADATA_REFRESH_ENABLED` remain off
+by default. A provider must explicitly declare support and supply the adapter
+and captured view token. Legacy providers retain their existing behavior.
+
+For participating stored layers, the runtime-schema endpoint uses the bound
+adapter's catalog, so its choices follow refreshed metadata just like view
+discovery. Participation classification normalizes invalid stored provider
types
+and configurations to the stable metadata configuration error. The
runtime-schema
+endpoint retains its existing unknown-type response.
+
+Before enabling, configure `DISTRIBUTED_COORDINATION_CONFIG` with Redis or
Redis
+Sentinel, and `SEMANTIC_LAYER_METADATA_NAMESPACE` with a trusted, nonempty
+string or zero-argument callable returning the deployment and tenant namespace.
+Never derive that namespace from unvalidated request fields. The host combines
+it with the stored connection UUID, provider type, configuration and
credentials
+under an HMAC using `SECRET_KEY`; keys do not contain raw credentials.
+
+Enable only on a homogeneous compatible host/provider fleet. Redis rollback or
+restore can resurrect retired entries: change the namespace before re-enabling
+a recovered fleet. This cache does not provide durable cross-failover ordering.
+
+## Publication and separate invalidation
+
+Each successful catalog publication receives a fresh opaque token, even if the
+discovery JSON is unchanged. A discovery digest is not a complete upstream
+semantic-model revision. Hits retain the token and expiry. Failures retain the
+previous observation's original expiry, when it still exists. Catalog
normalization
+preserves JSON numeric values, including decimals beyond binary floating-point
+precision and large or small exponents. Fresh metadata-database read failures
+report `unavailable` without driver or SQL details.
+
+A single lease admits one writer; publication atomically compares its owner,
+installs the observation and releases the lease. Catalog invalidation
atomically
+deletes both the observation and the old writer's authority. An expired or
+invalidated writer cannot publish over a newer observation.
+
+Compatibility answers and query results include the token captured with their
+view's metadata and the view configuration. Compatibility also has an
independent
+random generation: clearing compatibility retires all its selection variants
+without fetching metadata or invalidating query results. Late fills retain
their
+old captured key. Existing query/RLS identity and selected-query force refresh
+remain in the query-cache path. No global key scan or upstream cache purge
occurs.
+A SQL-backed chart's composite result key also captures participating semantic
+annotation sources. Async contribution tasks resolve totals using the dependent
+task's captured catalog: a matching entry is reused, while a different catalog
+requires recomputation before caching percentages. Failed totals acquisition
+(including a failed payload with an empty dataframe) stops the dependent query
+before contribution calculation or result caching. Pending participating tasks
+created without a serialized totals query fail closed; resubmit them after
+upgrading the fleet. Deploy workers before web nodes, or expect participating
+contribution tasks to fail until both are upgraded: an older worker cannot
accept
+the serialized totals query sent by a newer web node.
+
+The compatibility endpoint captures its generation before resolving the
provider
+view. A clear during that resolution cannot relabel the endpoint's old answer
with
+the new generation. Callers must not pre-resolve the view before this capture.
+
+Bounds are a 30-second metadata I/O budget, a non-renewing lease capped by the
+owner’s remaining budget (and at most 60 seconds), a
+configurable catalog lifetime measured from acquisition start (300 seconds by
+default), and a 10-MiB
+serialized envelope limit. Cold readers wait and re-read within the same
budget.
+A busy explicit refresh returns `in_progress`; an unknown write outcome returns
+`indeterminate`, not success or a blind retry.
+
+## Snapshot lifetime and chart-cache reuse
+
+`SEMANTIC_LAYER_METADATA_SNAPSHOT_TTL_SECONDS` sets the catalog lifetime and
the
+independent compatibility-generation lifetime. It defaults to `300` seconds and
+accepts integer values from `1` through `2147483647`; booleans, strings, zero,
+negative and out-of-range values fail with a configuration error. Catalog
+acquisition time counts against this lifetime. A discovery that consumes the
+entire lifetime fails with `deadline` and does not publish an expired snapshot.
+The setting does not extend the 30-second discovery budget or the writer lease.
+Atomic publication also caps freshness using the Redis lease's age, so
transport
+wait cannot add time to a snapshot. This conservative anchor begins at lease
+installation, before acquisition. An exhausted publication fence rejects the
+write and preserves any previous snapshot.
+
+Natural expiry still rotates the token, even when discovery returns identical
+fields: those fields need not contain the full metric definition. A longer
+lifetime lets chart results remain reachable longer but delays rediscovery of
+metadata changes; a shorter lifetime favors freshness and increases discovery
+and chart re-query work. This bounds reuse even when a chart has a longer
+`cache_timeout`. Explicit refresh still rotates immediately after successful
+publication. Hits and failures never renew snapshot expiry.
+
+Configure the same value on all participating workers. Changes affect newly
+published snapshots and newly created or invalidated compatibility generations;
+existing entries retain their original TTL. Use the scoped invalidation
controls
+when existing entries must expire earlier. Provider-supplied definition
revisions
+may enable safe same-definition reuse in a future change; they are not
supported
+by this setting.
+
+## Operation lifetime and transport
+
+HTTP requests establish the absolute monotonic deadline before authentication
+hooks. Celery tasks establish it before task execution; eager/nested work
shares
+the active operation. Other synchronous host callers must enter
+`metadata_operation()` before access checks. A later store call never
replenishes
+the budget; explicit worker budgets are capped at 30 seconds. The host passes
+that deadline explicitly to `adapter.bind(store, deadline=...)`. The adapter
+passes it to `store.read(fetch, deadline=...)` for discovery and to
+`store.refresh(fetch, deadline=...)` for an explicit refresh. An earlier caller
+deadline narrows both store work and Redis transport; a deadline beyond the
+operation ceiling is rejected. A call never mutates the operation or another
+call's budget. Invalid/exhausted call deadlines fail before even cache-hit
I/O. Access to an
+already-captured layer or view remains valid after that budget expires, so a
+long-running chart query does not lose its observation. Further metadata I/O
+still fails at the original deadline. Provider instances and views are scoped
to that operation, so reusing
+a SQLAlchemy model in a later request cannot reuse an old provider observation.
+Parsed configurations are cached only within that operation and by their stored
+JSON text; changing the stored configuration invalidates the parsed value.
+Provider mutation cannot alter the cached parse. Flag-off provider construction
+retains its existing cache behavior.
+
+Private Redis clients use the installed redis-py asyncio transport and one
+cancellation timeout per command, bounded by the operation's remaining time.
+This covers connection setup, Sentinel discovery and response parsing; retries
+are disabled. Configured `CACHE_REDIS_SOCKET_TIMEOUT` and
+`CACHE_REDIS_SOCKET_CONNECT_TIMEOUT` values are retained when shorter than the
+remaining budget, allowing Sentinel to try another node after a node timeout.
+For Sentinel deployments, start with finite positive per-node values such as
+`CACHE_REDIS_SOCKET_TIMEOUT = 1.0` and
Review Comment:
As written, an operator who copies these two standalone
`CACHE_REDIS_SOCKET_*` assignments into `superset_config.py` gets no Sentinel
timeout tuning: the private metadata client only reads these keys from the
`DISTRIBUTED_COORDINATION_CONFIG` dict (`DeadlineRedisBackend._config`), so
with no per-node timeout set it falls back to the full remaining operation
budget and a hung first node can still consume the whole 30s. Should the
example show them nested inside `DISTRIBUTED_COORDINATION_CONFIG`?
--
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]
