Do not send SKIP LOCKED to servers that silently ignore it

[1fanwang]

The scheduler claims task instances with SELECT ... FOR UPDATE SKIP LOCKED and
relies on the clause to keep concurrent schedulers off the same rows. A server
that accepts the clause and then discards it hands the same rows to every
scheduler at once — no error, no warning, no degraded-mode log line.

with_row_locks() already guards the case where a MySQL-family server cannot
lock at all. This extends that to a server that claims to skip but does not: read
the version banner once per engine, and fall back to plain blocking FOR UPDATE
when the server is known to ignore SKIP LOCKED. Schedulers then serialize
instead of skipping ahead, which is correct — and only on servers that were
never providing the guarantee. PostgreSQL and MySQL are untouched.

TiDB is the case I hit: it has parsed and ignored SKIP LOCKED since 2020
(pingcap/tidb#18207, still open,
listed under unsupported features).
Unlike FOR SHARE, which it rejects loudly with error 1235, there is nothing for
an operator to notice.

If the version probe fails the previous behaviour is kept, so a transient
connection problem cannot silently disable SKIP LOCKED on a server that
supports it.

Testing Done

Against TiDB v8.5.1, with MySQL 8.4 and PostgreSQL 16 as controls:

backend	emits SKIP LOCKED	locking clause
TiDB v8.5.1	no	FOR UPDATE OF task_instance
MySQL 8.4	yes	FOR UPDATE OF task_instance SKIP LOCKED
PostgreSQL 16	yes	FOR NO KEY UPDATE OF task_instance SKIP LOCKED

Two schedulers against one TiDB, 15 dag runs: all successful, zero retries, zero
duplicate attempts.

Raw logs

The hazard. Two workers running the scheduler's claim shape, each holding its
read before either commits:

claim sets: {'w1': [1, 2, 3], 'w2': [1, 2, 3]}
commits   : {'w1_commit': 'ok', 'w2_commit': 'ok'}
final rows: ((1, 'w2'), (2, None), (3, None))
OVERLAP: [1, 2, 3] -> both workers saw the same rows

Both claimed every row, both committed, the later write won. SHOW WARNINGS was
empty, tidb_enable_noop_functions was OFF. On MySQL 8 the claim sets are disjoint.

Same race with the fallback (plain FOR UPDATE, READ COMMITTED):

w1 claimed: [1, 2, 3]
w2 claimed: [4, 5, 6]
OVERLAP  : []
VERDICT: SAFE - disjoint claims, every row owned exactly once

Emitted SQL per backend, and the warning operators get:

[warning] Database server reports as '8.0.11-tidb-v8.5.1', which accepts SKIP LOCKED
but does not honor it. Falling back to plain FOR UPDATE so concurrent schedulers
cannot claim the same rows. Schedulers will block on each other instead of
skipping ahead. [airflow.utils.sqlalchemy]

backend     emits SKIP LOCKED  locking clause
----------------------------------------------------------------------------
tidb                    False  LIMIT 512 FOR UPDATE OF task_instance
mysql                    True  LIMIT 512 FOR UPDATE OF task_instance SKIP LOCKED
postgres                 True  LIMIT 512 FOR NO KEY UPDATE OF task_instance SKIP LOCKED

New tests against the unpatched source — they fail, which is the point:

E   Expected: with_for_update(key_share=True)
E   Actual: with_for_update(skip_locked=True, key_share=True)
FAILED ...[tidb-ignores-skip-locked]
FAILED ...[tidb-lowercase-banner]
2 failed, 3 passed

With the change: 35 passed in tests/unit/utils/test_sqlalchemy.py.

Two schedulers, one TiDB, 15 dag runs:

dag_run states      : {'success': 15}
task_instance states: {'success': 105}
task instances with try_number > 1: 0
duplicate attempt rows in task_instance_history: 0
live SchedulerJob rows: 2

Regressions: test_scheduler_job.py -k "critical_section or executable_task_instances or row_lock or pool" — 47 passed.

---

Was generative AI tooling used to co-author this PR?

• Yes — GitHub Copilot CLI (Claude Opus 5)

Generated-by: GitHub Copilot CLI (Claude Opus 5) following the guidelines

[vikramkoka]

Oh, fascinating

Stefan, I am not familiar with TiDB.
Doesn't have to be in response to this PR, but curious about your thoughts on why this for the Airflow meta database?
Presumably for scaling, but more details would be very useful.

[1fanwang]

Oh, fascinating

Stefan, I am not familiar with TiDB. Doesn't have to be in response to this PR, but curious about your thoughts on why this for the Airflow meta database? Presumably for scaling, but more details would be very useful.

Hey Vikram, I was actually just reading more about #46175 and #65453.

To be forthcoming - I'm not proposing official support for Airflow on a TiDB backend, and this PR isn't meant to be a step toward asking for it, at least for now. Let me share

1. what I'm doing internally (Since some of this info is already public ([1](https://www.youtube.com/watch?v=DzZ-I8WL[2](https://www.youtube.com/watch?v=YFNS3PCvuQQ)jM), 2))
2. and what I think is useful to the community today:

For scale context, our largest single cluster is reaching 25k+ Dags and still growing. Most of the scaling problems we've hit have answers that stay close to upstream: add schedulers, tune the executor, etc.. The metadata DB is the one that doesn't. It's the piece *can't easily be sharded, since the scheduler critical section, TI state, XCom and event logs all land on one primary, and when that saturates the options are a bigger box (we've done that many times) or changes that drift away from OSS Airflow. I'd rather not drift.

We've also been trying read/write splitting to take some pressure off it, since we already run read replicas. Either route reads explicitly in Airflow's own source, or put query routing rules in a [ProxySQL](https://proxysql.com/) layer so Airflow core can stay generic. No numbers to share yet, but the challenge is clear - it only moves read load, and the scheduler's hot path is writes. The queries that actually hurt are the ones you can't route away. That's what makes distributing writes interesting, and TiDB uses the MySQL wire protocol. I haven't benchmarked it though, so "scales writes" is a motivation and not a result I can show at this point.

So far this is just a small local cluster with dev Airflow pointed at it, checking the SQL queries (the ones scheduler depends on). Most of it holds up: pessimistic FOR UPDATE blocks, NOWAIT errors, GET_LOCK is exclusive, READ COMMITTED behaves, savepoints roll back, FK cascades are enforced. The 3 prs I opened addresses some of the minor issues I found during testing.

For this PR tho, my original thought is it isn't really a TiDB-specific problem. Any server that accepts SKIP LOCKED and quietly drops it hands two schedulers the same rows, and Airflow never finds out (it should fail loudly instead). with_row_locks() already degrades gracefully when a MySQL-family engine can't lock at all.

I do plan on trying this on our internal Airflow and TiDB clusters. Happy to share what we find running Airflow on TiDB at scale if folks are interested.

The three that came out of the exercise, for reference:

• Allow configuring the collation of asset name columns #71249
• Do not send SKIP LOCKED to servers that silently ignore it #71250
• Fix MySQL metadata database failing when only PyMySQL is installed #71251

[uranusjr]

I don’t quite understand. So TiDB silently ignores SKIP LOCKED, but this PR simply makes Airflow not send that. So the end result is unchanged? Why is this PR needed?