1fanwang opened a new pull request, #71307: URL: https://github.com/apache/airflow/pull/71307
A momentary metadata-database failure during a connection or variable lookup is currently indistinguishable from the secret not existing. `Connection.get_connection_from_secrets` and `Variable.get_variable_from_secrets` swallow every exception a backend raises, log it at **debug** level, and fall through to the next backend ([`connection.py:530`](https://github.com/apache/airflow/blob/2c9d91ef34/airflow-core/src/airflow/models/connection.py#L530), [`variable.py:491`](https://github.com/apache/airflow/blob/2c9d91ef34/airflow-core/src/airflow/models/variable.py#L491), and the same pattern in the Task SDK's server-context lookup at [`context.py:249`](https://github.com/apache/airflow/blob/2c9d91ef34/task-sdk/src/airflow/sdk/execution_time/context.py#L249)). So a dropped connection, a failover, or a deadlock reaches the caller as `The conn_id 'x' isn't defined` / a 404, not as a retryable server error — and the reason is invisible at default log level. Three consequences: - The Execution API returns **404** for a connection or variable that exists. A 404 is a permanent answer, so the Task SDK does not retry it and the task fails outright with a misleading message. - With `[secrets] use_cache = True`, `get_variable_from_secrets` caches the resulting `None` ([`variable.py:498`](https://github.com/apache/airflow/blob/2c9d91ef34/airflow-core/src/airflow/models/variable.py#L498) — "we save None as well"). One blip then poisons that key for the whole `cache_ttl_seconds` window without touching the database again. - The triggerer, Dag processor, and callback supervisor call `MetastoreBackend` in-process, so a deferred task hits the same bogus not-found. `MetastoreBackend` is the only backend in the default chain that talks to the metadata database, and Airflow already ships `retry_db_transaction` for this class of failure. Stacking it under `@provide_session` retries within a single session and rolls back between attempts, matching the existing use in [`renderedtifields.py:241`](https://github.com/apache/airflow/blob/2c9d91ef34/airflow-core/src/airflow/models/renderedtifields.py#L241), [`dagwarning.py:78`](https://github.com/apache/airflow/blob/2c9d91ef34/airflow-core/src/airflow/models/dagwarning.py#L78), and [`manager.py:701`](https://github.com/apache/airflow/blob/2c9d91ef34/airflow-core/src/airflow/dag_processing/manager.py#L701). Both lookups are reads, so retrying them is safe. Attempts are bounded by the existing `[database] max_db_retries`, and a genuinely missing secret still returns `None` on the first attempt — no new latency on the common path. ## Testing Done The end-to-end path first, since it is what actually breaks. `files/e2e_metastore_transient.py` seeds a connection and a variable that both exist, injects **one** transient `OperationalError` at the SQLAlchemy `Session.scalar` level (how a resolver failure, failover, or dropped connection presents), then calls `Connection.get_connection_from_secrets` / `Variable.get_variable_from_secrets` — the same functions the Execution API connection and variable routes call. <details><summary>Raw logs — end-to-end, before and after</summary> ```console $ git stash push -- airflow-core/src/airflow/secrets/metastore.py # pre-fix source $ uv run --no-sync --project airflow-core python files/e2e_metastore_transient.py seeded: connection 'e2e_conn' and variable 'e2e_var' both exist in the metastore --- Connection.get_connection_from_secrets('e2e_conn') with one transient DB error --- RESULT: SPURIOUS NOT-FOUND -> AirflowNotFoundException: The conn_id `e2e_conn` isn't defined --- Variable.get_variable_from_secrets('e2e_var') with one transient DB error --- RESULT: SPURIOUS NOT-FOUND -> None (Variable.get would raise KeyError -> HTTP 404) $ git stash pop # with the fix $ uv run --no-sync --project airflow-core python files/e2e_metastore_transient.py seeded: connection 'e2e_conn' and variable 'e2e_var' both exist in the metastore --- Connection.get_connection_from_secrets('e2e_conn') with one transient DB error --- RESULT: OK -> conn_id=e2e_conn host=db.example.com --- Variable.get_variable_from_secrets('e2e_var') with one transient DB error --- RESULT: OK -> 'e2e_value' ``` </details> The two new regression tests drive a real session against a real database and fail on unpatched source. <details><summary>Raw logs — regression tests, red then green</summary> ```console $ git stash push -- airflow-core/src/airflow/secrets/metastore.py # pre-fix source $ uv run --no-sync --project airflow-core pytest airflow-core/tests/unit/always/test_secrets_metastore.py -q -k TransientDBErrors airflow-core/src/airflow/secrets/metastore.py:78: in get_variable var_value = session.scalar( E sqlalchemy.exc.OperationalError: (builtins.Exception) server closed the connection unexpectedly E [SQL: SELECT 1] FAILED airflow-core/tests/unit/always/test_secrets_metastore.py::TestMetastoreBackendTransientDBErrors::test_get_connection_retries_transient_db_error FAILED airflow-core/tests/unit/always/test_secrets_metastore.py::TestMetastoreBackendTransientDBErrors::test_get_variable_retries_transient_db_error ================= 2 failed, 5 deselected, 1 warning in 11.67s ================== $ git stash pop # with the fix $ uv run --project airflow-core pytest airflow-core/tests/unit/always/test_secrets_metastore.py -q airflow-core/tests/unit/always/test_secrets_metastore.py::TestMetastoreBackendSessionSafety::test_get_connection_preserves_pending_session_objects[found] PASSED airflow-core/tests/unit/always/test_secrets_metastore.py::TestMetastoreBackendSessionSafety::test_get_connection_preserves_pending_session_objects[not_found] PASSED airflow-core/tests/unit/always/test_secrets_metastore.py::TestMetastoreBackendSessionSafety::test_get_variable_preserves_pending_session_objects[found] PASSED airflow-core/tests/unit/always/test_secrets_metastore.py::TestMetastoreBackendSessionSafety::test_get_variable_preserves_pending_session_objects[not_found] PASSED airflow-core/tests/unit/always/test_secrets_metastore.py::TestMetastoreBackendSessionSafety::test_get_connection_returns_detached_object PASSED airflow-core/tests/unit/always/test_secrets_metastore.py::TestMetastoreBackendTransientDBErrors::test_get_connection_retries_transient_db_error PASSED airflow-core/tests/unit/always/test_secrets_metastore.py::TestMetastoreBackendTransientDBErrors::test_get_variable_retries_transient_db_error PASSED ======================== 7 passed, 1 warning in 33.69s ========================= ``` </details> `ruff format`, `ruff check`, and `prek run --files` on both changed files are clean. -- 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]
