Cache dag access and teams check to be reused in grid ti_summary API calls - #61623

Closed
tirkarthi wants to merge 1 commit into
apache:mainfrom
tirkarthi:gh61485
Closed

Cache dag access and teams check to be reused in grid ti_summary API calls#61623
tirkarthi wants to merge 1 commit into
apache:mainfrom
tirkarthi:gh61485

Conversation

@tirkarthi

@tirkarthitirkarthi commented Feb 8, 2026

Copy link
Copy Markdown
Contributor

requires_access_dag is used in the ti_summary endpoint. When this method returns a result then this can be cached and reused for other API calls since access to a dag doesn't change based on taskinstance. On a similar note for dags that don't have many changes the serialized dag entry also remains the same.

This PR adds caching to airflow-core which is independent from fab provider. Hence the fab related ttl cannot be always used and this might need a new config if the approach is accepted.

command with 10 concurrent requests since the grid loads 10 dagruns by default.

hey -c 10 -H 'Cookie: _token=<token>' 'http://localhost:8000/ui/grid/ti_summaries/asset_produces_2/manual__2026-01-31T06:15:30.690694+00:00'

Main branch

Summary:
Total:	1.0294 secs
Slowest:	0.0801 secs
Fastest:	0.0160 secs
Average:	0.0460 secs
Requests/sec:	194.2854
Latency distribution:
10% in 0.0335 secs
25% in 0.0385 secs
50% in 0.0438 secs
75% in 0.0537 secs
90% in 0.0626 secs
95% in 0.0667 secs
99% in 0.0758 secs

cache only is_authorized_dag in fab auth manager used in requires_access_dag check

Summary:
Total:	0.9620 secs
Slowest:	0.0741 secs
Fastest:	0.0163 secs
Average:	0.0410 secs
Requests/sec:	207.8980
Latency distribution:
10% in 0.0295 secs
25% in 0.0328 secs
50% in 0.0403 secs
75% in 0.0496 secs
90% in 0.0537 secs
95% in 0.0560 secs
99% in 0.0630 secs

cache get_team_name and is_authorized_dag in fab auth manager used in requires_access_dag check

Summary:
Total:	0.8245 secs
Slowest:	0.0813 secs
Fastest:	0.0115 secs
Average:	0.0304 secs
Requests/sec:	242.5708
Latency distribution:
10% in 0.0204 secs
25% in 0.0234 secs
50% in 0.0262 secs
75% in 0.0365 secs
90% in 0.0497 secs
95% in 0.0531 secs
99% in 0.0628 secs

cache get_team_name and is_authorized_dag in fab auth manager used in requires_access_dag check and _get_serdag.

Summary:
Total:	0.6000 secs
Slowest:	0.0532 secs
Fastest:	0.0100 secs
Average:	0.0249 secs
Requests/sec:	333.3144
Latency distribution:
10% in 0.0176 secs
25% in 0.0198 secs
50% in 0.0240 secs
75% in 0.0305 secs
90% in 0.0341 secs
95% in 0.0367 secs
99% in 0.0402 secs

related: #61485

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

No

requires_access_dag is used in the ti_summary endpoint. When this
method returns a result then this can be cached and reused for other
API calls since access to a dag doesn't change based on taskinstance.
@tirkarthi

Copy link
Copy Markdown
ContributorAuthor

As per my understanding there are only 5 connections in the pool by default and with concurrent requests and db calls being made synchronously in requires_access_dag and in the endpoint other requests might need to wait based on max_overflow.

Prompt : If I have connection pool as 5 in sqlalchemy and create 10 sessions concurrently then do the last 5 sessions wait for the initial 5 sessions to be returned back to the pool Gemini output : The short answer is: not necessarily. It depends entirely on your max_overflow setting. By default, SQLAlchemy doesn't just make those extra 5 sessions sit in a queue; it often creates "overflow" connections to handle the spike.
Here is the breakdown of how SQLAlchemy manages this scenario:
1. The pool_size vs. max_overflow
When you define a pool, SQLAlchemy uses two primary parameters to determine how to handle concurrent requests:
pool_size (5 in your case): The number of "permanent" connections kept open in the pool.
max_overflow (Default is 10): The number of additional connections the pool is allowed to create if the pool_size is exhausted.
2. What happens to your 10 sessions?
If you are using the default settings (pool_size=5, max_overflow=10):
Sessions 1–5: Grab the 5 "checked-in" connections from the pool immediately.
Sessions 6–10: Since the pool is full, the engine looks at max_overflow. Because 5 is less than the default limit of 10, it creates 5 new temporary connections.
Result: All 10 sessions run concurrently. No one waits.
3. When do sessions actually wait?
The "waiting" behavior only kicks in if you exceed pool_size + max_overflow.
If you set max_overflow=0 or if you tried to open 16 sessions with default settings:
The 16th session would hit the limit.
It would wait for a connection to be returned to the pool.
This wait is governed by pool_timeout (default is 30 seconds). If no connection becomes free in that time, SQLAlchemy throws a TimeoutError

@pierrejeambrunpierrejeambrun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should think about the cache invalidation cycle too.

For instance if I update the permissions of the user the is_authorized_dag cache isn't invalidated, and therefore the user won't have access until cache expires (I don't know when in the current implementation)

Also different workers can have different version of the cache in these specific case, one worker might grant access, while the other can block the access, I think this is a problem.

@tirkarthi

Copy link
Copy Markdown
ContributorAuthor

Thanks @pierrejeambrun . I updated the signature for the caching key and still see failures about the permission which I assume is about caching invalidation. The primary intention of the PR was around recurring checks for dag access around the same time within 1-2 seconds the grid loads. I will see if I can bring up an approach or to have a shorter ttl like 1-2 seconds within the grid loading cycle.

@pierrejeambrunpierrejeambrun mentioned this pull request Feb 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We normally prefer core changes and provider changes to be in separated PRs.
Is there a reason to keep both changes together?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we probably need to split the team vs dag_access caching. This will be easier for cherry picking.

@pierrejeambrunpierrejeambrun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The primary intention of the PR was around recurring checks for dag access around the same time within 1-2 seconds the grid loads. I will see if I can bring up an approach or to have a shorter ttl like 1-2 seconds within the grid loading cycle.

I see, there's a 30 seconds TTL, that could work yes. Invalidation is automatically done. We just need to fix the CI. Maybe keep this one about Core and create a new one for fab provider change.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we probably need to split the team vs dag_access caching. This will be easier for cherry picking.

@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

@github-actionsgithub-actionsBot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Apr 25, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providersprovider:fabstaleStale PRs per the .github/workflows/stale.yml policy file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@tirkarthi@pierrejeambrun@eladkal
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Cache dag access and teams check to be reused in grid ti_summary API calls - #61623

Closed
tirkarthi wants to merge 1 commit into
apache:mainfrom
tirkarthi:gh61485
Closed

Cache dag access and teams check to be reused in grid ti_summary API calls#61623
tirkarthi wants to merge 1 commit into
apache:mainfrom
tirkarthi:gh61485

Conversation

@tirkarthi

@tirkarthitirkarthi commented Feb 8, 2026

Copy link
Copy Markdown
Contributor

requires_access_dag is used in the ti_summary endpoint. When this method returns a result then this can be cached and reused for other API calls since access to a dag doesn't change based on taskinstance. On a similar note for dags that don't have many changes the serialized dag entry also remains the same.

This PR adds caching to airflow-core which is independent from fab provider. Hence the fab related ttl cannot be always used and this might need a new config if the approach is accepted.

command with 10 concurrent requests since the grid loads 10 dagruns by default.

hey -c 10 -H 'Cookie: _token=<token>' 'http://localhost:8000/ui/grid/ti_summaries/asset_produces_2/manual__2026-01-31T06:15:30.690694+00:00'

Main branch

Summary:
Total:	1.0294 secs
Slowest:	0.0801 secs
Fastest:	0.0160 secs
Average:	0.0460 secs
Requests/sec:	194.2854
Latency distribution:
10% in 0.0335 secs
25% in 0.0385 secs
50% in 0.0438 secs
75% in 0.0537 secs
90% in 0.0626 secs
95% in 0.0667 secs
99% in 0.0758 secs

cache only is_authorized_dag in fab auth manager used in requires_access_dag check

Summary:
Total:	0.9620 secs
Slowest:	0.0741 secs
Fastest:	0.0163 secs
Average:	0.0410 secs
Requests/sec:	207.8980
Latency distribution:
10% in 0.0295 secs
25% in 0.0328 secs
50% in 0.0403 secs
75% in 0.0496 secs
90% in 0.0537 secs
95% in 0.0560 secs
99% in 0.0630 secs

cache get_team_name and is_authorized_dag in fab auth manager used in requires_access_dag check

Summary:
Total:	0.8245 secs
Slowest:	0.0813 secs
Fastest:	0.0115 secs
Average:	0.0304 secs
Requests/sec:	242.5708
Latency distribution:
10% in 0.0204 secs
25% in 0.0234 secs
50% in 0.0262 secs
75% in 0.0365 secs
90% in 0.0497 secs
95% in 0.0531 secs
99% in 0.0628 secs

cache get_team_name and is_authorized_dag in fab auth manager used in requires_access_dag check and _get_serdag.

Summary:
Total:	0.6000 secs
Slowest:	0.0532 secs
Fastest:	0.0100 secs
Average:	0.0249 secs
Requests/sec:	333.3144
Latency distribution:
10% in 0.0176 secs
25% in 0.0198 secs
50% in 0.0240 secs
75% in 0.0305 secs
90% in 0.0341 secs
95% in 0.0367 secs
99% in 0.0402 secs

related: #61485

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

No

requires_access_dag is used in the ti_summary endpoint. When this
method returns a result then this can be cached and reused for other
API calls since access to a dag doesn't change based on taskinstance.
@tirkarthi

Copy link
Copy Markdown
ContributorAuthor

As per my understanding there are only 5 connections in the pool by default and with concurrent requests and db calls being made synchronously in requires_access_dag and in the endpoint other requests might need to wait based on max_overflow.

Prompt : If I have connection pool as 5 in sqlalchemy and create 10 sessions concurrently then do the last 5 sessions wait for the initial 5 sessions to be returned back to the pool Gemini output : The short answer is: not necessarily. It depends entirely on your max_overflow setting. By default, SQLAlchemy doesn't just make those extra 5 sessions sit in a queue; it often creates "overflow" connections to handle the spike.
Here is the breakdown of how SQLAlchemy manages this scenario:
1. The pool_size vs. max_overflow
When you define a pool, SQLAlchemy uses two primary parameters to determine how to handle concurrent requests:
pool_size (5 in your case): The number of "permanent" connections kept open in the pool.
max_overflow (Default is 10): The number of additional connections the pool is allowed to create if the pool_size is exhausted.
2. What happens to your 10 sessions?
If you are using the default settings (pool_size=5, max_overflow=10):
Sessions 1–5: Grab the 5 "checked-in" connections from the pool immediately.
Sessions 6–10: Since the pool is full, the engine looks at max_overflow. Because 5 is less than the default limit of 10, it creates 5 new temporary connections.
Result: All 10 sessions run concurrently. No one waits.
3. When do sessions actually wait?
The "waiting" behavior only kicks in if you exceed pool_size + max_overflow.
If you set max_overflow=0 or if you tried to open 16 sessions with default settings:
The 16th session would hit the limit.
It would wait for a connection to be returned to the pool.
This wait is governed by pool_timeout (default is 30 seconds). If no connection becomes free in that time, SQLAlchemy throws a TimeoutError

@pierrejeambrunpierrejeambrun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should think about the cache invalidation cycle too.

For instance if I update the permissions of the user the is_authorized_dag cache isn't invalidated, and therefore the user won't have access until cache expires (I don't know when in the current implementation)

Also different workers can have different version of the cache in these specific case, one worker might grant access, while the other can block the access, I think this is a problem.

@tirkarthi

Copy link
Copy Markdown
ContributorAuthor

Thanks @pierrejeambrun . I updated the signature for the caching key and still see failures about the permission which I assume is about caching invalidation. The primary intention of the PR was around recurring checks for dag access around the same time within 1-2 seconds the grid loads. I will see if I can bring up an approach or to have a shorter ttl like 1-2 seconds within the grid loading cycle.

@pierrejeambrunpierrejeambrun mentioned this pull request Feb 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We normally prefer core changes and provider changes to be in separated PRs.
Is there a reason to keep both changes together?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we probably need to split the team vs dag_access caching. This will be easier for cherry picking.

@pierrejeambrunpierrejeambrun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The primary intention of the PR was around recurring checks for dag access around the same time within 1-2 seconds the grid loads. I will see if I can bring up an approach or to have a shorter ttl like 1-2 seconds within the grid loading cycle.

I see, there's a 30 seconds TTL, that could work yes. Invalidation is automatically done. We just need to fix the CI. Maybe keep this one about Core and create a new one for fab provider change.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we probably need to split the team vs dag_access caching. This will be easier for cherry picking.

@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

@github-actionsgithub-actionsBot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Apr 25, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providersprovider:fabstaleStale PRs per the .github/workflows/stale.yml policy file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@tirkarthi@pierrejeambrun@eladkal
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Cache dag access and teams check to be reused in grid ti_summary API calls - #61623

Closed
tirkarthi wants to merge 1 commit into
apache:mainfrom
tirkarthi:gh61485
Closed

Cache dag access and teams check to be reused in grid ti_summary API calls#61623
tirkarthi wants to merge 1 commit into
apache:mainfrom
tirkarthi:gh61485

Conversation

@tirkarthi

@tirkarthitirkarthi commented Feb 8, 2026

Copy link
Copy Markdown
Contributor

requires_access_dag is used in the ti_summary endpoint. When this method returns a result then this can be cached and reused for other API calls since access to a dag doesn't change based on taskinstance. On a similar note for dags that don't have many changes the serialized dag entry also remains the same.

This PR adds caching to airflow-core which is independent from fab provider. Hence the fab related ttl cannot be always used and this might need a new config if the approach is accepted.

command with 10 concurrent requests since the grid loads 10 dagruns by default.

hey -c 10 -H 'Cookie: _token=<token>' 'http://localhost:8000/ui/grid/ti_summaries/asset_produces_2/manual__2026-01-31T06:15:30.690694+00:00'

Main branch

Summary:
Total:	1.0294 secs
Slowest:	0.0801 secs
Fastest:	0.0160 secs
Average:	0.0460 secs
Requests/sec:	194.2854
Latency distribution:
10% in 0.0335 secs
25% in 0.0385 secs
50% in 0.0438 secs
75% in 0.0537 secs
90% in 0.0626 secs
95% in 0.0667 secs
99% in 0.0758 secs

cache only is_authorized_dag in fab auth manager used in requires_access_dag check

Summary:
Total:	0.9620 secs
Slowest:	0.0741 secs
Fastest:	0.0163 secs
Average:	0.0410 secs
Requests/sec:	207.8980
Latency distribution:
10% in 0.0295 secs
25% in 0.0328 secs
50% in 0.0403 secs
75% in 0.0496 secs
90% in 0.0537 secs
95% in 0.0560 secs
99% in 0.0630 secs

cache get_team_name and is_authorized_dag in fab auth manager used in requires_access_dag check

Summary:
Total:	0.8245 secs
Slowest:	0.0813 secs
Fastest:	0.0115 secs
Average:	0.0304 secs
Requests/sec:	242.5708
Latency distribution:
10% in 0.0204 secs
25% in 0.0234 secs
50% in 0.0262 secs
75% in 0.0365 secs
90% in 0.0497 secs
95% in 0.0531 secs
99% in 0.0628 secs

cache get_team_name and is_authorized_dag in fab auth manager used in requires_access_dag check and _get_serdag.

Summary:
Total:	0.6000 secs
Slowest:	0.0532 secs
Fastest:	0.0100 secs
Average:	0.0249 secs
Requests/sec:	333.3144
Latency distribution:
10% in 0.0176 secs
25% in 0.0198 secs
50% in 0.0240 secs
75% in 0.0305 secs
90% in 0.0341 secs
95% in 0.0367 secs
99% in 0.0402 secs

related: #61485

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

No

requires_access_dag is used in the ti_summary endpoint. When this
method returns a result then this can be cached and reused for other
API calls since access to a dag doesn't change based on taskinstance.
@tirkarthi

Copy link
Copy Markdown
ContributorAuthor

As per my understanding there are only 5 connections in the pool by default and with concurrent requests and db calls being made synchronously in requires_access_dag and in the endpoint other requests might need to wait based on max_overflow.

Prompt : If I have connection pool as 5 in sqlalchemy and create 10 sessions concurrently then do the last 5 sessions wait for the initial 5 sessions to be returned back to the pool Gemini output : The short answer is: not necessarily. It depends entirely on your max_overflow setting. By default, SQLAlchemy doesn't just make those extra 5 sessions sit in a queue; it often creates "overflow" connections to handle the spike.
Here is the breakdown of how SQLAlchemy manages this scenario:
1. The pool_size vs. max_overflow
When you define a pool, SQLAlchemy uses two primary parameters to determine how to handle concurrent requests:
pool_size (5 in your case): The number of "permanent" connections kept open in the pool.
max_overflow (Default is 10): The number of additional connections the pool is allowed to create if the pool_size is exhausted.
2. What happens to your 10 sessions?
If you are using the default settings (pool_size=5, max_overflow=10):
Sessions 1–5: Grab the 5 "checked-in" connections from the pool immediately.
Sessions 6–10: Since the pool is full, the engine looks at max_overflow. Because 5 is less than the default limit of 10, it creates 5 new temporary connections.
Result: All 10 sessions run concurrently. No one waits.
3. When do sessions actually wait?
The "waiting" behavior only kicks in if you exceed pool_size + max_overflow.
If you set max_overflow=0 or if you tried to open 16 sessions with default settings:
The 16th session would hit the limit.
It would wait for a connection to be returned to the pool.
This wait is governed by pool_timeout (default is 30 seconds). If no connection becomes free in that time, SQLAlchemy throws a TimeoutError

@pierrejeambrunpierrejeambrun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should think about the cache invalidation cycle too.

For instance if I update the permissions of the user the is_authorized_dag cache isn't invalidated, and therefore the user won't have access until cache expires (I don't know when in the current implementation)

Also different workers can have different version of the cache in these specific case, one worker might grant access, while the other can block the access, I think this is a problem.

@tirkarthi

Copy link
Copy Markdown
ContributorAuthor

Thanks @pierrejeambrun . I updated the signature for the caching key and still see failures about the permission which I assume is about caching invalidation. The primary intention of the PR was around recurring checks for dag access around the same time within 1-2 seconds the grid loads. I will see if I can bring up an approach or to have a shorter ttl like 1-2 seconds within the grid loading cycle.

@pierrejeambrunpierrejeambrun mentioned this pull request Feb 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We normally prefer core changes and provider changes to be in separated PRs.
Is there a reason to keep both changes together?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we probably need to split the team vs dag_access caching. This will be easier for cherry picking.

@pierrejeambrunpierrejeambrun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The primary intention of the PR was around recurring checks for dag access around the same time within 1-2 seconds the grid loads. I will see if I can bring up an approach or to have a shorter ttl like 1-2 seconds within the grid loading cycle.

I see, there's a 30 seconds TTL, that could work yes. Invalidation is automatically done. We just need to fix the CI. Maybe keep this one about Core and create a new one for fab provider change.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we probably need to split the team vs dag_access caching. This will be easier for cherry picking.

@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

@github-actionsgithub-actionsBot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Apr 25, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providersprovider:fabstaleStale PRs per the .github/workflows/stale.yml policy file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@tirkarthi@pierrejeambrun@eladkal
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Cache dag access and teams check to be reused in grid ti_summary API calls - #61623

Closed
tirkarthi wants to merge 1 commit into
apache:mainfrom
tirkarthi:gh61485
Closed

Cache dag access and teams check to be reused in grid ti_summary API calls#61623
tirkarthi wants to merge 1 commit into
apache:mainfrom
tirkarthi:gh61485

Conversation

@tirkarthi

@tirkarthitirkarthi commented Feb 8, 2026

Copy link
Copy Markdown
Contributor

requires_access_dag is used in the ti_summary endpoint. When this method returns a result then this can be cached and reused for other API calls since access to a dag doesn't change based on taskinstance. On a similar note for dags that don't have many changes the serialized dag entry also remains the same.

This PR adds caching to airflow-core which is independent from fab provider. Hence the fab related ttl cannot be always used and this might need a new config if the approach is accepted.

command with 10 concurrent requests since the grid loads 10 dagruns by default.

hey -c 10 -H 'Cookie: _token=<token>' 'http://localhost:8000/ui/grid/ti_summaries/asset_produces_2/manual__2026-01-31T06:15:30.690694+00:00'

Main branch

Summary:
Total:	1.0294 secs
Slowest:	0.0801 secs
Fastest:	0.0160 secs
Average:	0.0460 secs
Requests/sec:	194.2854
Latency distribution:
10% in 0.0335 secs
25% in 0.0385 secs
50% in 0.0438 secs
75% in 0.0537 secs
90% in 0.0626 secs
95% in 0.0667 secs
99% in 0.0758 secs

cache only is_authorized_dag in fab auth manager used in requires_access_dag check

Summary:
Total:	0.9620 secs
Slowest:	0.0741 secs
Fastest:	0.0163 secs
Average:	0.0410 secs
Requests/sec:	207.8980
Latency distribution:
10% in 0.0295 secs
25% in 0.0328 secs
50% in 0.0403 secs
75% in 0.0496 secs
90% in 0.0537 secs
95% in 0.0560 secs
99% in 0.0630 secs

cache get_team_name and is_authorized_dag in fab auth manager used in requires_access_dag check

Summary:
Total:	0.8245 secs
Slowest:	0.0813 secs
Fastest:	0.0115 secs
Average:	0.0304 secs
Requests/sec:	242.5708
Latency distribution:
10% in 0.0204 secs
25% in 0.0234 secs
50% in 0.0262 secs
75% in 0.0365 secs
90% in 0.0497 secs
95% in 0.0531 secs
99% in 0.0628 secs

cache get_team_name and is_authorized_dag in fab auth manager used in requires_access_dag check and _get_serdag.

Summary:
Total:	0.6000 secs
Slowest:	0.0532 secs
Fastest:	0.0100 secs
Average:	0.0249 secs
Requests/sec:	333.3144
Latency distribution:
10% in 0.0176 secs
25% in 0.0198 secs
50% in 0.0240 secs
75% in 0.0305 secs
90% in 0.0341 secs
95% in 0.0367 secs
99% in 0.0402 secs

related: #61485

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

No

requires_access_dag is used in the ti_summary endpoint. When this
method returns a result then this can be cached and reused for other
API calls since access to a dag doesn't change based on taskinstance.
@tirkarthi

Copy link
Copy Markdown
ContributorAuthor

As per my understanding there are only 5 connections in the pool by default and with concurrent requests and db calls being made synchronously in requires_access_dag and in the endpoint other requests might need to wait based on max_overflow.

Prompt : If I have connection pool as 5 in sqlalchemy and create 10 sessions concurrently then do the last 5 sessions wait for the initial 5 sessions to be returned back to the pool Gemini output : The short answer is: not necessarily. It depends entirely on your max_overflow setting. By default, SQLAlchemy doesn't just make those extra 5 sessions sit in a queue; it often creates "overflow" connections to handle the spike.
Here is the breakdown of how SQLAlchemy manages this scenario:
1. The pool_size vs. max_overflow
When you define a pool, SQLAlchemy uses two primary parameters to determine how to handle concurrent requests:
pool_size (5 in your case): The number of "permanent" connections kept open in the pool.
max_overflow (Default is 10): The number of additional connections the pool is allowed to create if the pool_size is exhausted.
2. What happens to your 10 sessions?
If you are using the default settings (pool_size=5, max_overflow=10):
Sessions 1–5: Grab the 5 "checked-in" connections from the pool immediately.
Sessions 6–10: Since the pool is full, the engine looks at max_overflow. Because 5 is less than the default limit of 10, it creates 5 new temporary connections.
Result: All 10 sessions run concurrently. No one waits.
3. When do sessions actually wait?
The "waiting" behavior only kicks in if you exceed pool_size + max_overflow.
If you set max_overflow=0 or if you tried to open 16 sessions with default settings:
The 16th session would hit the limit.
It would wait for a connection to be returned to the pool.
This wait is governed by pool_timeout (default is 30 seconds). If no connection becomes free in that time, SQLAlchemy throws a TimeoutError

@pierrejeambrunpierrejeambrun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should think about the cache invalidation cycle too.

For instance if I update the permissions of the user the is_authorized_dag cache isn't invalidated, and therefore the user won't have access until cache expires (I don't know when in the current implementation)

Also different workers can have different version of the cache in these specific case, one worker might grant access, while the other can block the access, I think this is a problem.

@tirkarthi

Copy link
Copy Markdown
ContributorAuthor

Thanks @pierrejeambrun . I updated the signature for the caching key and still see failures about the permission which I assume is about caching invalidation. The primary intention of the PR was around recurring checks for dag access around the same time within 1-2 seconds the grid loads. I will see if I can bring up an approach or to have a shorter ttl like 1-2 seconds within the grid loading cycle.

@pierrejeambrunpierrejeambrun mentioned this pull request Feb 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We normally prefer core changes and provider changes to be in separated PRs.
Is there a reason to keep both changes together?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we probably need to split the team vs dag_access caching. This will be easier for cherry picking.

@pierrejeambrunpierrejeambrun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The primary intention of the PR was around recurring checks for dag access around the same time within 1-2 seconds the grid loads. I will see if I can bring up an approach or to have a shorter ttl like 1-2 seconds within the grid loading cycle.

I see, there's a 30 seconds TTL, that could work yes. Invalidation is automatically done. We just need to fix the CI. Maybe keep this one about Core and create a new one for fab provider change.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we probably need to split the team vs dag_access caching. This will be easier for cherry picking.

@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

@github-actionsgithub-actionsBot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Apr 25, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providersprovider:fabstaleStale PRs per the .github/workflows/stale.yml policy file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@tirkarthi@pierrejeambrun@eladkal
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Cache dag access and teams check to be reused in grid ti_summary API calls - #61623

Closed
tirkarthi wants to merge 1 commit into
apache:mainfrom
tirkarthi:gh61485
Closed

Cache dag access and teams check to be reused in grid ti_summary API calls#61623
tirkarthi wants to merge 1 commit into
apache:mainfrom
tirkarthi:gh61485

Conversation

@tirkarthi

@tirkarthitirkarthi commented Feb 8, 2026

Copy link
Copy Markdown
Contributor

requires_access_dag is used in the ti_summary endpoint. When this method returns a result then this can be cached and reused for other API calls since access to a dag doesn't change based on taskinstance. On a similar note for dags that don't have many changes the serialized dag entry also remains the same.

This PR adds caching to airflow-core which is independent from fab provider. Hence the fab related ttl cannot be always used and this might need a new config if the approach is accepted.

command with 10 concurrent requests since the grid loads 10 dagruns by default.

hey -c 10 -H 'Cookie: _token=<token>' 'http://localhost:8000/ui/grid/ti_summaries/asset_produces_2/manual__2026-01-31T06:15:30.690694+00:00'

Main branch

Summary:
Total:	1.0294 secs
Slowest:	0.0801 secs
Fastest:	0.0160 secs
Average:	0.0460 secs
Requests/sec:	194.2854
Latency distribution:
10% in 0.0335 secs
25% in 0.0385 secs
50% in 0.0438 secs
75% in 0.0537 secs
90% in 0.0626 secs
95% in 0.0667 secs
99% in 0.0758 secs

cache only is_authorized_dag in fab auth manager used in requires_access_dag check

Summary:
Total:	0.9620 secs
Slowest:	0.0741 secs
Fastest:	0.0163 secs
Average:	0.0410 secs
Requests/sec:	207.8980
Latency distribution:
10% in 0.0295 secs
25% in 0.0328 secs
50% in 0.0403 secs
75% in 0.0496 secs
90% in 0.0537 secs
95% in 0.0560 secs
99% in 0.0630 secs

cache get_team_name and is_authorized_dag in fab auth manager used in requires_access_dag check

Summary:
Total:	0.8245 secs
Slowest:	0.0813 secs
Fastest:	0.0115 secs
Average:	0.0304 secs
Requests/sec:	242.5708
Latency distribution:
10% in 0.0204 secs
25% in 0.0234 secs
50% in 0.0262 secs
75% in 0.0365 secs
90% in 0.0497 secs
95% in 0.0531 secs
99% in 0.0628 secs

cache get_team_name and is_authorized_dag in fab auth manager used in requires_access_dag check and _get_serdag.

Summary:
Total:	0.6000 secs
Slowest:	0.0532 secs
Fastest:	0.0100 secs
Average:	0.0249 secs
Requests/sec:	333.3144
Latency distribution:
10% in 0.0176 secs
25% in 0.0198 secs
50% in 0.0240 secs
75% in 0.0305 secs
90% in 0.0341 secs
95% in 0.0367 secs
99% in 0.0402 secs

related: #61485

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

No

requires_access_dag is used in the ti_summary endpoint. When this
method returns a result then this can be cached and reused for other
API calls since access to a dag doesn't change based on taskinstance.
@tirkarthi

Copy link
Copy Markdown
ContributorAuthor

As per my understanding there are only 5 connections in the pool by default and with concurrent requests and db calls being made synchronously in requires_access_dag and in the endpoint other requests might need to wait based on max_overflow.

Prompt : If I have connection pool as 5 in sqlalchemy and create 10 sessions concurrently then do the last 5 sessions wait for the initial 5 sessions to be returned back to the pool Gemini output : The short answer is: not necessarily. It depends entirely on your max_overflow setting. By default, SQLAlchemy doesn't just make those extra 5 sessions sit in a queue; it often creates "overflow" connections to handle the spike.
Here is the breakdown of how SQLAlchemy manages this scenario:
1. The pool_size vs. max_overflow
When you define a pool, SQLAlchemy uses two primary parameters to determine how to handle concurrent requests:
pool_size (5 in your case): The number of "permanent" connections kept open in the pool.
max_overflow (Default is 10): The number of additional connections the pool is allowed to create if the pool_size is exhausted.
2. What happens to your 10 sessions?
If you are using the default settings (pool_size=5, max_overflow=10):
Sessions 1–5: Grab the 5 "checked-in" connections from the pool immediately.
Sessions 6–10: Since the pool is full, the engine looks at max_overflow. Because 5 is less than the default limit of 10, it creates 5 new temporary connections.
Result: All 10 sessions run concurrently. No one waits.
3. When do sessions actually wait?
The "waiting" behavior only kicks in if you exceed pool_size + max_overflow.
If you set max_overflow=0 or if you tried to open 16 sessions with default settings:
The 16th session would hit the limit.
It would wait for a connection to be returned to the pool.
This wait is governed by pool_timeout (default is 30 seconds). If no connection becomes free in that time, SQLAlchemy throws a TimeoutError

@pierrejeambrunpierrejeambrun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should think about the cache invalidation cycle too.

For instance if I update the permissions of the user the is_authorized_dag cache isn't invalidated, and therefore the user won't have access until cache expires (I don't know when in the current implementation)

Also different workers can have different version of the cache in these specific case, one worker might grant access, while the other can block the access, I think this is a problem.

@tirkarthi

Copy link
Copy Markdown
ContributorAuthor

Thanks @pierrejeambrun . I updated the signature for the caching key and still see failures about the permission which I assume is about caching invalidation. The primary intention of the PR was around recurring checks for dag access around the same time within 1-2 seconds the grid loads. I will see if I can bring up an approach or to have a shorter ttl like 1-2 seconds within the grid loading cycle.

@pierrejeambrunpierrejeambrun mentioned this pull request Feb 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We normally prefer core changes and provider changes to be in separated PRs.
Is there a reason to keep both changes together?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we probably need to split the team vs dag_access caching. This will be easier for cherry picking.

@pierrejeambrunpierrejeambrun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The primary intention of the PR was around recurring checks for dag access around the same time within 1-2 seconds the grid loads. I will see if I can bring up an approach or to have a shorter ttl like 1-2 seconds within the grid loading cycle.

I see, there's a 30 seconds TTL, that could work yes. Invalidation is automatically done. We just need to fix the CI. Maybe keep this one about Core and create a new one for fab provider change.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we probably need to split the team vs dag_access caching. This will be easier for cherry picking.

@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

@github-actionsgithub-actionsBot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Apr 25, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providersprovider:fabstaleStale PRs per the .github/workflows/stale.yml policy file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@tirkarthi@pierrejeambrun@eladkal
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Cache dag access and teams check to be reused in grid ti_summary API calls - #61623

Closed
tirkarthi wants to merge 1 commit into
apache:mainfrom
tirkarthi:gh61485
Closed

Cache dag access and teams check to be reused in grid ti_summary API calls#61623
tirkarthi wants to merge 1 commit into
apache:mainfrom
tirkarthi:gh61485

Conversation

@tirkarthi

@tirkarthitirkarthi commented Feb 8, 2026

Copy link
Copy Markdown
Contributor

requires_access_dag is used in the ti_summary endpoint. When this method returns a result then this can be cached and reused for other API calls since access to a dag doesn't change based on taskinstance. On a similar note for dags that don't have many changes the serialized dag entry also remains the same.

This PR adds caching to airflow-core which is independent from fab provider. Hence the fab related ttl cannot be always used and this might need a new config if the approach is accepted.

command with 10 concurrent requests since the grid loads 10 dagruns by default.

hey -c 10 -H 'Cookie: _token=<token>' 'http://localhost:8000/ui/grid/ti_summaries/asset_produces_2/manual__2026-01-31T06:15:30.690694+00:00'

Main branch

Summary:
Total:	1.0294 secs
Slowest:	0.0801 secs
Fastest:	0.0160 secs
Average:	0.0460 secs
Requests/sec:	194.2854
Latency distribution:
10% in 0.0335 secs
25% in 0.0385 secs
50% in 0.0438 secs
75% in 0.0537 secs
90% in 0.0626 secs
95% in 0.0667 secs
99% in 0.0758 secs

cache only is_authorized_dag in fab auth manager used in requires_access_dag check

Summary:
Total:	0.9620 secs
Slowest:	0.0741 secs
Fastest:	0.0163 secs
Average:	0.0410 secs
Requests/sec:	207.8980
Latency distribution:
10% in 0.0295 secs
25% in 0.0328 secs
50% in 0.0403 secs
75% in 0.0496 secs
90% in 0.0537 secs
95% in 0.0560 secs
99% in 0.0630 secs

cache get_team_name and is_authorized_dag in fab auth manager used in requires_access_dag check

Summary:
Total:	0.8245 secs
Slowest:	0.0813 secs
Fastest:	0.0115 secs
Average:	0.0304 secs
Requests/sec:	242.5708
Latency distribution:
10% in 0.0204 secs
25% in 0.0234 secs
50% in 0.0262 secs
75% in 0.0365 secs
90% in 0.0497 secs
95% in 0.0531 secs
99% in 0.0628 secs

cache get_team_name and is_authorized_dag in fab auth manager used in requires_access_dag check and _get_serdag.

Summary:
Total:	0.6000 secs
Slowest:	0.0532 secs
Fastest:	0.0100 secs
Average:	0.0249 secs
Requests/sec:	333.3144
Latency distribution:
10% in 0.0176 secs
25% in 0.0198 secs
50% in 0.0240 secs
75% in 0.0305 secs
90% in 0.0341 secs
95% in 0.0367 secs
99% in 0.0402 secs

related: #61485

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

No

requires_access_dag is used in the ti_summary endpoint. When this
method returns a result then this can be cached and reused for other
API calls since access to a dag doesn't change based on taskinstance.
@tirkarthi

Copy link
Copy Markdown
ContributorAuthor

As per my understanding there are only 5 connections in the pool by default and with concurrent requests and db calls being made synchronously in requires_access_dag and in the endpoint other requests might need to wait based on max_overflow.

Prompt : If I have connection pool as 5 in sqlalchemy and create 10 sessions concurrently then do the last 5 sessions wait for the initial 5 sessions to be returned back to the pool Gemini output : The short answer is: not necessarily. It depends entirely on your max_overflow setting. By default, SQLAlchemy doesn't just make those extra 5 sessions sit in a queue; it often creates "overflow" connections to handle the spike.
Here is the breakdown of how SQLAlchemy manages this scenario:
1. The pool_size vs. max_overflow
When you define a pool, SQLAlchemy uses two primary parameters to determine how to handle concurrent requests:
pool_size (5 in your case): The number of "permanent" connections kept open in the pool.
max_overflow (Default is 10): The number of additional connections the pool is allowed to create if the pool_size is exhausted.
2. What happens to your 10 sessions?
If you are using the default settings (pool_size=5, max_overflow=10):
Sessions 1–5: Grab the 5 "checked-in" connections from the pool immediately.
Sessions 6–10: Since the pool is full, the engine looks at max_overflow. Because 5 is less than the default limit of 10, it creates 5 new temporary connections.
Result: All 10 sessions run concurrently. No one waits.
3. When do sessions actually wait?
The "waiting" behavior only kicks in if you exceed pool_size + max_overflow.
If you set max_overflow=0 or if you tried to open 16 sessions with default settings:
The 16th session would hit the limit.
It would wait for a connection to be returned to the pool.
This wait is governed by pool_timeout (default is 30 seconds). If no connection becomes free in that time, SQLAlchemy throws a TimeoutError

@pierrejeambrunpierrejeambrun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should think about the cache invalidation cycle too.

For instance if I update the permissions of the user the is_authorized_dag cache isn't invalidated, and therefore the user won't have access until cache expires (I don't know when in the current implementation)

Also different workers can have different version of the cache in these specific case, one worker might grant access, while the other can block the access, I think this is a problem.

@tirkarthi

Copy link
Copy Markdown
ContributorAuthor

Thanks @pierrejeambrun . I updated the signature for the caching key and still see failures about the permission which I assume is about caching invalidation. The primary intention of the PR was around recurring checks for dag access around the same time within 1-2 seconds the grid loads. I will see if I can bring up an approach or to have a shorter ttl like 1-2 seconds within the grid loading cycle.

@pierrejeambrunpierrejeambrun mentioned this pull request Feb 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We normally prefer core changes and provider changes to be in separated PRs.
Is there a reason to keep both changes together?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we probably need to split the team vs dag_access caching. This will be easier for cherry picking.

@pierrejeambrunpierrejeambrun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The primary intention of the PR was around recurring checks for dag access around the same time within 1-2 seconds the grid loads. I will see if I can bring up an approach or to have a shorter ttl like 1-2 seconds within the grid loading cycle.

I see, there's a 30 seconds TTL, that could work yes. Invalidation is automatically done. We just need to fix the CI. Maybe keep this one about Core and create a new one for fab provider change.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we probably need to split the team vs dag_access caching. This will be easier for cherry picking.

@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

@github-actionsgithub-actionsBot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Apr 25, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providersprovider:fabstaleStale PRs per the .github/workflows/stale.yml policy file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@tirkarthi@pierrejeambrun@eladkal
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Cache dag access and teams check to be reused in grid ti_summary API calls - #61623

Closed
tirkarthi wants to merge 1 commit into
apache:mainfrom
tirkarthi:gh61485
Closed

Cache dag access and teams check to be reused in grid ti_summary API calls#61623
tirkarthi wants to merge 1 commit into
apache:mainfrom
tirkarthi:gh61485

Conversation

@tirkarthi

@tirkarthitirkarthi commented Feb 8, 2026

Copy link
Copy Markdown
Contributor

requires_access_dag is used in the ti_summary endpoint. When this method returns a result then this can be cached and reused for other API calls since access to a dag doesn't change based on taskinstance. On a similar note for dags that don't have many changes the serialized dag entry also remains the same.

This PR adds caching to airflow-core which is independent from fab provider. Hence the fab related ttl cannot be always used and this might need a new config if the approach is accepted.

command with 10 concurrent requests since the grid loads 10 dagruns by default.

hey -c 10 -H 'Cookie: _token=<token>' 'http://localhost:8000/ui/grid/ti_summaries/asset_produces_2/manual__2026-01-31T06:15:30.690694+00:00'

Main branch

Summary:
Total:	1.0294 secs
Slowest:	0.0801 secs
Fastest:	0.0160 secs
Average:	0.0460 secs
Requests/sec:	194.2854
Latency distribution:
10% in 0.0335 secs
25% in 0.0385 secs
50% in 0.0438 secs
75% in 0.0537 secs
90% in 0.0626 secs
95% in 0.0667 secs
99% in 0.0758 secs

cache only is_authorized_dag in fab auth manager used in requires_access_dag check

Summary:
Total:	0.9620 secs
Slowest:	0.0741 secs
Fastest:	0.0163 secs
Average:	0.0410 secs
Requests/sec:	207.8980
Latency distribution:
10% in 0.0295 secs
25% in 0.0328 secs
50% in 0.0403 secs
75% in 0.0496 secs
90% in 0.0537 secs
95% in 0.0560 secs
99% in 0.0630 secs

cache get_team_name and is_authorized_dag in fab auth manager used in requires_access_dag check

Summary:
Total:	0.8245 secs
Slowest:	0.0813 secs
Fastest:	0.0115 secs
Average:	0.0304 secs
Requests/sec:	242.5708
Latency distribution:
10% in 0.0204 secs
25% in 0.0234 secs
50% in 0.0262 secs
75% in 0.0365 secs
90% in 0.0497 secs
95% in 0.0531 secs
99% in 0.0628 secs

cache get_team_name and is_authorized_dag in fab auth manager used in requires_access_dag check and _get_serdag.

Summary:
Total:	0.6000 secs
Slowest:	0.0532 secs
Fastest:	0.0100 secs
Average:	0.0249 secs
Requests/sec:	333.3144
Latency distribution:
10% in 0.0176 secs
25% in 0.0198 secs
50% in 0.0240 secs
75% in 0.0305 secs
90% in 0.0341 secs
95% in 0.0367 secs
99% in 0.0402 secs

related: #61485

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

No

requires_access_dag is used in the ti_summary endpoint. When this
method returns a result then this can be cached and reused for other
API calls since access to a dag doesn't change based on taskinstance.
@tirkarthi

Copy link
Copy Markdown
ContributorAuthor

As per my understanding there are only 5 connections in the pool by default and with concurrent requests and db calls being made synchronously in requires_access_dag and in the endpoint other requests might need to wait based on max_overflow.

Prompt : If I have connection pool as 5 in sqlalchemy and create 10 sessions concurrently then do the last 5 sessions wait for the initial 5 sessions to be returned back to the pool Gemini output : The short answer is: not necessarily. It depends entirely on your max_overflow setting. By default, SQLAlchemy doesn't just make those extra 5 sessions sit in a queue; it often creates "overflow" connections to handle the spike.
Here is the breakdown of how SQLAlchemy manages this scenario:
1. The pool_size vs. max_overflow
When you define a pool, SQLAlchemy uses two primary parameters to determine how to handle concurrent requests:
pool_size (5 in your case): The number of "permanent" connections kept open in the pool.
max_overflow (Default is 10): The number of additional connections the pool is allowed to create if the pool_size is exhausted.
2. What happens to your 10 sessions?
If you are using the default settings (pool_size=5, max_overflow=10):
Sessions 1–5: Grab the 5 "checked-in" connections from the pool immediately.
Sessions 6–10: Since the pool is full, the engine looks at max_overflow. Because 5 is less than the default limit of 10, it creates 5 new temporary connections.
Result: All 10 sessions run concurrently. No one waits.
3. When do sessions actually wait?
The "waiting" behavior only kicks in if you exceed pool_size + max_overflow.
If you set max_overflow=0 or if you tried to open 16 sessions with default settings:
The 16th session would hit the limit.
It would wait for a connection to be returned to the pool.
This wait is governed by pool_timeout (default is 30 seconds). If no connection becomes free in that time, SQLAlchemy throws a TimeoutError

@pierrejeambrunpierrejeambrun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should think about the cache invalidation cycle too.

For instance if I update the permissions of the user the is_authorized_dag cache isn't invalidated, and therefore the user won't have access until cache expires (I don't know when in the current implementation)

Also different workers can have different version of the cache in these specific case, one worker might grant access, while the other can block the access, I think this is a problem.

@tirkarthi

Copy link
Copy Markdown
ContributorAuthor

Thanks @pierrejeambrun . I updated the signature for the caching key and still see failures about the permission which I assume is about caching invalidation. The primary intention of the PR was around recurring checks for dag access around the same time within 1-2 seconds the grid loads. I will see if I can bring up an approach or to have a shorter ttl like 1-2 seconds within the grid loading cycle.

@pierrejeambrunpierrejeambrun mentioned this pull request Feb 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We normally prefer core changes and provider changes to be in separated PRs.
Is there a reason to keep both changes together?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we probably need to split the team vs dag_access caching. This will be easier for cherry picking.

@pierrejeambrunpierrejeambrun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The primary intention of the PR was around recurring checks for dag access around the same time within 1-2 seconds the grid loads. I will see if I can bring up an approach or to have a shorter ttl like 1-2 seconds within the grid loading cycle.

I see, there's a 30 seconds TTL, that could work yes. Invalidation is automatically done. We just need to fix the CI. Maybe keep this one about Core and create a new one for fab provider change.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we probably need to split the team vs dag_access caching. This will be easier for cherry picking.

@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

@github-actionsgithub-actionsBot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Apr 25, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providersprovider:fabstaleStale PRs per the .github/workflows/stale.yml policy file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@tirkarthi@pierrejeambrun@eladkal
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Cache dag access and teams check to be reused in grid ti_summary API calls - #61623

Closed
tirkarthi wants to merge 1 commit into
apache:mainfrom
tirkarthi:gh61485
Closed

Cache dag access and teams check to be reused in grid ti_summary API calls#61623
tirkarthi wants to merge 1 commit into
apache:mainfrom
tirkarthi:gh61485

Conversation

@tirkarthi

@tirkarthitirkarthi commented Feb 8, 2026

Copy link
Copy Markdown
Contributor

requires_access_dag is used in the ti_summary endpoint. When this method returns a result then this can be cached and reused for other API calls since access to a dag doesn't change based on taskinstance. On a similar note for dags that don't have many changes the serialized dag entry also remains the same.

This PR adds caching to airflow-core which is independent from fab provider. Hence the fab related ttl cannot be always used and this might need a new config if the approach is accepted.

command with 10 concurrent requests since the grid loads 10 dagruns by default.

hey -c 10 -H 'Cookie: _token=<token>' 'http://localhost:8000/ui/grid/ti_summaries/asset_produces_2/manual__2026-01-31T06:15:30.690694+00:00'

Main branch

Summary:
Total:	1.0294 secs
Slowest:	0.0801 secs
Fastest:	0.0160 secs
Average:	0.0460 secs
Requests/sec:	194.2854
Latency distribution:
10% in 0.0335 secs
25% in 0.0385 secs
50% in 0.0438 secs
75% in 0.0537 secs
90% in 0.0626 secs
95% in 0.0667 secs
99% in 0.0758 secs

cache only is_authorized_dag in fab auth manager used in requires_access_dag check

Summary:
Total:	0.9620 secs
Slowest:	0.0741 secs
Fastest:	0.0163 secs
Average:	0.0410 secs
Requests/sec:	207.8980
Latency distribution:
10% in 0.0295 secs
25% in 0.0328 secs
50% in 0.0403 secs
75% in 0.0496 secs
90% in 0.0537 secs
95% in 0.0560 secs
99% in 0.0630 secs

cache get_team_name and is_authorized_dag in fab auth manager used in requires_access_dag check

Summary:
Total:	0.8245 secs
Slowest:	0.0813 secs
Fastest:	0.0115 secs
Average:	0.0304 secs
Requests/sec:	242.5708
Latency distribution:
10% in 0.0204 secs
25% in 0.0234 secs
50% in 0.0262 secs
75% in 0.0365 secs
90% in 0.0497 secs
95% in 0.0531 secs
99% in 0.0628 secs

cache get_team_name and is_authorized_dag in fab auth manager used in requires_access_dag check and _get_serdag.

Summary:
Total:	0.6000 secs
Slowest:	0.0532 secs
Fastest:	0.0100 secs
Average:	0.0249 secs
Requests/sec:	333.3144
Latency distribution:
10% in 0.0176 secs
25% in 0.0198 secs
50% in 0.0240 secs
75% in 0.0305 secs
90% in 0.0341 secs
95% in 0.0367 secs
99% in 0.0402 secs

related: #61485

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

No

requires_access_dag is used in the ti_summary endpoint. When this
method returns a result then this can be cached and reused for other
API calls since access to a dag doesn't change based on taskinstance.
@tirkarthi

Copy link
Copy Markdown
ContributorAuthor

As per my understanding there are only 5 connections in the pool by default and with concurrent requests and db calls being made synchronously in requires_access_dag and in the endpoint other requests might need to wait based on max_overflow.

Prompt : If I have connection pool as 5 in sqlalchemy and create 10 sessions concurrently then do the last 5 sessions wait for the initial 5 sessions to be returned back to the pool Gemini output : The short answer is: not necessarily. It depends entirely on your max_overflow setting. By default, SQLAlchemy doesn't just make those extra 5 sessions sit in a queue; it often creates "overflow" connections to handle the spike.
Here is the breakdown of how SQLAlchemy manages this scenario:
1. The pool_size vs. max_overflow
When you define a pool, SQLAlchemy uses two primary parameters to determine how to handle concurrent requests:
pool_size (5 in your case): The number of "permanent" connections kept open in the pool.
max_overflow (Default is 10): The number of additional connections the pool is allowed to create if the pool_size is exhausted.
2. What happens to your 10 sessions?
If you are using the default settings (pool_size=5, max_overflow=10):
Sessions 1–5: Grab the 5 "checked-in" connections from the pool immediately.
Sessions 6–10: Since the pool is full, the engine looks at max_overflow. Because 5 is less than the default limit of 10, it creates 5 new temporary connections.
Result: All 10 sessions run concurrently. No one waits.
3. When do sessions actually wait?
The "waiting" behavior only kicks in if you exceed pool_size + max_overflow.
If you set max_overflow=0 or if you tried to open 16 sessions with default settings:
The 16th session would hit the limit.
It would wait for a connection to be returned to the pool.
This wait is governed by pool_timeout (default is 30 seconds). If no connection becomes free in that time, SQLAlchemy throws a TimeoutError

@pierrejeambrunpierrejeambrun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should think about the cache invalidation cycle too.

For instance if I update the permissions of the user the is_authorized_dag cache isn't invalidated, and therefore the user won't have access until cache expires (I don't know when in the current implementation)

Also different workers can have different version of the cache in these specific case, one worker might grant access, while the other can block the access, I think this is a problem.

@tirkarthi

Copy link
Copy Markdown
ContributorAuthor

Thanks @pierrejeambrun . I updated the signature for the caching key and still see failures about the permission which I assume is about caching invalidation. The primary intention of the PR was around recurring checks for dag access around the same time within 1-2 seconds the grid loads. I will see if I can bring up an approach or to have a shorter ttl like 1-2 seconds within the grid loading cycle.

@pierrejeambrunpierrejeambrun mentioned this pull request Feb 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We normally prefer core changes and provider changes to be in separated PRs.
Is there a reason to keep both changes together?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we probably need to split the team vs dag_access caching. This will be easier for cherry picking.

@pierrejeambrunpierrejeambrun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The primary intention of the PR was around recurring checks for dag access around the same time within 1-2 seconds the grid loads. I will see if I can bring up an approach or to have a shorter ttl like 1-2 seconds within the grid loading cycle.

I see, there's a 30 seconds TTL, that could work yes. Invalidation is automatically done. We just need to fix the CI. Maybe keep this one about Core and create a new one for fab provider change.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we probably need to split the team vs dag_access caching. This will be easier for cherry picking.

@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

@github-actionsgithub-actionsBot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Apr 25, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providersprovider:fabstaleStale PRs per the .github/workflows/stale.yml policy file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@tirkarthi@pierrejeambrun@eladkal