hughhhh opened a new pull request, #45559:
URL: https://github.com/apache/superset/pull/45559
### SUMMARY
Stacked on #43870, addressing the review point on probe bounds. Before this
change, the synchronous probe on the chart-query path had nothing limiting how
much work it did:
- **Persistent failures were re-probed on every query.** Failures were
deliberately never cached.
- **No distinct-value cap.** Each value becomes one `SELECT` column, so
1,665 values produce 1,665 columns. Reproduced against `postgres:17`: 1,664
columns succeed, and 1,665 fail with `target lists can have at most 1664
entries`. That failure was also not cached, so it repeated on every load.
- **No probe timeout.** Chart requests carry no query deadline.
- **Default `NullCache` re-probed successes too,** and the pruning-indicator
verdict was never stored.
Chart-result cache hits were already protected: `get_extra_cache_keys`
builds without mirrors.
### Changes
| Config | Default | Effect |
|---|---|---|
| `PARTITION_TRANSFORM_PROBE_MAX_VALUES` | 1000 | Filters with more distinct
values are declined (no pruning, no warehouse call); the reason goes to the
preview's error sink. `0` means no cap. |
| `PARTITION_TRANSFORM_PROBE_TIMEOUT` | 10s | `utils.timeout` around
`get_df`, best effort like Superset's other timeouts (SIGALRM, main thread
only). `0` disables it. |
| `PARTITION_TRANSFORM_PROBE_FAILURE_CACHE_TIMEOUT` | 5m | Failed probes are
remembered under `<key>:failed` along with the engine's messages, so the
preview can still explain them. `0` restores retry-on-every-query. |
- **Cache fallback:** when `CACHE_CONFIG` is a `NullCache`, probe results
and verdicts go to `DATA_CACHE_CONFIG`, and when that is also a `NullCache`, to
a per-process `SimpleCache(threshold=1024)`. The preview rate limiter still
reads `CACHE_CONFIG` only.
- **Metrics** under `partition_mapping.probe.`: `cache_hit`, `cache_miss`,
`failure_cache_hit`, `success`, `error`, `timeout`,
`declined.{too_many_values,value_does_not_round_trip,unstorable_transform,unsafe_probe_sql}`,
plus a `latency_ms` timing. A stats-logger failure never affects the chart.
- **Tests:** `test_a_failed_probe_is_not_cached` asserted the old contract.
It is replaced by "remembered briefly" and "retried once expired (TTL 0)"
tests, plus new tests for the cap, the timeout, the NullCache fallback,
metrics, and a broken stats logger. A `tests/unit_tests/conftest.py` fixture
clears the per-process fallback between tests.
- **Docs:** the admin docs gain the new settings, the bounds, the metrics,
and the cache fallback.
### TESTING INSTRUCTIONS
```
pytest tests/unit_tests/connectors/sqla/partition_mapping_test.py
tests/unit_tests/datasets/partition_mapping_preview_test.py
tests/unit_tests/models/partition_mirroring_test.py
```
Locally: 644 passed for those three files. A wider sweep (`models`,
`connectors`, `datasets`, `commands/dataset`, `views/datasource`, `common`,
`charts`, `dao/dataset_test.py`, plus the Hive and Impala engine-spec tests)
gave 2,936 passed and 2 xfailed. ruff and the pre-commit mypy 1.15 env are
clean.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags: `PARTITION_FILTER_MAPPING`
- [ ] Changes UI
- [ ] Includes DB Migration
- [x] Introduces new feature or API (config keys)
- [ ] Removes existing feature or API
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
