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/01/14 03:38:16 UTC

[GitHub] [airflow] lokeshlal opened a new pull request #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)

lokeshlal opened a new pull request #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)
URL: https://github.com/apache/airflow/pull/7160
 
 
   PR contains changes in pool and task instance to provide functionality to tasks to use more than one pool slot.
   
   - Added pool_capacity field in TaskInstance (pool_capacity is defaulted to 1, to maintain the current behavior)
   - Added pool_capacity in baseoperator
   - Modified pools functionality to calculate the used/queued/occupied/available slots
   - Modified pool_slots_available_dep.py to check against task _pool_capacity_ field instead of 1 
   - Modifed test case in file test_pool_slots_available_dep.py to include pool_capacity in the Mock object
   
   -----------------------------------------------------------------------------------------------
   
   - [x] Description above provides context of the change
   - [X] Commit message contains [\[AIRFLOW-1467\]](https://issues.apache.org/jira/browse/AIRFLOW-1467) or `[AIRFLOW-XXXX]` for document-only changes
   - [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).
   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] kaxil commented on issue #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)

Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)
URL: https://github.com/apache/airflow/pull/7160#issuecomment-574870008
 
 
   Thanks for waiting @lokeshlal , I am currently on Leave but would be reviewing it very soon :)

----------------------------------------------------------------
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 #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)
URL: https://github.com/apache/airflow/pull/7160#issuecomment-574096290
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/7160?src=pr&el=h1) Report
   > Merging [#7160](https://codecov.io/gh/apache/airflow/pull/7160?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/5abce471e0690c6b8d06ca25685b0845c5fd270f?src=pr&el=desc) will **decrease** coverage by `0.99%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/7160/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/7160?src=pr&el=tree)
   
   ```diff
   @@           Coverage Diff            @@
   ##           master    #7160    +/-   ##
   ========================================
   - Coverage   85.41%   84.42%    -1%     
   ========================================
     Files         753      753            
     Lines       39685    39693     +8     
   ========================================
   - Hits        33898    33509   -389     
   - Misses       5787     6184   +397
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7160?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/models/pool.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvcG9vbC5weQ==) | `97.36% <ø> (ø)` | :arrow_up: |
   | [airflow/models/baseoperator.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvYmFzZW9wZXJhdG9yLnB5) | `96.28% <100%> (+0.02%)` | :arrow_up: |
   | [airflow/models/taskinstance.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvdGFza2luc3RhbmNlLnB5) | `94.96% <100%> (+0.03%)` | :arrow_up: |
   | [airflow/ti\_deps/deps/pool\_slots\_available\_dep.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy90aV9kZXBzL2RlcHMvcG9vbF9zbG90c19hdmFpbGFibGVfZGVwLnB5) | `100% <100%> (ø)` | :arrow_up: |
   | [airflow/operators/mysql\_operator.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXlzcWxfb3BlcmF0b3IucHk=) | `0% <0%> (-100%)` | :arrow_down: |
   | [airflow/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXlzcWxfdG9faGl2ZS5weQ==) | `0% <0%> (-100%)` | :arrow_down: |
   | [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.51% <0%> (-72.16%)` | :arrow_down: |
   | [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
   | [airflow/api/auth/backend/kerberos\_auth.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9hcGkvYXV0aC9iYWNrZW5kL2tlcmJlcm9zX2F1dGgucHk=) | `28.16% <0%> (-54.93%)` | :arrow_down: |
   | [...irflow/contrib/operators/redis\_publish\_operator.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9yZWRpc19wdWJsaXNoX29wZXJhdG9yLnB5) | `50% <0%> (-50%)` | :arrow_down: |
   | ... and [16 more](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7160?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/7160?src=pr&el=footer). Last update [5abce47...3a5351d](https://codecov.io/gh/apache/airflow/pull/7160?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] codecov-io edited a comment on issue #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)
URL: https://github.com/apache/airflow/pull/7160#issuecomment-574096290
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/7160?src=pr&el=h1) Report
   > Merging [#7160](https://codecov.io/gh/apache/airflow/pull/7160?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/a0568b9536225a89e7364278450f2677fd2851d3?src=pr&el=desc) will **decrease** coverage by `0.83%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/7160/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/7160?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff            @@
   ##           master   #7160      +/-   ##
   =========================================
   - Coverage   85.43%   84.6%   -0.84%     
   =========================================
     Files         710     723      +13     
     Lines       39373   39554     +181     
   =========================================
   - Hits        33639   33463     -176     
   - Misses       5734    6091     +357
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7160?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/models/pool.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvcG9vbC5weQ==) | `97.36% <ø> (ø)` | :arrow_up: |
   | [airflow/models/baseoperator.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvYmFzZW9wZXJhdG9yLnB5) | `96.28% <100%> (+0.02%)` | :arrow_up: |
   | [airflow/models/taskinstance.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvdGFza2luc3RhbmNlLnB5) | `94.96% <100%> (+0.03%)` | :arrow_up: |
   | [airflow/ti\_deps/deps/pool\_slots\_available\_dep.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy90aV9kZXBzL2RlcHMvcG9vbF9zbG90c19hdmFpbGFibGVfZGVwLnB5) | `100% <100%> (ø)` | :arrow_up: |
   | [airflow/contrib/hooks/azure\_data\_lake\_hook.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL2F6dXJlX2RhdGFfbGFrZV9ob29rLnB5) | `0% <0%> (-93.11%)` | :arrow_down: |
   | [airflow/contrib/sensors/azure\_cosmos\_sensor.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL3NlbnNvcnMvYXp1cmVfY29zbW9zX3NlbnNvci5weQ==) | `0% <0%> (-81.25%)` | :arrow_down: |
   | [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.51% <0%> (-72.16%)` | :arrow_down: |
   | [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
   | [airflow/api/auth/backend/kerberos\_auth.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9hcGkvYXV0aC9iYWNrZW5kL2tlcmJlcm9zX2F1dGgucHk=) | `28.16% <0%> (-54.93%)` | :arrow_down: |
   | [...irflow/contrib/operators/redis\_publish\_operator.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9yZWRpc19wdWJsaXNoX29wZXJhdG9yLnB5) | `50% <0%> (-50%)` | :arrow_down: |
   | ... and [99 more](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7160?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/7160?src=pr&el=footer). Last update [a0568b9...609e09b](https://codecov.io/gh/apache/airflow/pull/7160?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] codecov-io edited a comment on issue #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)
URL: https://github.com/apache/airflow/pull/7160#issuecomment-574096290
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/7160?src=pr&el=h1) Report
   > Merging [#7160](https://codecov.io/gh/apache/airflow/pull/7160?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/a0568b9536225a89e7364278450f2677fd2851d3?src=pr&el=desc) will **decrease** coverage by `0.83%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/7160/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/7160?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff            @@
   ##           master   #7160      +/-   ##
   =========================================
   - Coverage   85.43%   84.6%   -0.84%     
   =========================================
     Files         710     723      +13     
     Lines       39373   39554     +181     
   =========================================
   - Hits        33639   33463     -176     
   - Misses       5734    6091     +357
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7160?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/models/pool.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvcG9vbC5weQ==) | `97.36% <ø> (ø)` | :arrow_up: |
   | [airflow/models/baseoperator.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvYmFzZW9wZXJhdG9yLnB5) | `96.28% <100%> (+0.02%)` | :arrow_up: |
   | [airflow/models/taskinstance.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvdGFza2luc3RhbmNlLnB5) | `94.96% <100%> (+0.03%)` | :arrow_up: |
   | [airflow/ti\_deps/deps/pool\_slots\_available\_dep.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy90aV9kZXBzL2RlcHMvcG9vbF9zbG90c19hdmFpbGFibGVfZGVwLnB5) | `100% <100%> (ø)` | :arrow_up: |
   | [airflow/contrib/hooks/azure\_data\_lake\_hook.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL2F6dXJlX2RhdGFfbGFrZV9ob29rLnB5) | `0% <0%> (-93.11%)` | :arrow_down: |
   | [airflow/contrib/sensors/azure\_cosmos\_sensor.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL3NlbnNvcnMvYXp1cmVfY29zbW9zX3NlbnNvci5weQ==) | `0% <0%> (-81.25%)` | :arrow_down: |
   | [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.51% <0%> (-72.16%)` | :arrow_down: |
   | [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
   | [airflow/api/auth/backend/kerberos\_auth.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9hcGkvYXV0aC9iYWNrZW5kL2tlcmJlcm9zX2F1dGgucHk=) | `28.16% <0%> (-54.93%)` | :arrow_down: |
   | [...irflow/contrib/operators/redis\_publish\_operator.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9yZWRpc19wdWJsaXNoX29wZXJhdG9yLnB5) | `50% <0%> (-50%)` | :arrow_down: |
   | ... and [99 more](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7160?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/7160?src=pr&el=footer). Last update [a0568b9...609e09b](https://codecov.io/gh/apache/airflow/pull/7160?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 #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)
URL: https://github.com/apache/airflow/pull/7160#discussion_r366306476
 
 

 ##########
 File path: airflow/models/baseoperator.py
 ##########
 @@ -178,6 +178,9 @@ class derived from this one results in the creation of a task object,
     :param pool: the slot pool this task should run in, slot pools are a
         way to limit concurrency for certain tasks
     :type pool: str
+    :param pool_capacity: the number of pool slots this task should use (>= 1)
 
 Review comment:
   Capacity is the overall size of the pool, and in the logs etc we talk about slots ("Not scheduling since there are %s open slots in pool %s") so do you think this would be better named as `pool_slots`
   
   WDYT @tooptoop4 ?

----------------------------------------------------------------
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 #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)

Posted by GitBox <gi...@apache.org>.
ashb commented on issue #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)
URL: https://github.com/apache/airflow/pull/7160#issuecomment-574172034
 
 
   (sorry to add more work for you after the first PR was merged!)

----------------------------------------------------------------
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 #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)
URL: https://github.com/apache/airflow/pull/7160#issuecomment-574096290
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/7160?src=pr&el=h1) Report
   > Merging [#7160](https://codecov.io/gh/apache/airflow/pull/7160?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/a0568b9536225a89e7364278450f2677fd2851d3?src=pr&el=desc) will **decrease** coverage by `0.83%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/7160/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/7160?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff            @@
   ##           master   #7160      +/-   ##
   =========================================
   - Coverage   85.43%   84.6%   -0.84%     
   =========================================
     Files         710     723      +13     
     Lines       39373   39554     +181     
   =========================================
   - Hits        33639   33463     -176     
   - Misses       5734    6091     +357
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7160?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/models/pool.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvcG9vbC5weQ==) | `97.36% <ø> (ø)` | :arrow_up: |
   | [airflow/models/baseoperator.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvYmFzZW9wZXJhdG9yLnB5) | `96.28% <100%> (+0.02%)` | :arrow_up: |
   | [airflow/models/taskinstance.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvdGFza2luc3RhbmNlLnB5) | `94.96% <100%> (+0.03%)` | :arrow_up: |
   | [airflow/ti\_deps/deps/pool\_slots\_available\_dep.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy90aV9kZXBzL2RlcHMvcG9vbF9zbG90c19hdmFpbGFibGVfZGVwLnB5) | `100% <100%> (ø)` | :arrow_up: |
   | [airflow/contrib/hooks/azure\_data\_lake\_hook.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL2F6dXJlX2RhdGFfbGFrZV9ob29rLnB5) | `0% <0%> (-93.11%)` | :arrow_down: |
   | [airflow/contrib/sensors/azure\_cosmos\_sensor.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL3NlbnNvcnMvYXp1cmVfY29zbW9zX3NlbnNvci5weQ==) | `0% <0%> (-81.25%)` | :arrow_down: |
   | [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.51% <0%> (-72.16%)` | :arrow_down: |
   | [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
   | [airflow/api/auth/backend/kerberos\_auth.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9hcGkvYXV0aC9iYWNrZW5kL2tlcmJlcm9zX2F1dGgucHk=) | `28.16% <0%> (-54.93%)` | :arrow_down: |
   | [...irflow/contrib/operators/redis\_publish\_operator.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9yZWRpc19wdWJsaXNoX29wZXJhdG9yLnB5) | `50% <0%> (-50%)` | :arrow_down: |
   | ... and [99 more](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7160?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/7160?src=pr&el=footer). Last update [a0568b9...609e09b](https://codecov.io/gh/apache/airflow/pull/7160?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 #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)
URL: https://github.com/apache/airflow/pull/7160#discussion_r366304906
 
 

 ##########
 File path: airflow/models/taskinstance.py
 ##########
 @@ -194,6 +195,11 @@ def __init__(self, task, execution_date, state=None):
 
         self.queue = task.queue
         self.pool = task.pool
+        if hasattr(task, 'pool_capacity'):
 
 Review comment:
   This hasattr check shouldn't be needed -- you've added it to baseoperator

----------------------------------------------------------------
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 #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)

Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)
URL: https://github.com/apache/airflow/pull/7160#issuecomment-574096290
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/7160?src=pr&el=h1) Report
   > Merging [#7160](https://codecov.io/gh/apache/airflow/pull/7160?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/a0568b9536225a89e7364278450f2677fd2851d3?src=pr&el=desc) will **decrease** coverage by `0.09%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/7160/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/7160?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff            @@
   ##           master    #7160     +/-   ##
   =========================================
   - Coverage   85.43%   85.33%   -0.1%     
   =========================================
     Files         710      710             
     Lines       39373    40177    +804     
   =========================================
   + Hits        33639    34286    +647     
   - Misses       5734     5891    +157
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7160?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/models/pool.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvcG9vbC5weQ==) | `97.36% <ø> (ø)` | :arrow_up: |
   | [airflow/models/baseoperator.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvYmFzZW9wZXJhdG9yLnB5) | `96.26% <100%> (ø)` | :arrow_up: |
   | [airflow/models/taskinstance.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvdGFza2luc3RhbmNlLnB5) | `95% <100%> (+0.07%)` | :arrow_up: |
   | [airflow/ti\_deps/deps/pool\_slots\_available\_dep.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy90aV9kZXBzL2RlcHMvcG9vbF9zbG90c19hdmFpbGFibGVfZGVwLnB5) | `100% <100%> (ø)` | :arrow_up: |
   | [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7160/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/7160/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/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
   | [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
   | [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.31% <0%> (-20.49%)` | :arrow_down: |
   | [airflow/contrib/hooks/gcp\_container\_hook.py](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL2djcF9jb250YWluZXJfaG9vay5weQ==) | `100% <0%> (ø)` | :arrow_up: |
   | ... and [3 more](https://codecov.io/gh/apache/airflow/pull/7160/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7160?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/7160?src=pr&el=footer). Last update [a0568b9...491b222](https://codecov.io/gh/apache/airflow/pull/7160?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 #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)
URL: https://github.com/apache/airflow/pull/7160#discussion_r366305319
 
 

 ##########
 File path: airflow/ti_deps/deps/pool_slots_available_dep.py
 ##########
 @@ -62,9 +62,9 @@ def _get_dep_statuses(self, ti, session, dep_context=None):
             open_slots = pools[0].open_slots()
 
         if ti.state in STATES_TO_COUNT_AS_RUNNING:
-            open_slots += 1
+            open_slots += ti.pool_capacity
 
-        if open_slots <= 0:
+        if open_slots <= (ti.pool_capacity - 1):
             yield self._failing_status(
                 reason=("Not scheduling since there are %s open slots in pool %s",
 
 Review comment:
   Extend this message to say how many slots we are looking for.

----------------------------------------------------------------
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] lokeshlal commented on issue #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)

Posted by GitBox <gi...@apache.org>.
lokeshlal commented on issue #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)
URL: https://github.com/apache/airflow/pull/7160#issuecomment-574255590
 
 
   > > The other thing you will need to do is add support for this column in to the Serialized DAG format.
   > > We should add this as an (optional) field in to `airflow/serialization/schema.json` and most things should be handled already. The important thing to test is that the existing "ground truth" dag in `tests/serializtion/test_dag_serialization.py` should have this field set correctly when it is deserializaed without having to update the JSON blob -- that ensures that when upgrading this will behave itself. Please add tests covering that.
   > 
   > I have added automated test covering this case - so that in the future this should be apparent that you should do it #7162
   
   I will wait for your inputs on #7162 .

----------------------------------------------------------------
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] lokeshlal commented on issue #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)

Posted by GitBox <gi...@apache.org>.
lokeshlal commented on issue #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)
URL: https://github.com/apache/airflow/pull/7160#issuecomment-574206056
 
 
   Thank you everyone for the direction. I am looking at them one by 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] lokeshlal commented on a change in pull request #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)

Posted by GitBox <gi...@apache.org>.
lokeshlal commented on a change in pull request #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)
URL: https://github.com/apache/airflow/pull/7160#discussion_r366435007
 
 

 ##########
 File path: airflow/models/taskinstance.py
 ##########
 @@ -194,6 +195,11 @@ def __init__(self, task, execution_date, state=None):
 
         self.queue = task.queue
         self.pool = task.pool
+        if hasattr(task, 'pool_capacity'):
+            self.pool_capacity = task.pool_capacity
+            if task.pool_capacity < 1:
 
 Review comment:
   moved the check to baseoperator

----------------------------------------------------------------
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 #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)

Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)
URL: https://github.com/apache/airflow/pull/7160#issuecomment-574187418
 
 
   > The other thing you will need to do is add support for this column in to the Serialized DAG format.
   > 
   > We should add this as an (optional) field in to `airflow/serialization/schema.json` and most things should be handled already. The important thing to test is that the existing "ground truth" dag in `tests/serializtion/test_dag_serialization.py` should have this field set correctly when it is deserializaed without having to update the JSON blob -- that ensures that when upgrading this will behave itself. Please add tests covering that.
   
   I have added automated test covering this case - so that in the future this should be apparent that you should do it #7162 

----------------------------------------------------------------
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] lokeshlal commented on issue #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)

Posted by GitBox <gi...@apache.org>.
lokeshlal commented on issue #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)
URL: https://github.com/apache/airflow/pull/7160#issuecomment-574097236
 
 
   Hello @dimberman, Could you please review this PR. This is same PR as earlier https://github.com/apache/airflow/pull/6975. In this PR, I have updated the migration head as suggested by potiuk. Thank you.

----------------------------------------------------------------
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 #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)

Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)
URL: https://github.com/apache/airflow/pull/7160#issuecomment-574258715
 
 
   > I will wait for your inputs on #7162 .
   
   Thanks for your patience @lokeshlal -> I think we would like to wait with that for @dimberman or @kaxil who knows best the serialisation part and agree with him how to handle the scenario of added field in BaseOperator.
   
   It happened for the first time since we introduced serialisation, so we do not have fully hashed-out scenario!
   
   Kaxil is now travelling to India and have some holidays so it might take some time (few days maybe) until we synchronise, so if you can bear with us a little more, that will be great! 
   
   Thanks for the contribution BTW. It looks great!

----------------------------------------------------------------
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] lokeshlal commented on a change in pull request #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)

Posted by GitBox <gi...@apache.org>.
lokeshlal commented on a change in pull request #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)
URL: https://github.com/apache/airflow/pull/7160#discussion_r366435766
 
 

 ##########
 File path: tests/ti_deps/deps/test_pool_slots_available_dep.py
 ##########
 @@ -40,22 +40,22 @@ def tearDown(self):
     @patch('airflow.models.Pool.open_slots', return_value=0)
     # pylint: disable=unused-argument
     def test_pooled_task_reached_concurrency(self, mock_open_slots):
-        ti = Mock(pool='test_pool')
+        ti = Mock(pool='test_pool', pool_capacity=1)
         self.assertFalse(PoolSlotsAvailableDep().is_met(ti=ti))
 
     @patch('airflow.models.Pool.open_slots', return_value=1)
     # pylint: disable=unused-argument
     def test_pooled_task_pass(self, mock_open_slots):
-        ti = Mock(pool='test_pool')
+        ti = Mock(pool='test_pool', pool_capacity=1)
 
 Review comment:
   removed pool_capacity from most of the places except few where it is absolutely required.

----------------------------------------------------------------
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] lokeshlal edited a comment on issue #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)

Posted by GitBox <gi...@apache.org>.
lokeshlal edited a comment on issue #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)
URL: https://github.com/apache/airflow/pull/7160#issuecomment-574255590
 
 
   > > The other thing you will need to do is add support for this column in to the Serialized DAG format.
   > > We should add this as an (optional) field in to `airflow/serialization/schema.json` and most things should be handled already. The important thing to test is that the existing "ground truth" dag in `tests/serializtion/test_dag_serialization.py` should have this field set correctly when it is deserializaed without having to update the JSON blob -- that ensures that when upgrading this will behave itself. Please add tests covering that.
   > 
   > I have added automated test covering this case - so that in the future this should be apparent that you should do it #7162
   
   I have added the pool_slots in schema.json. I will wait for your inputs on #7162 . Just to clarify, the test should create a dag and deserialize the dag and check it against the "ground truth" dict. and pool_slots needs to be added in the ground truth dict 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] ashb commented on a change in pull request #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)
URL: https://github.com/apache/airflow/pull/7160#discussion_r366305613
 
 

 ##########
 File path: tests/ti_deps/deps/test_pool_slots_available_dep.py
 ##########
 @@ -40,22 +40,22 @@ def tearDown(self):
     @patch('airflow.models.Pool.open_slots', return_value=0)
     # pylint: disable=unused-argument
     def test_pooled_task_reached_concurrency(self, mock_open_slots):
-        ti = Mock(pool='test_pool')
+        ti = Mock(pool='test_pool', pool_capacity=1)
         self.assertFalse(PoolSlotsAvailableDep().is_met(ti=ti))
 
     @patch('airflow.models.Pool.open_slots', return_value=1)
     # pylint: disable=unused-argument
     def test_pooled_task_pass(self, mock_open_slots):
-        ti = Mock(pool='test_pool')
+        ti = Mock(pool='test_pool', pool_capacity=1)
 
 Review comment:
   Isn't 1 the default, meaning most of these changes in tests aren't needed?

----------------------------------------------------------------
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 merged pull request #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)

Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)
URL: https://github.com/apache/airflow/pull/7160
 
 
   

----------------------------------------------------------------
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] lokeshlal commented on issue #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)

Posted by GitBox <gi...@apache.org>.
lokeshlal commented on issue #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)
URL: https://github.com/apache/airflow/pull/7160#issuecomment-575173295
 
 
   Thanks @potiuk - I have already updated schema.json file with pool_slots field. Plus this field needs to be added to the test case you have added for #7162 ... correct.

----------------------------------------------------------------
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 issue #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)

Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)
URL: https://github.com/apache/airflow/pull/7160#issuecomment-575992423
 
 
   Good Work @lokeshlal 

----------------------------------------------------------------
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] lokeshlal commented on a change in pull request #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)

Posted by GitBox <gi...@apache.org>.
lokeshlal commented on a change in pull request #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)
URL: https://github.com/apache/airflow/pull/7160#discussion_r366435391
 
 

 ##########
 File path: airflow/ti_deps/deps/pool_slots_available_dep.py
 ##########
 @@ -62,9 +62,9 @@ def _get_dep_statuses(self, ti, session, dep_context=None):
             open_slots = pools[0].open_slots()
 
         if ti.state in STATES_TO_COUNT_AS_RUNNING:
-            open_slots += 1
+            open_slots += ti.pool_capacity
 
-        if open_slots <= 0:
+        if open_slots <= (ti.pool_capacity - 1):
             yield self._failing_status(
                 reason=("Not scheduling since there are %s open slots in pool %s",
 
 Review comment:
   modified the message to 
   ```
   reason=("Not scheduling since there are %s open slots in pool %s "
                           "and require %s pool slots",
                           open_slots, pool_name, ti.pool_slots)
   ```

----------------------------------------------------------------
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] lokeshlal commented on a change in pull request #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)

Posted by GitBox <gi...@apache.org>.
lokeshlal commented on a change in pull request #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)
URL: https://github.com/apache/airflow/pull/7160#discussion_r366436018
 
 

 ##########
 File path: airflow/models/baseoperator.py
 ##########
 @@ -178,6 +178,9 @@ class derived from this one results in the creation of a task object,
     :param pool: the slot pool this task should run in, slot pools are a
         way to limit concurrency for certain tasks
     :type pool: str
+    :param pool_capacity: the number of pool slots this task should use (>= 1)
 
 Review comment:
   yes pool_slots sounds more reasonable. renamed pool_capacity to pool_slots

----------------------------------------------------------------
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 #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)
URL: https://github.com/apache/airflow/pull/7160#discussion_r366305054
 
 

 ##########
 File path: airflow/models/taskinstance.py
 ##########
 @@ -194,6 +195,11 @@ def __init__(self, task, execution_date, state=None):
 
         self.queue = task.queue
         self.pool = task.pool
+        if hasattr(task, 'pool_capacity'):
+            self.pool_capacity = task.pool_capacity
+            if task.pool_capacity < 1:
 
 Review comment:
   This check should be done when creating/setting it on the Task/operator, not here.

----------------------------------------------------------------
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 #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)

Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)
URL: https://github.com/apache/airflow/pull/7160#issuecomment-575169715
 
 
   So just update the json @lokeshlal. Here is the current message that you will get when we merge #7162. So please follow it and let us know if it's clear message
   
   ```
   ACTION NEEDED! PLEASE READ THIS CAREFULLY AND CORRECT TESTS CAREFULLY
   
   Some fields were added to the BaseOperator! Please add them to the list 
   above and make sure that you add support for DAG serialization - 
   you should add the field to `airflow/serialization/schema.json` - 
   they should have correct type defined there.
   
   Note that we do not support versioning yet so you should
   only add optional fields to BaseOperator.
   ```

----------------------------------------------------------------
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] lokeshlal removed a comment on issue #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)

Posted by GitBox <gi...@apache.org>.
lokeshlal removed a comment on issue #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)
URL: https://github.com/apache/airflow/pull/7160#issuecomment-575993366
 
 
   Thank you everyone. This was my first PR to airflow :)

----------------------------------------------------------------
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] lokeshlal commented on issue #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)

Posted by GitBox <gi...@apache.org>.
lokeshlal commented on issue #7160: [AIRFLOW-1467] Dynamic pooling via allowing tasks to use more than one pool slot (depending upon the need)
URL: https://github.com/apache/airflow/pull/7160#issuecomment-575993366
 
 
   Thank you everyone. This was my first PR to airflow :)

----------------------------------------------------------------
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