You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "XD-DENG (via GitHub)" <gi...@apache.org> on 2024/02/28 17:43:25 UTC
[PR] Support adding custom TI Deps to help DagRun make more flexible TI scheduling decisions [airflow]
XD-DENG opened a new pull request, #37778:
URL: https://github.com/apache/airflow/pull/37778
This PR adds the ability to support adding custom TI Deps to allow more flexible TI scheduling.
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support adding custom TI Deps to help DagRun make more flexible TI scheduling decisions [airflow]
Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb commented on PR #37778:
URL: https://github.com/apache/airflow/pull/37778#issuecomment-1981466986
From the description though I'm not sure why this is need? Why can't the dep be set directly on the Operator object?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support adding custom TI Deps to help DagRun make more flexible TI scheduling decisions [airflow]
Posted by "XD-DENG (via GitHub)" <gi...@apache.org>.
XD-DENG commented on PR #37778:
URL: https://github.com/apache/airflow/pull/37778#issuecomment-1981784836
> From the description though I'm not sure why this is need? Why can't the dep be set directly on the Operator object?
Hi @ashb ,
The purpose of this change is to control if a TI is schedulable according to custom logics, in scheduler/executor.
Adding a custom TI Dep into the Operator object (via task policy) is possible, but it will not achieve the purpose and may result in unexpected side effect:
- the Scheduler/Executor does not check the custom TI deps when it makes decision whether a TI is schedulable or not.
- the custom TI Dep may be checked when a task is about to start, but that's in the Worker. This means the job may end up in a `fail`/`retry` status. This also means it's much less efficient in `KubernetesExecutor` (because the deps will be check when the worker pod is already launched)
- it may also result in Task Deadlock (I shared more details in [my earlier email int he mailing list dicussion](https://lists.apache.org/thread/wbn5fchopycv52f3obzpv6zc5wtrdp63))
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support adding custom TI Deps to help DagRun make more flexible TI scheduling decisions [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on PR #37778:
URL: https://github.com/apache/airflow/pull/37778#issuecomment-1992322204
A simple example on how this can be used (that cannot be covered otherwise) would be useful.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support adding custom TI Deps to help DagRun make more flexible TI scheduling decisions [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #37778:
URL: https://github.com/apache/airflow/pull/37778#issuecomment-1979527866
For visibility this PR is related to dev list discussion[ Idea for Discussion: custom TI dependencies](https://lists.apache.org/thread/0mxyx8kvbl7bvhth099zdrqqcdt0tp25)
@XD-DENG better to add also docs to explain the benefit and how to use this feature (we can also add warnings about performance)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support adding custom TI Deps to help DagRun make more flexible TI scheduling decisions [airflow]
Posted by "XD-DENG (via GitHub)" <gi...@apache.org>.
XD-DENG commented on PR #37778:
URL: https://github.com/apache/airflow/pull/37778#issuecomment-1981430720
Thanks @eladkal . I purposely avoided adding doc for this, per @ashb 's suggestion at the mailing list discussion.
In addition, adding custom Dep class via Plugin (which is already supported earlier) was not documented either. I assume it was by purpose too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support adding custom TI Deps to help DagRun make more flexible TI scheduling decisions [airflow]
Posted by "XD-DENG (via GitHub)" <gi...@apache.org>.
XD-DENG commented on PR #37778:
URL: https://github.com/apache/airflow/pull/37778#issuecomment-1995139815
> A simple example on how this can be used (that cannot be covered otherwise) would be useful.
Hi @uranusjr , sure thing, let me try to elaborate a bit more and get your opinions/inputs on this.
In different scenarios, we realize our TIs may have different extra dependencies. For example,
- Scenario 1: The TI may have to use separate hardware resource. It can be external GPU/acceleration hardware, other than CPU/memory resource (this is literally similar to the built-in Dep `PoolSlotsAvailableDep` or `DagTISlotsAvailableDep`).
- Scenario 2: the TI is only supposed to start when an event is identified (this event may be checked via an API, but it's not present as a `Dataset` in Airflow, and adding Operator to describe the dependency here is not desirable)
- Scenario 3: Among all the TIs (they may not belong to the same DAG), we may want them to be executed in certain customized order. By default, Airflow will execute the TIs in a somehow "FIFO" order, OR take the TI `Priority Weight` into consideration. But we are having a bigger idea in mind: based on the TIs' expected duration + the global concurrency we allow + resource availability, we may want to shuffle the execution order of the TIs, in order to achieve the best global efficiency.
The easiest way to achieve these ideas above, as far as my team can see, is to ensure we can add our custom TI Deps into the DagRun's TI scheduling decision making process.
I would love to hear how you think of this, or if you have any good alternative solution to share. Many thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support adding custom TI Deps to help DagRun make more flexible TI scheduling decisions [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #37778:
URL: https://github.com/apache/airflow/pull/37778#issuecomment-2016865953
Agree with TP that if we were to merge this one, we will need quite a bit more:
* documentation with some examples
* general use cases where it would be available
* some performance considerations (i.e. what those who create the custom deps should be aware of when writing their own deps
* dos and don'ts
* possibly some way of detecting when someone is doing bad things there (not sure how it should look like).
The big difference vs. what we have now with "built-in" deps, is that scheduler gives control to "user" code in a very, very "hot area" in the code. Unlike most of the other code we allow our users to configure in Airflow, this one wlll have a crucial and very serious impact on the scheduling performance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support adding custom TI Deps to help DagRun make more flexible TI scheduling decisions [airflow]
Posted by "XD-DENG (via GitHub)" <gi...@apache.org>.
XD-DENG commented on PR #37778:
URL: https://github.com/apache/airflow/pull/37778#issuecomment-1981787008
@ashb , the use case my team is having in mind is: we would want the DagRun to be scheduled/created as normal, but we would like to add custom logics to control if a TI under this DagRun should start. This is part of our works for "smarter orchestration".
The custom logics can be relevant to resource, job dependency, event dependency, etc.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support adding custom TI Deps to help DagRun make more flexible TI scheduling decisions [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on PR #37778:
URL: https://github.com/apache/airflow/pull/37778#issuecomment-2033363322
The scenarios do make sense to me (although No. 2 is a bit weak; we should probably instead improve Dataset and Triggerer to cover the use case instead). But in all scenarios we’d likely need a bit more than just user-provided deps, so it’s clear there’s a bigger picture behind the change. I would suggest communicate the end target directly first in a full document (either AIP or somewhere discussions can be gathered), that custom deps are a solution to.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support adding custom TI Deps to help DagRun make more flexible TI scheduling decisions [airflow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37778:
URL: https://github.com/apache/airflow/pull/37778#issuecomment-2125973183
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support adding custom TI Deps to help DagRun make more flexible TI scheduling decisions [airflow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #37778: Support adding custom TI Deps to help DagRun make more flexible TI scheduling decisions
URL: https://github.com/apache/airflow/pull/37778
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org