You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2020/02/23 06:30:14 UTC
[GitHub] [airflow] mik-laj opened a new pull request #7510:
[AIRFLOW-6887][WIP] Do not check the state of fresh DAGRun
mik-laj opened a new pull request #7510: [AIRFLOW-6887][WIP] Do not check the state of fresh DAGRun
URL: https://github.com/apache/airflow/pull/7510
When I have following DAG File:
```python
from datetime import timedelta
from airflow.models import DAG
from airflow.operators.dummy_operator import DummyOperator
from airflow.utils.dates import days_ago
args = {
'owner': 'airflow',
'start_date': days_ago(3),
}
def create_dag(dag_number):
dag = DAG(
dag_id=f'perf_50_dag_dummy_tasks_{dag_number}_of_50', default_args=args,
schedule_interval=timedelta(minutes=30),
# schedule_interval=None,
dagrun_timeout=timedelta(minutes=60),
is_paused_upon_creation=False,
)
DummyOperator(
task_id='task_1_of_5',
dag=dag
)
return dag
for i in range(1, 200):
globals()[f"dag_{i}"] = create_dag(i)
```
I ran following code
```
from airflow.jobs.scheduler_job import DagFileProcessor
processor = DagFileProcessor([], log)
processor.process_file(DAG_FILE, None, pickle_dags=False)
```
I got the following values:
**Before:**
Query count: 1800
Average time 6104.482 ms
**After:**
Query count: 1601
Average time 5541.349 ms
**Diff**
Query count: 199 (-11%)
Average time 563 ms (-9%)
---
Issue link: WILL BE INSERTED BY [boring-cyborg](https://github.com/kaxil/boring-cyborg)
Make sure to mark the boxes below before creating PR: [x]
- [ ] Description above provides context of the change
- [ ] Commit message/PR title starts with `[AIRFLOW-NNNN]`. AIRFLOW-NNNN = JIRA ID<sup>*</sup>
- [ ] Unit tests coverage for changes (not needed for documentation changes)
- [ ] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
- [ ] Relevant documentation is updated including usage instructions.
- [ ] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
<sup>*</sup> For document-only changes commit message can start with `[AIRFLOW-XXXX]`.
---
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+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 [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj edited a comment on issue #7510:
[AIRFLOW-6887][DONT-MERGE] Do not check the state of fresh DAGRun
Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on issue #7510: [AIRFLOW-6887][DONT-MERGE] Do not check the state of fresh DAGRun
URL: https://github.com/apache/airflow/pull/7510#issuecomment-591381280
@Fokko This PR is okay. I wanted more people to have a chance to review because it's is important PR, so I added "DONT-MERGE" in the title.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] ashb commented on issue #7510: [AIRFLOW-6887][WIP] Do not
check the state of fresh DAGRun
Posted by GitBox <gi...@apache.org>.
ashb commented on issue #7510: [AIRFLOW-6887][WIP] Do not check the state of fresh DAGRun
URL: https://github.com/apache/airflow/pull/7510#issuecomment-590453194
What script are you using to get the numbers btw @mik-laj?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on issue #7510: [AIRFLOW-6887][WIP] Do
not check the state of fresh DAGRun
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #7510: [AIRFLOW-6887][WIP] Do not check the state of fresh DAGRun
URL: https://github.com/apache/airflow/pull/7510#issuecomment-590472486
These part of code is most important:
```
@timing(RETRY)
@retry(RETRY)
@timing()
@count_queries
def slow_case():
from airflow.models import DagRun
session.query(DagRun).delete()
from airflow.models import TaskInstance
session.query(TaskInstance).delete()
session.commit()
processor.process_file(DAG_FILE, None, pickle_dags=False)
slow_case()
```
and I ran this code in 90% of cases.
@count_queries - count queries in block and displays on the screen
@timing - display executon time
@retry(10) - runs functions in loop 10 times
@timing(10) - display executon time but divides the result by 10.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on issue #7510:
[AIRFLOW-6887][DONT-MERGE] Do not check the state of fresh DAGRun
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #7510: [AIRFLOW-6887][DONT-MERGE] Do not check the state of fresh DAGRun
URL: https://github.com/apache/airflow/pull/7510#issuecomment-591380876
@ashb I added new rule to Github bot. https://github.com/apache/airflow/pull/7543
Draft PR does not work on GH. If you create a normal PR then you can't convert it to a draft.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] ashb commented on issue #7510: [AIRFLOW-6887][DONT-MERGE]
Do not check the state of fresh DAGRun
Posted by GitBox <gi...@apache.org>.
ashb commented on issue #7510: [AIRFLOW-6887][DONT-MERGE] Do not check the state of fresh DAGRun
URL: https://github.com/apache/airflow/pull/7510#issuecomment-591363625
@Fokko The title said "DONT-MERGE" :)
@mik-laj Do we need to revert this one? Perhaps you should be creating Draft PRs instead https://github.blog/2019-02-14-introducing-draft-pull-requests/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] ashb commented on issue #7510: [AIRFLOW-6887][DONT-MERGE]
Do not check the state of fresh DAGRun
Posted by GitBox <gi...@apache.org>.
ashb commented on issue #7510: [AIRFLOW-6887][DONT-MERGE] Do not check the state of fresh DAGRun
URL: https://github.com/apache/airflow/pull/7510#issuecomment-591367254
You picked a good PR to mess up on. This one is okay.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io commented on issue #7510: [AIRFLOW-6887][WIP]
Do not check the state of fresh DAGRun
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #7510: [AIRFLOW-6887][WIP] Do not check the state of fresh DAGRun
URL: https://github.com/apache/airflow/pull/7510#issuecomment-590034803
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7510?src=pr&el=h1) Report
> Merging [#7510](https://codecov.io/gh/apache/airflow/pull/7510?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/61a8bb65818521ccbb846e647103535b3e36b26d?src=pr&el=desc) will **decrease** coverage by `0.29%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/airflow/pull/7510?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7510 +/- ##
=========================================
- Coverage 86.82% 86.53% -0.3%
=========================================
Files 891 891
Lines 42095 42094 -1
=========================================
- Hits 36549 36425 -124
- Misses 5546 5669 +123
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7510?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/models/dag.py](https://codecov.io/gh/apache/airflow/pull/7510/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvZGFnLnB5) | `91.28% <ø> (-0.02%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7510/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7510/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/7510/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `47.18% <0%> (-45.08%)` | :arrow_down: |
| [...viders/cncf/kubernetes/operators/kubernetes\_pod.py](https://codecov.io/gh/apache/airflow/pull/7510/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvY25jZi9rdWJlcm5ldGVzL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZC5weQ==) | `69.38% <0%> (-25.52%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/7510/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [airflow/jobs/backfill\_job.py](https://codecov.io/gh/apache/airflow/pull/7510/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL2JhY2tmaWxsX2pvYi5weQ==) | `90.98% <0%> (-1.13%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7510?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7510?src=pr&el=footer). Last update [61a8bb6...aa07d56](https://codecov.io/gh/apache/airflow/pull/7510?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] Fokko merged pull request #7510:
[AIRFLOW-6887][DONT-MERGE] Do not check the state of fresh DAGRun
Posted by GitBox <gi...@apache.org>.
Fokko merged pull request #7510: [AIRFLOW-6887][DONT-MERGE] Do not check the state of fresh DAGRun
URL: https://github.com/apache/airflow/pull/7510
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj edited a comment on issue #7510:
[AIRFLOW-6887][WIP] Do not check the state of fresh DAGRun
Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on issue #7510: [AIRFLOW-6887][WIP] Do not check the state of fresh DAGRun
URL: https://github.com/apache/airflow/pull/7510#issuecomment-590472486
These part of code is most important:
```
@timing(RETRY)
@retry(RETRY)
@timing()
@count_queries
def slow_case():
from airflow.models import DagRun
session.query(DagRun).delete()
from airflow.models import TaskInstance
session.query(TaskInstance).delete()
session.commit()
processor.process_file(DAG_FILE, None, pickle_dags=False)
slow_case()
```
and I ran this code in 90% of cases.
@count_queries - count queries in block and displays on the screen
@timing() - display executon time
@retry(10) - runs functions in loop 10 times
@timing(10) - display executon time but divides the result by 10.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] ashb edited a comment on issue #7510:
[AIRFLOW-6887][DONT-MERGE] Do not check the state of fresh DAGRun
Posted by GitBox <gi...@apache.org>.
ashb edited a comment on issue #7510: [AIRFLOW-6887][DONT-MERGE] Do not check the state of fresh DAGRun
URL: https://github.com/apache/airflow/pull/7510#issuecomment-591363625
@Fokko The title said "DONT-MERGE" :)
@mik-laj Do we need to revert this one? Perhaps you should be creating Draft PRs instead https://github.blog/2019-02-14-introducing-draft-pull-requests/ (annoyingly you can't make a PR draft if you open it in the wrong mode, you have to open it as a draft one.)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on issue #7510: [AIRFLOW-6887][WIP] Do
not check the state of fresh DAGRun
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #7510: [AIRFLOW-6887][WIP] Do not check the state of fresh DAGRun
URL: https://github.com/apache/airflow/pull/7510#issuecomment-590470029
You're right. I plan to write a blogspot to describe exactly the measurement methodology.
If you want to get the same numbers as in my script then you need to run it using the Python interpreter.
```
python script-name.py
```
After then you will message similar to the following:
```
['', '/opt/airflow', '/usr/local/lib/python37.zip', '/usr/local/lib/python3.7', '/usr/local/lib/python3.7/lib-dynload', '/usr/local/lib/python3.7/site-packages', '/root/airflow/dags', '/root/airflow/config', '/root/airflow/plugins']
[2020-02-24 16:06:14,377] {dagbag.py:376} INFO - Filling up the DagBag from /files/dags/50_dag_5_dummy_tasks.py
[2020-02-24 16:06:15,281] {dag.py:1478} INFO - Sync 299 DAGs
Query count: 7
Retry took 1157.678 ms
[2020-02-24 16:06:15,537] {dagbag.py:376} INFO - Filling up the DagBag from /files/dags/50_dag_5_dummy_tasks.py
[2020-02-24 16:06:15,825] {dag.py:1478} INFO - Sync 299 DAGs
Query count: 7
Retry took 473.342 ms
[2020-02-24 16:06:16,004] {dagbag.py:376} INFO - Filling up the DagBag from /files/dags/50_dag_5_dummy_tasks.py
[2020-02-24 16:06:16,296] {dag.py:1478} INFO - Sync 299 DAGs
Query count: 7
Retry took 570.475 ms
[2020-02-24 16:06:16,579] {dagbag.py:376} INFO - Filling up the DagBag from /files/dags/50_dag_5_dummy_tasks.py
[2020-02-24 16:06:16,898] {dag.py:1478} INFO - Sync 299 DAGs
Query count: 7
Retry took 543.631 ms
[2020-02-24 16:06:17,119] {dagbag.py:376} INFO - Filling up the DagBag from /files/dags/50_dag_5_dummy_tasks.py
[2020-02-24 16:06:17,404] {dag.py:1478} INFO - Sync 299 DAGs
Query count: 7
Retry took 779.900 ms
[2020-02-24 16:06:17,907] {dagbag.py:376} INFO - Filling up the DagBag from /files/dags/50_dag_5_dummy_tasks.py
[2020-02-24 16:06:18,279] {dag.py:1478} INFO - Sync 299 DAGs
Query count: 7
Retry took 572.046 ms
[2020-02-24 16:06:18,473] {dagbag.py:376} INFO - Filling up the DagBag from /files/dags/50_dag_5_dummy_tasks.py
[2020-02-24 16:06:18,764] {dag.py:1478} INFO - Sync 299 DAGs
Query count: 7
Retry took 465.645 ms
[2020-02-24 16:06:18,938] {dagbag.py:376} INFO - Filling up the DagBag from /files/dags/50_dag_5_dummy_tasks.py
[2020-02-24 16:06:19,360] {dag.py:1478} INFO - Sync 299 DAGs
Query count: 7
Retry took 606.974 ms
[2020-02-24 16:06:19,544] {dagbag.py:376} INFO - Filling up the DagBag from /files/dags/50_dag_5_dummy_tasks.py
[2020-02-24 16:06:19,886] {dag.py:1478} INFO - Sync 299 DAGs
Query count: 7
Retry took 543.896 ms
[2020-02-24 16:06:20,088] {dagbag.py:376} INFO - Filling up the DagBag from /files/dags/50_dag_5_dummy_tasks.py
[2020-02-24 16:06:20,468] {dag.py:1478} INFO - Sync 299 DAGs
Query count: 7
Retry took 573.048 ms
Retry took 628.801 ms
```
The numbers seem self-explanatory except for the last value. This is the average of all try. If the script does not work then you may not have the DAG file. You should create it in the following file: /files/dags/50_dag_5_dummy_tasks.py
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] ashb commented on issue #7510: [AIRFLOW-6887][WIP] Do not
check the state of fresh DAGRun
Posted by GitBox <gi...@apache.org>.
ashb commented on issue #7510: [AIRFLOW-6887][WIP] Do not check the state of fresh DAGRun
URL: https://github.com/apache/airflow/pull/7510#issuecomment-591112184
@Fokko `run.refresh_from_db()` just loads the id and state columns. The case you are talking about is still handled correctly (by the sligtly badly named `verify_integrity` function.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on issue #7510: [AIRFLOW-6887][WIP] Do
not check the state of fresh DAGRun
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #7510: [AIRFLOW-6887][WIP] Do not check the state of fresh DAGRun
URL: https://github.com/apache/airflow/pull/7510#issuecomment-590467263
> @ashb code that is part of....the Airflow codebase.
> https://github.com/apache/airflow/blob/master/tests/test_utils/asserts.py#L37-L58
> but I also have other scripts that help me. Here is my notepad:
> https://gist.github.com/mik-laj/2f41e3e66b4331b8d79d90912e4e7c81
Yeah. It would be great to have those contributed with short instructions how to use them to get those numbers maybe ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] ashb commented on issue #7510: [AIRFLOW-6887][WIP] Do not
check the state of fresh DAGRun
Posted by GitBox <gi...@apache.org>.
ashb commented on issue #7510: [AIRFLOW-6887][WIP] Do not check the state of fresh DAGRun
URL: https://github.com/apache/airflow/pull/7510#issuecomment-590469732
Ah yeah, I have a similar notepad.
And which helper fn for instance in this case?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on issue #7510: [AIRFLOW-6887][WIP] Do
not check the state of fresh DAGRun
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #7510: [AIRFLOW-6887][WIP] Do not check the state of fresh DAGRun
URL: https://github.com/apache/airflow/pull/7510#issuecomment-590460156
@ashb code that is part of....the Airflow codebase.
https://github.com/apache/airflow/blob/master/tests/test_utils/asserts.py#L37-L58
but I also have other scripts that help me. Here is my notepad:
https://gist.github.com/mik-laj/2f41e3e66b4331b8d79d90912e4e7c81
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] Fokko commented on issue #7510:
[AIRFLOW-6887][DONT-MERGE] Do not check the state of fresh DAGRun
Posted by GitBox <gi...@apache.org>.
Fokko commented on issue #7510: [AIRFLOW-6887][DONT-MERGE] Do not check the state of fresh DAGRun
URL: https://github.com/apache/airflow/pull/7510#issuecomment-591366852
Ugh, didn't have my coffee yet. I can revert the commit if you like. @mik-laj please advise
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on issue #7510:
[AIRFLOW-6887][DONT-MERGE] Do not check the state of fresh DAGRun
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #7510: [AIRFLOW-6887][DONT-MERGE] Do not check the state of fresh DAGRun
URL: https://github.com/apache/airflow/pull/7510#issuecomment-591381280
@mik-laj This PR is okay. I wanted more people to have a chance to review because it's is important PR, so I added "DONT-MERGE" in the title.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services