Repository navigation
Make finished TIs more lightweight when scanning task instances for scheduling - #71737
Conversation
Nataneljpwd
left a comment
There was a problem hiding this comment.
Looks good, hadn't had a look at the tests yet, there are questions that arose multiple times, I would appreciate if you could answer them
|
@Nataneljpwd Thank you for the review! I've addressed all your questions. |
edd3176 to
06a3588
Compare
06a3588 to
66da8df
Compare
24350a8 to
7644cf1
Compare
shahar1
left a comment
There was a problem hiding this comment.
Overall LGTM - could you please resolve all open threads if you resolved them?
|
@shahar1 Thank you for the review! I've addressed all the comments but the threads have been open without any activity for more than a month, so I resolved them. @Nataneljpwd If any of my answers don't make sense to you or you just disagree, add a comment to the corresponding thread and I will unresolve it. |
|
@ephraimbuddy / @uranusjr / @ashb - I'll appreciate a second look, thanks! |
|
@vatsrahul1001 has also worked on a similar improvement in the past. Rahul please take a look when you get a chance. Thanks! |
026dc2b to
bbeaf74
Compare
e52699e to
f3e3d8a
Compare
There was a problem hiding this comment.
Looks good to me. Finished task instances are read as lightweight rows while the unfinished ones stay full ORM objects, every consumer I traced only reads fields that FinishedTI has, and the Dag callbacks get a full TaskInstance back through the primary-key re-fetch.
3362392 to
5dee30e
Compare
|
Because of all the task changes lately, I re-ran the benchmark tests and results are still good. I also ran some tests with a small number of dag runs and tasks. The difference is negligible and it's actually in favor of the new code. The user won't know the difference but in the measurements, the total time is always 2-3 seconds less with the code in this PR. |
5dee30e to
e47d791
Compare
|
Removing the backport label for now as it's not worth including this in 3.4.0. We will backport it once .0 is released. |
1da0584 to
6b49ef1
Compare
6b49ef1 to
fe11402
Compare
|
@ashb I addressed all of your comments and I also re-ran all the benchmark tests. Results are still looking good. No issues from the new changes. Please take another look when you get a chance. Thanks! |
The goal of this PR is to increase scheduler performance.
Scheduler investigation
In our production environment, sometimes schedulers become very slow under load.
During my investigation, I added debug spans for every major step of the scheduler's iteration loop to help understand the issue. There is an open PR with the changes
#69809
I ran various tests and with the help of the debug spans from the PR, I was able to identify a possible bottleneck.
When we have multiple dags with a lot of tasks running in parallel, the scheduler spends most of the time scanning the task instances of the active dag runs to decide which can be queued. It actually looks for SCHEDULED tasks that can be moved to QUEUED state.
Once the tasks are moved to QUEUED state, then the worker can pick them up and it's no longer up to the scheduler.
The scan is consisted of 2 parts. The actual DB query that fetches the data and the SQL-alchemy part which hydrates ORM objects with the query results. Basically, it creates
taskinstanceobjects and fills them with data from each row that we get back. According to the spans, the query is very fast and the entire time is spent on the object hydration part.The code distinguishes between finished and unfinished tasks but only after the query. It fetches all of them from the DB and creates objects and then splits them into 2 separate lists.
Solution
Let's assume that we have a dag run with 1000 tasks. If 100 tasks have finished, then during the scan we hydrate 100 objects for finished tasks and 900 for unfinished. Similarly, if 400 tasks have finished, then we hydrate 400 objects for finished tasks and 600 for unfinished. As the dag run progresses we get more and more finished tasks.
For finished tasks, we don't really need full ORM objects because we only read 5 fields and we never modify them. Instead of creating a full
taskinstanceobject, we can hydrate a more lightweight immutable object with only the 5 necessary fields.Instead of having 1 query that fetches everything, creates objects and then splits them into 2 lists, we can have 2 queries, one for unfinished tasks that hydrates full objects and one for finished tasks that hydrates lightweight read-only objects.
There is a scenario with dag callbacks where we need the actual
taskinstanceobject in which case we fetch it from the DB. Still cheaper than the current approach.Testing
Setup
Config
Workload
The deferrable task doesn't have any special meaning. It just helps to make sure that most of the mapped tasks are running in parallel.
So for example, if the 1st task returns 10, the 1st stage will expand into 10 tasks. Once these 10 finish, then the 2nd expands into 10 tasks. Once, these finish then the 3rd expands into 10 tasks and so on. And this is running for 35 parallel dags.
I used 70 for the number of mapped tasks, which translates into 9stages * 70tasks * 35dags = 22050 tasks and another 35 deferred, 22085 in total.
The code from
maintakes 35 minutes to run the 22000 tasks, while the code from this patch took 28 minutes. The more the load, the bigger the difference.Here are some gathered metrics during the test. Left is the code from
mainand right is the code from this patch.Time needed for each scan (the PR improvement)
Before it reached up to 94ms while after it never exceeded 20ms.
Scheduler loop duration per iteration
Iterations became faster.
Scheduler vs worker vs worker slot utilization
Scheduler is working at 100% in both cases but before it was spending most of its resources on the ORM hydration and it wouldn't hand enough tasks to workers.
I've set max slot occupancy to 54. Before the change, at any given moment we would have up to 34% of the 54 slots occupied. After, we reach 100% occupancy and that didn't occur once but for most of the time.
Red line is the worker slot occupancy.
Time waiting for the scheduler vs workers
At the end, tasks end up waiting for workers instead of the scheduler.
Increased number of running tasks
Waiting on the scheduler (SCHEDULED -> QUEUED) vs waiting for the workers (QUEUED -> RUNNING)
Tasks wait less on the scheduler and more for a worker.
Same number of tasks, finish faster
Was generative AI tooling used to co-author this PR?
Claude code Opus 4.8
{pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.