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/03/07 21:30:48 UTC
[GitHub] [airflow] mik-laj opened a new pull request #7650:
[AIRFLOW-7008][WIP] Add perf kit with common used decorators/contexts
mik-laj opened a new pull request #7650: [AIRFLOW-7008][WIP] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650
---
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]
- [X] Description above provides context of the change
- [X] Commit message/PR title starts with `[AIRFLOW-NNNN]`. AIRFLOW-NNNN = JIRA ID<sup>*</sup>
- [X] Unit tests coverage for changes (not needed for documentation changes)
- [X] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
- [X] Relevant documentation is updated including usage instructions.
- [X] 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 commented on issue #7650: [AIRFLOW-7008] Add perf
kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #7650: [AIRFLOW-7008] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#issuecomment-613775703
@zhongjiajie I am afraid that this file will be too long if we add any information. In my opinion, the CONTRIBUTOR Guide should contain the most important information for beginners. Scheduler optimization is not a key task for these people. I would like to see a guidebook that describes more advanced information on the model of a contributor's guide, but advanced people are already quite familiar with the structure of the project. WDYT?
----------------------------------------------------------------
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] kaxil commented on a change in pull request #7650:
[AIRFLOW-7008] Add perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #7650: [AIRFLOW-7008] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#discussion_r408823923
##########
File path: scripts/perf/perf_kit/__init__.py
##########
@@ -0,0 +1,119 @@
+# 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.
+""""
+
+Perf-kit
+========
+
+Useful decorators and context managers used when testing the performance of various Airflow components.
+
+To use this package, you must add the parent directory to the ``PYTHONPATH`` environment variable. If the
+repository root directory is ``/opt/airflow/``, then you need to run the following command:
+
+.. code-block:: bash
+ export PYTHONPATH=/opt/airflow/script/perf
+
+Content
+=======
+The following decorators and context managers are included.
+
+.. autofunction:: perf_kit.memory.trace_memory
+
+.. autofunction:: perf_kit.python.pyspy
+
+.. autofunction:: perf_kit.python.profiled
+
+.. autofunction:: perf_kit.repeat_and_time.timing
+
+.. autofunction:: perf_kit.repeat_and_time.repeat
+
+.. autofunction:: perf_kit.repeat_and_time.timeout
+
+.. autofunction:: perf_kit.sqlalchemy.trace_queries
+
+.. autofunction:: perf_kit.sqlalchemy.count_queries
+
+Documentation for each function is provided in the function docstrings. Each module also has an example in
+the main section of the module.
+
+Examples
+========
+
+If you want to run an all example for ``perf_kit.sqlalchemy``, you can run the following command.
+
+.. code-block:: bash
+ python -m perf_kit.sqlalchemy
+
+If you want to know how to use these functions, it is worth to familiarize yourself with these examples.
+
+Use in tests
+============
+
+If you need it, you can easily test only one test using context manager.
+
+Suppose we have the following fragment of the file with tests.
+
+.. code-block:: python
+
+ prev = dag.previous_schedule(_next)
+ prev_local = local_tz.convert(prev)
+
+ self.assertEqual(prev_local.isoformat(), "2018-03-24T03:00:00+01:00")
+ self.assertEqual(prev.isoformat(), "2018-03-24T02:00:00+00:00")
+
+ def test_bulk_sync_to_db(self):
+ clear_db_dags()
+ dags = [
+ DAG(f'dag-bulk-sync-{i}', start_date=DEFAULT_DATE, tags=["test-dag"]) for i in range(0, 4)
+ ]
+
+ with assert_queries_count(3):
+ DAG.bulk_sync_to_db(dags)
+
+You can add a code snippet before the method definition, and then perform only one test and count the
+queries in it.
+
+.. code-block:: python
+
+ prev = dag.previous_schedule(_next)
+ prev_local = local_tz.convert(prev)
+
+ self.assertEqual(prev_local.isoformat(), "2018-03-24T03:00:00+01:00")
+ self.assertEqual(prev.isoformat(), "2018-03-24T02:00:00+00:00")
+
+ from perf_kit.sqlalchemy.sqlalchemy import trace_queries
+
+ @trace_queries
+ def test_bulk_sync_to_db(self):
+ clear_db_dags()
+ dags = [
+ DAG(f'dag-bulk-sync-{i}', start_date=DEFAULT_DATE, tags=["test-dag"]) for i in range(0, 4)
+ ]
+
+ with assert_queries_count(3):
+ DAG.bulk_sync_to_db(dags)
+
+To run the test, execute the command
+
+.. code-block:: bash
+ pytest tests.models.dag -k test_bulk_sync_to_db -s
+
+This is not a beautiful solution, but it allows you to easily check a random piece of code.
+
+Personally, I also have a separate file - a notebook in which I save various test cases and run them
+like a classic Python program.
Review comment:
Let's use the third person here or something like "Having a separate file to save various tests cases can be helpful"
----------------------------------------------------------------
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 a change in pull request #7650:
[AIRFLOW-7008] Add perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #7650: [AIRFLOW-7008] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#discussion_r408710764
##########
File path: scripts/perf/perf_kit/repeat_and_time.py
##########
@@ -0,0 +1,148 @@
+# 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.
+import contextlib
+import functools
+import math
+import random
+import signal
+import time
+
+
+class TimingResult:
+ def __init__(self):
+ self.start_time = 0
+ self.end_time = 0
+ self.value = 0
+
+
+@contextlib.contextmanager
+def timing(repeat_count: int = 1):
+ """
+ Measures code execution time.
+
+ :param repeat_count: If passed, the result will be divided by the value.
+ """
+ result = TimingResult()
+ result.start_time = time.monotonic()
+ try:
+ yield result
+ finally:
+ end_time = time.monotonic()
+ diff = (end_time - result.start_time) * 1000.0
+
+ result.end_time = end_time
+ if repeat_count == 1:
+ result.value = diff
+ print(f"Loop time: {diff:.3f} ms")
+ else:
+ average_time = diff / repeat_count
+ result.value = average_time
+ print(f"Average time: {average_time:.3f} ms")
+
+
+def repeat(repeat_count=5):
+ """
+ Function decorators that repeat function many times.
+
+ :param repeat_count: The repeat count
+ """
+
+ def repeat_decorator(f):
+ @functools.wraps(f)
+ def wrap(*args, **kwargs):
+ last_result = None
+ for i in range(repeat_count):
+ last_result = f(*args, **kwargs)
+ return last_result
+
+ return wrap
+
+ return repeat_decorator
+
+
+class TimeoutException(Exception):
+ pass
+
+
+@contextlib.contextmanager
+def timeout(seconds=1):
+ """
+ Executes code only limited seconds. If the code does not end during this time, it will be interrupted.
+
+ :param seconds: Number of seconds
+ """
+ def handle_timeout(signum, frame):
+ raise TimeoutException("Process timed out.")
+
+ try:
+ signal.signal(signal.SIGALRM, handle_timeout)
+ signal.alarm(seconds)
+ except ValueError:
+ raise Exception("timeout can't be used in the current context")
+
+ try:
+ yield
+ except TimeoutException:
+ print("Process timed out.")
+
+ finally:
+ try:
+ signal.alarm(0)
+ except ValueError:
+ raise Exception("timeout can't be used in the current context")
+
+
+if __name__ == "__main__":
+
+ def monte_carlo(total=10000):
+ # Monte Carlo
+ inside = 0
+
+ for i in range(0, total):
+ x2 = random.random() ** 2
+ y2 = random.random() ** 2
+ if math.sqrt(x2 + y2) < 1.0:
+ inside += 1
+
+ return (float(inside) / total) * 4
+
+ # Example 1:s
+ with timeout(1):
+ print("Sleep 5s with 1s timeout")
+ time.sleep(4)
+ print(":-/")
+
+ print()
+
+ # Example 2:
+ REPEAT_COUNT = 5
+
+ @timing(REPEAT_COUNT)
Review comment:
```suggestion
```
Timing is here twice - don't think you want this 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] potiuk commented on issue #7650: [AIRFLOW-7008][WIP] Add
perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #7650: [AIRFLOW-7008][WIP] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#issuecomment-596162412
Some static checks need to be fixed as well :)
----------------------------------------------------------------
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 #7650: [AIRFLOW-7008][WIP] Add
perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #7650: [AIRFLOW-7008][WIP] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#issuecomment-596162956
I want to add documentation in this PR, but for now I wanted to take the first step. This PR is still WIP.
----------------------------------------------------------------
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 edited a comment on issue #7650:
[AIRFLOW-7008] Add perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7650: [AIRFLOW-7008] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#issuecomment-596778149
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7650?src=pr&el=h1) Report
> Merging [#7650](https://codecov.io/gh/apache/airflow/pull/7650?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/45c8983306ab1c54abdacd8f870e790fad25cb37&el=desc) will **decrease** coverage by `0.76%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/airflow/pull/7650?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7650 +/- ##
==========================================
- Coverage 88.45% 87.68% -0.77%
==========================================
Files 937 940 +3
Lines 45234 45355 +121
==========================================
- Hits 40011 39770 -241
- Misses 5223 5585 +362
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7650?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.25% <0.00%> (-72.50%)` | :arrow_down: |
| [...w/providers/apache/hive/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2hpdmUvb3BlcmF0b3JzL215c3FsX3RvX2hpdmUucHk=) | `35.84% <0.00%> (-64.16%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0.00%> (-55.56%)` | :arrow_down: |
| [airflow/providers/postgres/operators/postgres.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvb3BlcmF0b3JzL3Bvc3RncmVzLnB5) | `47.82% <0.00%> (-52.18%)` | :arrow_down: |
| [airflow/providers/redis/operators/redis\_publish.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvb3BlcmF0b3JzL3JlZGlzX3B1Ymxpc2gucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0.00%> (-47.06%)` | :arrow_down: |
| [airflow/providers/mongo/sensors/mongo.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbW9uZ28vc2Vuc29ycy9tb25nby5weQ==) | `53.33% <0.00%> (-46.67%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `47.18% <0.00%> (-45.08%)` | :arrow_down: |
| [airflow/providers/mysql/operators/mysql.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbXlzcWwvb3BlcmF0b3JzL215c3FsLnB5) | `55.00% <0.00%> (-45.00%)` | :arrow_down: |
| [airflow/providers/redis/sensors/redis\_key.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvc2Vuc29ycy9yZWRpc19rZXkucHk=) | `61.53% <0.00%> (-38.47%)` | :arrow_down: |
| ... and [16 more](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7650?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/7650?src=pr&el=footer). Last update [45c8983...82205bc](https://codecov.io/gh/apache/airflow/pull/7650?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] ashb commented on a change in pull request #7650:
[AIRFLOW-7008][WIP] Add perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #7650: [AIRFLOW-7008][WIP] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#discussion_r389578441
##########
File path: scripts/perf/perf_kit/retry_and_time.py
##########
@@ -0,0 +1,133 @@
+# 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.
+import contextlib
+import functools
+import math
+import random
+import signal
+import time
+
+
+class TimingResult:
+ def __init__(self):
+ self.start_time = 0
+ self.end_time = 0
+ self.value = 0
+
+
+@contextlib.contextmanager
+def timing(retry_count=1):
+ start_time = time.time()
Review comment:
```suggestion
start_time = time.monotonic()
```
----------------------------------------------------------------
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 a change in pull request #7650:
[AIRFLOW-7008] Add perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #7650: [AIRFLOW-7008] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#discussion_r408709577
##########
File path: scripts/perf/perf_kit/memory.py
##########
@@ -0,0 +1,77 @@
+# 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.
+
+import os
+from contextlib import contextmanager
+
+import psutil
+
+
+def _get_process_memory():
+ process = psutil.Process(os.getpid())
+ return process.memory_info().rss
+
+
+def _human_readable_size(size, decimal_places=3):
+ for unit in ["B", "KiB", "MiB", "GiB", "TiB"]:
+ if size < 1024.0:
+ break
+ size /= 1024.0
+ return f"{size:.{decimal_places}f}{unit}"
+
+
+class TraceMemoryResult:
+ def __init__(self):
+ self.before = 0
+ self.after = 0
+ self.value = 0
+
+
+@contextmanager
+def trace_memory(human_readable=True):
+ """
+ Calculates the amount of difference in free memory before and after script execution.
+
+ In other words, how much data the code snippet has used up memory.
+
+ :param human_readable: If yes, the result will be displayed in human readable units.
+ If no, the result will be displayed as bytes.
+ """
+ before = _get_process_memory()
+ result = TraceMemoryResult()
+ try:
+ yield result
Review comment:
For this to be accurate/stable we probably want to trigger a GC run.
`import gc; gc.collect()` give or take.
----------------------------------------------------------------
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 a change in pull request #7650:
[AIRFLOW-7008] Add perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #7650: [AIRFLOW-7008] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#discussion_r408743446
##########
File path: scripts/perf/perf_kit/memory.py
##########
@@ -0,0 +1,77 @@
+# 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.
+
+import os
+from contextlib import contextmanager
+
+import psutil
+
+
+def _get_process_memory():
+ process = psutil.Process(os.getpid())
+ return process.memory_info().rss
+
+
+def _human_readable_size(size, decimal_places=3):
+ for unit in ["B", "KiB", "MiB", "GiB", "TiB"]:
+ if size < 1024.0:
+ break
+ size /= 1024.0
+ return f"{size:.{decimal_places}f}{unit}"
+
+
+class TraceMemoryResult:
+ def __init__(self):
+ self.before = 0
+ self.after = 0
+ self.value = 0
+
+
+@contextmanager
+def trace_memory(human_readable=True):
+ """
+ Calculates the amount of difference in free memory before and after script execution.
+
+ In other words, how much data the code snippet has used up memory.
+
+ :param human_readable: If yes, the result will be displayed in human readable units.
+ If no, the result will be displayed as bytes.
+ """
+ before = _get_process_memory()
+ result = TraceMemoryResult()
+ try:
+ yield result
Review comment:
It depends on the specific case you want to measure. Sometimes we need to measure the amount of memory used together with garbage, and sometimes without. It depends on the intention of the particular 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] ashb commented on a change in pull request #7650:
[AIRFLOW-7008][WIP] Add perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #7650: [AIRFLOW-7008][WIP] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#discussion_r389578111
##########
File path: scripts/perf/perf_kit/imports.py
##########
@@ -0,0 +1,68 @@
+# 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.
+
+import builtins
+import sys
+from contextlib import contextmanager
+
+
+class TraceImportResult:
+ def __init__(self):
+ self.new_modules = None
+
+ def print_new_modules(self):
+ for i, new_module in enumerate(sorted(trace_resuslt.new_modules)):
+ print(f"{i}. {new_module}")
+
+
+@contextmanager
+def trace_imports():
+ real_import = builtins.__import__
+ before_modules = set(sys.modules.keys())
+ result = TraceImportResult()
+
+ def debug_import(*args, **kwargs):
+ frame = sys._getframe(1)
+ source = frame.f_back.f_code.co_filename if frame.f_back else "<unknown>"
+ imported_name = args[0]
+
+ if "airflow" in source:
+ print(f"{source} >>> {imported_name}")
+ return real_import(*args, **kwargs)
+
+ builtins.__import__ = debug_import
+ try:
+ yield result
+ finally:
+ builtins.__import__ = real_import
+ after_modules = set(sys.modules.keys())
+ before_count = len(before_modules)
+ after_count = len(after_modules)
+ diff_count = after_count - before_count
+ result.new_modules = after_modules - before_modules
+ print(f"Modules loaded - before: {before_count}")
+ print(f"Modules loaded - after: {after_count}")
+ print(f"Modules loaded - diff: {diff_count}")
+
+
+if __name__ == "__main__":
+ # Example:
+
+ with trace_imports() as trace_resuslt:
+ import airflow # noqa # pylint: disable=unused-import
Review comment:
Python has most of this built in already - https://docs.python.org/3/using/cmdline.html#id5
> `-X importtime` to show how long each import takes. It shows module name, cumulative time (including nested imports) and self time (excluding nested imports). Note that its output may be broken in multi-threaded application. Typical usage is `python3 -X importtime -c 'import asyncio'`. See also [`PYTHONPROFILEIMPORTTIME`](https://docs.python.org/3/using/cmdline.html#envvar-PYTHONPROFILEIMPORTTIME).
And can be visualized easily with https://github.com/nschloe/tuna
----------------------------------------------------------------
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] milton0825 commented on a change in pull request #7650:
[AIRFLOW-7008] Add perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
milton0825 commented on a change in pull request #7650: [AIRFLOW-7008] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#discussion_r408670056
##########
File path: scripts/perf/perf_kit/__init__.py
##########
@@ -0,0 +1,119 @@
+# 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.
+""""
+
+Perf-kit
+========
+
+Useful decorators and context managers used when testing the performance of various Airflow components.
+
+To use this package, you must add the parent directory to the ``PYTHONPATH`` environment variable. If the
+repository root directory is ``/opt/airflow/``, then you need to run the following command:
+
+.. code-block:: bash
+ export PYTHONPATH=/opt/airflow/script/perf
+
+Content
+=======
+The following decorators and context managers are included.
+
+.. autofunction:: perf_kit.memory.trace_memory
+
+.. autofunction:: perf_kit.python.pyspy
+
+.. autofunction:: perf_kit.python.profiled
+
+.. autofunction:: perf_kit.repeat_and_time.timing
+
+.. autofunction:: perf_kit.repeat_and_time.repeat
+
+.. autofunction:: perf_kit.repeat_and_time.timeout
+
+.. autofunction:: perf_kit.sqlalchemy.trace_queries
+
+.. autofunction:: perf_kit.sqlalchemy.count_queries
+
+Documentation for each function is provided in the function docstrings. Each module also has an example in
+the main section of the module.
+
+Examples
+========
+
+If you want to run an all example for ``perf_kit.sqlalchemy``, you can run the following command.
+
+.. code-block:: bash
+ python -m perf_kit.sqlalchemy
+
+If you want to know how to use these functions, it is worth to familiarize yourself with these examples.
+
+Use in tests
+============
+
+If you need it, you can easily test only one test using context manager.
+
+Suppose we have the following fragment of the file with tests.
+
+.. code-block:: python
+
+ prev = dag.previous_schedule(_next)
+ prev_local = local_tz.convert(prev)
+
+ self.assertEqual(prev_local.isoformat(), "2018-03-24T03:00:00+01:00")
+ self.assertEqual(prev.isoformat(), "2018-03-24T02:00:00+00:00")
+
+ def test_bulk_sync_to_db(self):
+ clear_db_dags()
+ dags = [
+ DAG(f'dag-bulk-sync-{i}', start_date=DEFAULT_DATE, tags=["test-dag"]) for i in range(0, 4)
+ ]
+
+ with assert_queries_count(3):
+ DAG.bulk_sync_to_db(dags)
+
+You can add a code snippet before the method definition, and then perform only one test and count the
+queries in it.
+
+.. code-block:: python
Review comment:
+1 on using the diff here if .rst support that
----------------------------------------------------------------
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 edited a comment on issue #7650:
[AIRFLOW-7008] Add perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7650: [AIRFLOW-7008] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#issuecomment-596778149
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7650?src=pr&el=h1) Report
> Merging [#7650](https://codecov.io/gh/apache/airflow/pull/7650?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/45c8983306ab1c54abdacd8f870e790fad25cb37&el=desc) will **decrease** coverage by `0.76%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/airflow/pull/7650?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7650 +/- ##
==========================================
- Coverage 88.45% 87.68% -0.77%
==========================================
Files 937 940 +3
Lines 45234 45355 +121
==========================================
- Hits 40011 39770 -241
- Misses 5223 5585 +362
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7650?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.25% <0.00%> (-72.50%)` | :arrow_down: |
| [...w/providers/apache/hive/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2hpdmUvb3BlcmF0b3JzL215c3FsX3RvX2hpdmUucHk=) | `35.84% <0.00%> (-64.16%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0.00%> (-55.56%)` | :arrow_down: |
| [airflow/providers/postgres/operators/postgres.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvb3BlcmF0b3JzL3Bvc3RncmVzLnB5) | `47.82% <0.00%> (-52.18%)` | :arrow_down: |
| [airflow/providers/redis/operators/redis\_publish.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvb3BlcmF0b3JzL3JlZGlzX3B1Ymxpc2gucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0.00%> (-47.06%)` | :arrow_down: |
| [airflow/providers/mongo/sensors/mongo.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbW9uZ28vc2Vuc29ycy9tb25nby5weQ==) | `53.33% <0.00%> (-46.67%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `47.18% <0.00%> (-45.08%)` | :arrow_down: |
| [airflow/providers/mysql/operators/mysql.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbXlzcWwvb3BlcmF0b3JzL215c3FsLnB5) | `55.00% <0.00%> (-45.00%)` | :arrow_down: |
| [airflow/providers/redis/sensors/redis\_key.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvc2Vuc29ycy9yZWRpc19rZXkucHk=) | `61.53% <0.00%> (-38.47%)` | :arrow_down: |
| ... and [16 more](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7650?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/7650?src=pr&el=footer). Last update [45c8983...82205bc](https://codecov.io/gh/apache/airflow/pull/7650?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] milton0825 commented on a change in pull request #7650:
[AIRFLOW-7008] Add perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
milton0825 commented on a change in pull request #7650: [AIRFLOW-7008] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#discussion_r408668533
##########
File path: scripts/perf/perf_kit/__init__.py
##########
@@ -0,0 +1,119 @@
+# 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.
+""""
+
+Perf-kit
+========
+
+Useful decorators and context managers used when testing the performance of various Airflow components.
+
+To use this package, you must add the parent directory to the ``PYTHONPATH`` environment variable. If the
+repository root directory is ``/opt/airflow/``, then you need to run the following command:
+
+.. code-block:: bash
+ export PYTHONPATH=/opt/airflow/script/perf
Review comment:
```suggestion
export PYTHONPATH=/opt/airflow/scripts/perf
```
----------------------------------------------------------------
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 a change in pull request #7650:
[AIRFLOW-7008] Add perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #7650: [AIRFLOW-7008] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#discussion_r408741801
##########
File path: scripts/perf/perf_kit/repeat_and_time.py
##########
@@ -0,0 +1,148 @@
+# 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.
+import contextlib
+import functools
+import math
+import random
+import signal
+import time
+
+
+class TimingResult:
+ def __init__(self):
+ self.start_time = 0
+ self.end_time = 0
+ self.value = 0
+
+
+@contextlib.contextmanager
+def timing(repeat_count: int = 1):
+ """
+ Measures code execution time.
+
+ :param repeat_count: If passed, the result will be divided by the value.
+ """
+ result = TimingResult()
+ result.start_time = time.monotonic()
+ try:
+ yield result
+ finally:
+ end_time = time.monotonic()
+ diff = (end_time - result.start_time) * 1000.0
+
+ result.end_time = end_time
+ if repeat_count == 1:
+ result.value = diff
+ print(f"Loop time: {diff:.3f} ms")
+ else:
+ average_time = diff / repeat_count
+ result.value = average_time
+ print(f"Average time: {average_time:.3f} ms")
+
+
+def repeat(repeat_count=5):
+ """
+ Function decorators that repeat function many times.
+
+ :param repeat_count: The repeat count
+ """
+
+ def repeat_decorator(f):
+ @functools.wraps(f)
+ def wrap(*args, **kwargs):
+ last_result = None
+ for i in range(repeat_count):
+ last_result = f(*args, **kwargs)
+ return last_result
+
+ return wrap
+
+ return repeat_decorator
+
+
+class TimeoutException(Exception):
+ pass
+
+
+@contextlib.contextmanager
+def timeout(seconds=1):
+ """
+ Executes code only limited seconds. If the code does not end during this time, it will be interrupted.
+
+ :param seconds: Number of seconds
+ """
+ def handle_timeout(signum, frame):
+ raise TimeoutException("Process timed out.")
+
+ try:
+ signal.signal(signal.SIGALRM, handle_timeout)
+ signal.alarm(seconds)
+ except ValueError:
+ raise Exception("timeout can't be used in the current context")
+
+ try:
+ yield
+ except TimeoutException:
+ print("Process timed out.")
+
+ finally:
+ try:
+ signal.alarm(0)
+ except ValueError:
+ raise Exception("timeout can't be used in the current context")
+
+
+if __name__ == "__main__":
+
+ def monte_carlo(total=10000):
+ # Monte Carlo
+ inside = 0
+
+ for i in range(0, total):
+ x2 = random.random() ** 2
+ y2 = random.random() ** 2
+ if math.sqrt(x2 + y2) < 1.0:
+ inside += 1
+
+ return (float(inside) / total) * 4
+
+ # Example 1:s
+ with timeout(1):
+ print("Sleep 5s with 1s timeout")
+ time.sleep(4)
+ print(":-/")
+
+ print()
+
+ # Example 2:
+ REPEAT_COUNT = 5
+
+ @timing(REPEAT_COUNT)
Review comment:
Yes. In this case, the average for all runs will be calculated.
----------------------------------------------------------------
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] zhongjiajie commented on issue #7650: [AIRFLOW-7008] Add
perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on issue #7650: [AIRFLOW-7008] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#issuecomment-613794873
Looking forward your public blog, BTW, code LGTM
----------------------------------------------------------------
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 #7650: [AIRFLOW-7008] Add perf
kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #7650: [AIRFLOW-7008] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#issuecomment-613780821
I will add a short note in the file TESTING.rst.
----------------------------------------------------------------
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 #7650:
[AIRFLOW-7008][WIP] Add perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on issue #7650: [AIRFLOW-7008][WIP] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#issuecomment-596162956
I want to add documentation in this PR, but for now I wanted to take the first step. This PR is still WIP.
----------------------------------------------------------------
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 edited a comment on issue #7650:
[AIRFLOW-7008] Add perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7650: [AIRFLOW-7008] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#issuecomment-596778149
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7650?src=pr&el=h1) Report
> Merging [#7650](https://codecov.io/gh/apache/airflow/pull/7650?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/45c8983306ab1c54abdacd8f870e790fad25cb37&el=desc) will **decrease** coverage by `0.76%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/airflow/pull/7650?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7650 +/- ##
==========================================
- Coverage 88.45% 87.68% -0.77%
==========================================
Files 937 940 +3
Lines 45234 45355 +121
==========================================
- Hits 40011 39770 -241
- Misses 5223 5585 +362
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7650?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.25% <0.00%> (-72.50%)` | :arrow_down: |
| [...w/providers/apache/hive/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2hpdmUvb3BlcmF0b3JzL215c3FsX3RvX2hpdmUucHk=) | `35.84% <0.00%> (-64.16%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0.00%> (-55.56%)` | :arrow_down: |
| [airflow/providers/postgres/operators/postgres.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvb3BlcmF0b3JzL3Bvc3RncmVzLnB5) | `47.82% <0.00%> (-52.18%)` | :arrow_down: |
| [airflow/providers/redis/operators/redis\_publish.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvb3BlcmF0b3JzL3JlZGlzX3B1Ymxpc2gucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0.00%> (-47.06%)` | :arrow_down: |
| [airflow/providers/mongo/sensors/mongo.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbW9uZ28vc2Vuc29ycy9tb25nby5weQ==) | `53.33% <0.00%> (-46.67%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `47.18% <0.00%> (-45.08%)` | :arrow_down: |
| [airflow/providers/mysql/operators/mysql.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbXlzcWwvb3BlcmF0b3JzL215c3FsLnB5) | `55.00% <0.00%> (-45.00%)` | :arrow_down: |
| [airflow/providers/redis/sensors/redis\_key.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvc2Vuc29ycy9yZWRpc19rZXkucHk=) | `61.53% <0.00%> (-38.47%)` | :arrow_down: |
| ... and [16 more](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7650?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/7650?src=pr&el=footer). Last update [45c8983...82205bc](https://codecov.io/gh/apache/airflow/pull/7650?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] mik-laj commented on a change in pull request #7650:
[AIRFLOW-7008] Add perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #7650: [AIRFLOW-7008] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#discussion_r408746669
##########
File path: scripts/perf/perf_kit/memory.py
##########
@@ -0,0 +1,77 @@
+# 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.
+
+import os
+from contextlib import contextmanager
+
+import psutil
+
+
+def _get_process_memory():
+ process = psutil.Process(os.getpid())
+ return process.memory_info().rss
+
+
+def _human_readable_size(size, decimal_places=3):
+ for unit in ["B", "KiB", "MiB", "GiB", "TiB"]:
+ if size < 1024.0:
+ break
+ size /= 1024.0
+ return f"{size:.{decimal_places}f}{unit}"
+
+
+class TraceMemoryResult:
+ def __init__(self):
+ self.before = 0
+ self.after = 0
+ self.value = 0
+
+
+@contextmanager
+def trace_memory(human_readable=True):
+ """
+ Calculates the amount of difference in free memory before and after script execution.
+
+ In other words, how much data the code snippet has used up memory.
+
+ :param human_readable: If yes, the result will be displayed in human readable units.
+ If no, the result will be displayed as bytes.
+ """
+ before = _get_process_memory()
+ result = TraceMemoryResult()
+ try:
+ yield result
Review comment:
I will add a note in docstring.
----------------------------------------------------------------
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] zhongjiajie commented on a change in pull request #7650:
[AIRFLOW-7008] Add perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on a change in pull request #7650: [AIRFLOW-7008] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#discussion_r408540897
##########
File path: scripts/perf/perf_kit/__init__.py
##########
@@ -0,0 +1,119 @@
+# 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.
+""""
+
+Perf-kit
+========
+
+Useful decorators and context managers used when testing the performance of various Airflow components.
+
+To use this package, you must add the parent directory to the ``PYTHONPATH`` environment variable. If the
+repository root directory is ``/opt/airflow/``, then you need to run the following command:
+
+.. code-block:: bash
+ export PYTHONPATH=/opt/airflow/script/perf
+
+Content
+=======
+The following decorators and context managers are included.
+
+.. autofunction:: perf_kit.memory.trace_memory
+
+.. autofunction:: perf_kit.python.pyspy
+
+.. autofunction:: perf_kit.python.profiled
+
+.. autofunction:: perf_kit.repeat_and_time.timing
+
+.. autofunction:: perf_kit.repeat_and_time.repeat
+
+.. autofunction:: perf_kit.repeat_and_time.timeout
+
+.. autofunction:: perf_kit.sqlalchemy.trace_queries
+
+.. autofunction:: perf_kit.sqlalchemy.count_queries
+
+Documentation for each function is provided in the function docstrings. Each module also has an example in
+the main section of the module.
+
+Examples
+========
+
+If you want to run an all example for ``perf_kit.sqlalchemy``, you can run the following command.
+
+.. code-block:: bash
+ python -m perf_kit.sqlalchemy
+
+If you want to know how to use these functions, it is worth to familiarize yourself with these examples.
+
+Use in tests
+============
+
+If you need it, you can easily test only one test using context manager.
+
+Suppose we have the following fragment of the file with tests.
+
+.. code-block:: python
+
+ prev = dag.previous_schedule(_next)
+ prev_local = local_tz.convert(prev)
+
+ self.assertEqual(prev_local.isoformat(), "2018-03-24T03:00:00+01:00")
+ self.assertEqual(prev.isoformat(), "2018-03-24T02:00:00+00:00")
+
+ def test_bulk_sync_to_db(self):
+ clear_db_dags()
+ dags = [
+ DAG(f'dag-bulk-sync-{i}', start_date=DEFAULT_DATE, tags=["test-dag"]) for i in range(0, 4)
+ ]
+
+ with assert_queries_count(3):
+ DAG.bulk_sync_to_db(dags)
+
+You can add a code snippet before the method definition, and then perform only one test and count the
+queries in it.
+
+.. code-block:: python
Review comment:
I will use type `diff` in some change document in my company, for more easy to trace what change we did, do you think we should use diff here? And I don't know whether rst file work with `diff`
```diff
prev = dag.previous_schedule(_next)
prev_local = local_tz.convert(prev)
self.assertEqual(prev_local.isoformat(), "2018-03-24T03:00:00+01:00")
self.assertEqual(prev.isoformat(), "2018-03-24T02:00:00+00:00")
+ from perf_kit.sqlalchemy.sqlalchemy import trace_queries
+ @trace_queries
def test_bulk_sync_to_db(self):
clear_db_dags()
dags = [
DAG(f'dag-bulk-sync-{i}', start_date=DEFAULT_DATE, tags=["test-dag"]) for i in range(0, 4)
]
with assert_queries_count(3):
DAG.bulk_sync_to_db(dags)
```
----------------------------------------------------------------
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] zhongjiajie edited a comment on issue #7650:
[AIRFLOW-7008] Add perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
zhongjiajie edited a comment on issue #7650: [AIRFLOW-7008] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#issuecomment-613794873
Looking forward your public blog, BTW, code&doc LGTM, except TESTING.rst what you mention above
----------------------------------------------------------------
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 #7650: [AIRFLOW-7008] Add perf
kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #7650: [AIRFLOW-7008] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#issuecomment-613682797
I added documentation. Do you want someone to look at these changes?
----------------------------------------------------------------
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 a change in pull request #7650:
[AIRFLOW-7008] Add perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #7650: [AIRFLOW-7008] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#discussion_r408745987
##########
File path: scripts/perf/perf_kit/__init__.py
##########
@@ -0,0 +1,119 @@
+# 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.
+""""
+
+Perf-kit
+========
+
+Useful decorators and context managers used when testing the performance of various Airflow components.
+
+To use this package, you must add the parent directory to the ``PYTHONPATH`` environment variable. If the
+repository root directory is ``/opt/airflow/``, then you need to run the following command:
+
+.. code-block:: bash
+ export PYTHONPATH=/opt/airflow/script/perf
+
+Content
+=======
+The following decorators and context managers are included.
+
+.. autofunction:: perf_kit.memory.trace_memory
+
+.. autofunction:: perf_kit.python.pyspy
+
+.. autofunction:: perf_kit.python.profiled
+
+.. autofunction:: perf_kit.repeat_and_time.timing
+
+.. autofunction:: perf_kit.repeat_and_time.repeat
+
+.. autofunction:: perf_kit.repeat_and_time.timeout
+
+.. autofunction:: perf_kit.sqlalchemy.trace_queries
+
+.. autofunction:: perf_kit.sqlalchemy.count_queries
+
+Documentation for each function is provided in the function docstrings. Each module also has an example in
+the main section of the module.
+
+Examples
+========
+
+If you want to run an all example for ``perf_kit.sqlalchemy``, you can run the following command.
+
+.. code-block:: bash
+ python -m perf_kit.sqlalchemy
+
+If you want to know how to use these functions, it is worth to familiarize yourself with these examples.
+
+Use in tests
+============
+
+If you need it, you can easily test only one test using context manager.
+
+Suppose we have the following fragment of the file with tests.
+
+.. code-block:: python
+
+ prev = dag.previous_schedule(_next)
+ prev_local = local_tz.convert(prev)
+
+ self.assertEqual(prev_local.isoformat(), "2018-03-24T03:00:00+01:00")
+ self.assertEqual(prev.isoformat(), "2018-03-24T02:00:00+00:00")
+
+ def test_bulk_sync_to_db(self):
+ clear_db_dags()
+ dags = [
+ DAG(f'dag-bulk-sync-{i}', start_date=DEFAULT_DATE, tags=["test-dag"]) for i in range(0, 4)
+ ]
+
+ with assert_queries_count(3):
+ DAG.bulk_sync_to_db(dags)
+
+You can add a code snippet before the method definition, and then perform only one test and count the
+queries in it.
+
+.. code-block:: python
Review comment:
If we use diff here, we will lose support for Python syntax highlight. We can't use syntax highlight for Diff and Python at the same time. I will add ` :emphasize-lines: `` to make the added line more visible.
----------------------------------------------------------------
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 a change in pull request #7650:
[AIRFLOW-7008][WIP] Add perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #7650: [AIRFLOW-7008][WIP] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#discussion_r389756810
##########
File path: scripts/perf/perf_kit/imports.py
##########
@@ -0,0 +1,68 @@
+# 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.
+
+import builtins
+import sys
+from contextlib import contextmanager
+
+
+class TraceImportResult:
+ def __init__(self):
+ self.new_modules = None
+
+ def print_new_modules(self):
+ for i, new_module in enumerate(sorted(trace_resuslt.new_modules)):
+ print(f"{i}. {new_module}")
+
+
+@contextmanager
+def trace_imports():
+ real_import = builtins.__import__
+ before_modules = set(sys.modules.keys())
+ result = TraceImportResult()
+
+ def debug_import(*args, **kwargs):
+ frame = sys._getframe(1)
+ source = frame.f_back.f_code.co_filename if frame.f_back else "<unknown>"
+ imported_name = args[0]
+
+ if "airflow" in source:
+ print(f"{source} >>> {imported_name}")
+ return real_import(*args, **kwargs)
+
+ builtins.__import__ = debug_import
+ try:
+ yield result
+ finally:
+ builtins.__import__ = real_import
+ after_modules = set(sys.modules.keys())
+ before_count = len(before_modules)
+ after_count = len(after_modules)
+ diff_count = after_count - before_count
+ result.new_modules = after_modules - before_modules
+ print(f"Modules loaded - before: {before_count}")
+ print(f"Modules loaded - after: {after_count}")
+ print(f"Modules loaded - diff: {diff_count}")
+
+
+if __name__ == "__main__":
+ # Example:
+
+ with trace_imports() as trace_resuslt:
+ import airflow # noqa # pylint: disable=unused-import
Review comment:
Thanks for recommending other tools. I will try them when I need them.
----------------------------------------------------------------
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 #7650: [AIRFLOW-7008]
Add perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on issue #7650: [AIRFLOW-7008] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#issuecomment-613682797
I added documentation. Does someone want to look at these changes? That would be helpful.
CC: @zhongjiajie
----------------------------------------------------------------
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 a change in pull request #7650:
[AIRFLOW-7008][WIP] Add perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #7650: [AIRFLOW-7008][WIP] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#discussion_r389580011
##########
File path: scripts/perf/perf_kit/retry_and_time.py
##########
@@ -0,0 +1,133 @@
+# 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.
+import contextlib
+import functools
+import math
+import random
+import signal
+import time
+
+
+class TimingResult:
+ def __init__(self):
+ self.start_time = 0
+ self.end_time = 0
+ self.value = 0
+
+
+@contextlib.contextmanager
+def timing(retry_count=1):
+ start_time = time.time()
+ result = TimingResult
+ try:
+ yield result
+ finally:
+ end_time = time.time()
+ diff = (end_time - start_time) * 1000.0
+
+ result.start = start_time
+ result.end_time = end_time
+ if retry_count == 1:
+ result.value = diff
+ print(f"Retry time: {diff:.3f} ms")
+ else:
+ average_time = diff / retry_count
+ result.value = average_time
+ print(f"Average time: {average_time:.3f} ms")
+
+
+def retry(retry_count=5):
Review comment:
```suggestion
def repeat(retry_count=5):
```
Retry usually means "try up to N times, but stop when we get a success" -- this always runs the given number of times.
----------------------------------------------------------------
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] zhongjiajie edited a comment on issue #7650:
[AIRFLOW-7008] Add perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
zhongjiajie edited a comment on issue #7650: [AIRFLOW-7008] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#issuecomment-613794873
Looking forward your public blog, BTW, code&doc LGTM
----------------------------------------------------------------
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 a change in pull request #7650:
[AIRFLOW-7008][WIP] Add perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #7650: [AIRFLOW-7008][WIP] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#discussion_r389578543
##########
File path: scripts/perf/perf_kit/retry_and_time.py
##########
@@ -0,0 +1,133 @@
+# 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.
+import contextlib
+import functools
+import math
+import random
+import signal
+import time
+
+
+class TimingResult:
+ def __init__(self):
+ self.start_time = 0
+ self.end_time = 0
+ self.value = 0
+
+
+@contextlib.contextmanager
+def timing(retry_count=1):
+ start_time = time.time()
+ result = TimingResult
+ try:
+ yield result
+ finally:
+ end_time = time.time()
Review comment:
```suggestion
end_time = time.monotonic()
```
----------------------------------------------------------------
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 #7650: [AIRFLOW-7008][WIP] Add
perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #7650: [AIRFLOW-7008][WIP] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#issuecomment-596236336
Previous scripts track SQL queries for many processes and for all code. This script allows you to trace only part of the code and doesn't work properly with many processes. We also cannot share the code, because perf_kit is a separate package to be able to monitor the Airflow initialization process.
----------------------------------------------------------------
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 #7650: [AIRFLOW-7008][WIP]
Add perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #7650: [AIRFLOW-7008][WIP] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#issuecomment-596778149
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7650?src=pr&el=h1) Report
> Merging [#7650](https://codecov.io/gh/apache/airflow/pull/7650?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/cad20c28da341e6247c9ebece2589f246fd5392c?src=pr&el=desc) will **decrease** coverage by `0.12%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/airflow/pull/7650?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7650 +/- ##
==========================================
- Coverage 86.66% 86.54% -0.13%
==========================================
Files 897 897
Lines 42808 42869 +61
==========================================
- Hits 37101 37100 -1
- Misses 5707 5769 +62
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7650?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7650/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/7650/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/7650/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/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvY25jZi9rdWJlcm5ldGVzL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZC5weQ==) | `69.69% <0%> (-25.26%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/providers/google/cloud/operators/dataflow.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL29wZXJhdG9ycy9kYXRhZmxvdy5weQ==) | `90.44% <0%> (-8.65%)` | :arrow_down: |
| [airflow/jobs/backfill\_job.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL2JhY2tmaWxsX2pvYi5weQ==) | `91.03% <0%> (-1.13%)` | :arrow_down: |
| [airflow/providers/mysql/hooks/mysql.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbXlzcWwvaG9va3MvbXlzcWwucHk=) | `92.7% <0%> (-0.07%)` | :arrow_down: |
| [.../cloud/operators/cloud\_storage\_transfer\_service.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL29wZXJhdG9ycy9jbG91ZF9zdG9yYWdlX3RyYW5zZmVyX3NlcnZpY2UucHk=) | `95.62% <0%> (-0.02%)` | :arrow_down: |
| [airflow/providers/sftp/operators/sftp.py](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvc2Z0cC9vcGVyYXRvcnMvc2Z0cC5weQ==) | `87.67% <0%> (ø)` | :arrow_up: |
| ... and [11 more](https://codecov.io/gh/apache/airflow/pull/7650/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7650?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/7650?src=pr&el=footer). Last update [cad20c2...0f42be8](https://codecov.io/gh/apache/airflow/pull/7650?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] zhongjiajie commented on issue #7650: [AIRFLOW-7008] Add
perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on issue #7650: [AIRFLOW-7008] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#issuecomment-613780352
BTW, I find out we have some scheduler doc only in confluence https://cwiki.apache.org/confluence/display/AIRFLOW/Scheduler+Basics, should we add this page to airflow document 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.
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 #7650: [AIRFLOW-7008] Add perf
kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #7650: [AIRFLOW-7008] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#issuecomment-613784971
I saw this article, but I'm afraid we should check and update it first. I started to write internal materials for my company. I started to write internal materials for my company. I hope that some excerpts can be published on the company blog sometime.
----------------------------------------------------------------
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 a change in pull request #7650:
[AIRFLOW-7008][WIP] Add perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #7650: [AIRFLOW-7008][WIP] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#discussion_r389755483
##########
File path: scripts/perf/perf_kit/imports.py
##########
@@ -0,0 +1,68 @@
+# 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.
+
+import builtins
+import sys
+from contextlib import contextmanager
+
+
+class TraceImportResult:
+ def __init__(self):
+ self.new_modules = None
+
+ def print_new_modules(self):
+ for i, new_module in enumerate(sorted(trace_resuslt.new_modules)):
+ print(f"{i}. {new_module}")
+
+
+@contextmanager
+def trace_imports():
+ real_import = builtins.__import__
+ before_modules = set(sys.modules.keys())
+ result = TraceImportResult()
+
+ def debug_import(*args, **kwargs):
+ frame = sys._getframe(1)
+ source = frame.f_back.f_code.co_filename if frame.f_back else "<unknown>"
+ imported_name = args[0]
+
+ if "airflow" in source:
+ print(f"{source} >>> {imported_name}")
+ return real_import(*args, **kwargs)
+
+ builtins.__import__ = debug_import
+ try:
+ yield result
+ finally:
+ builtins.__import__ = real_import
+ after_modules = set(sys.modules.keys())
+ before_count = len(before_modules)
+ after_count = len(after_modules)
+ diff_count = after_count - before_count
+ result.new_modules = after_modules - before_modules
+ print(f"Modules loaded - before: {before_count}")
+ print(f"Modules loaded - after: {after_count}")
+ print(f"Modules loaded - diff: {diff_count}")
+
+
+if __name__ == "__main__":
+ # Example:
+
+ with trace_imports() as trace_resuslt:
+ import airflow # noqa # pylint: disable=unused-import
Review comment:
There are many very complex tools designed for this, but it is a simple tool that is easy to use. I would also like to track only imports that are from Airflow, because I have no influence on those that are in other libraries. This tool was very useful, when worked on https://github.com/apache/airflow/pull/7327
This is not a tool that is now actively used, so I can add it later. For now, I will delete it from this PR.
----------------------------------------------------------------
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] zhongjiajie commented on issue #7650: [AIRFLOW-7008] Add
perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on issue #7650: [AIRFLOW-7008] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#issuecomment-613779057
> In my opinion, the CONTRIBUTOR Guide should contain the most important information for beginners.
Make sence, but I still think we should add doc somewhere else, not only in python docsting
----------------------------------------------------------------
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] zhongjiajie commented on issue #7650: [AIRFLOW-7008] Add
perf kit with common used decorators/contexts
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on issue #7650: [AIRFLOW-7008] Add perf kit with common used decorators/contexts
URL: https://github.com/apache/airflow/pull/7650#issuecomment-613769496
Should we add some document to `CONTRIBUTING.rst` 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services