Let an auth manager filter dags in SQL instead of materializing every id - #71341
Let an auth manager filter dags in SQL instead of materializing every id#713411fanwang wants to merge 1 commit into
Conversation
cb4476c to
6c06640
Compare
get_authorized_dag_ids returns a set, so every authorized dag id is loaded into memory before pagination is applied. FabAuthManager keeps its grants in the metadata database and still does this: a user authorized on all dags gets select(DagModel.dag_id) materialized on every list request. get_authorized_dag_ids_select lets a manager return a select instead, which the permitted-dag filters apply as a subquery. Returning None, the default, keeps the existing behaviour, and every permitted-* filter inherits it because they all build the clause with in_(). Signed-off-by: 1fanwang <1fannnw@gmail.com>
6c06640 to
fe5c60f
Compare
vincbeck
left a comment
There was a problem hiding this comment.
I understand the motivation but I am against introducing a new method in base_auth_manager class that is only applicable to Fab. Fab auth manager is the only auth manager using the Db to manage permissions, so get_authorized_dag_ids_select is Fab specific. Ideally, we should get rid of Fab, and not add Fab specific features in Airflow core.
Agreed, and thanks for the context on the direction. @vincbeck I checked and confirmed there's also no easy way to update this in FAB only. Closing. |
Closes: #71309
Why
get_authorized_dag_idsis where every list endpoint starts, and its default implementation readsthe whole dag table before any manager is consulted, groups the rows by team, then calls
filter_authorized_dag_idsonce per team. A request for 50 dags loads every dag id in thedeployment, so the cost scales with the deployment rather than the page.
That is core's own path, not one provider's.
FabAuthManageroverrides it and still reads everyrow. Keycloak inherits it and then calls out per dag, which
#69041 and
#61686 report as ten and twenty-five second
pages.
It also makes the documented answer expensive.
#23638 asked for dag permissions by tag or owner
rather than by name, and Airflow 3 answers that by writing an auth manager over
get_authorized_dag_ids. Tags, bundles and teams are all rows in this database, andget_db_managerlets a manager add its own. Theset[str]return type is what stops any of thembeing answered as a query.
What changed
BaseAuthManagergainsget_authorized_dag_ids_select, returning a select of dag ids orNone.Noneis the default and keeps today's behaviour, so managers backed by an external policy service(Keycloak, Amazon Verified Permissions) are unaffected.
A returned select is applied as
dag_id IN (subquery). The dependency graph services and therun-state counts endpoint still need ids in memory, and still get them:
PermittedDagFiltermaterializes on first read.
A select replaces
get_authorized_dag_idsoutright, including its per-team grouping, so amulti-team manager scopes the select itself or returns
Noneand keeps the fan-out.FabAuthManagerimplements it with the grant query it already knows how to write.Testing Done
End to end against a running API server. A 40-line manager authorizing by tag, no FAB. Six
dags seeded, three tagged
team-a.The statement that server executed, captured with a
before_cursor_executelistener:A subquery rather than bind parameters,
LIMITin the same statement, and the same subqueryreaching the latest-run join.
Measured. The same shape at scale. MySQL 8, 41,606 dags with an environment tag and a team
tag. Page of 50, median of 15 rounds.
Measured, FAB. Same database at 41,606 dags, 1,610 roles, 66,529 per-dag edit grants.
The expensive row in both is the ordinary one: a tag most dags carry, or a role with global
can_read, which is Viewer and up.Regression tests. Eight, covering the
Nonedefault, a select reaching the SQL as a subquery,an empty select honoured as "nothing is permitted", a subclass filter inheriting it, a team-scoped
select, and the id set materializing once and only when read. Reverting the fix:
Restored: