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 2022/03/20 08:51:00 UTC
[GitHub] [airflow] vladyslavnsa opened a new issue #22381: AthenaOperator retries max_tries mix-up
vladyslavnsa opened a new issue #22381:
URL: https://github.com/apache/airflow/issues/22381
### Apache Airflow version
2.2.4 (latest released)
### What happened
After a recent upgrade from 1.10.9 to 2.2.4, an odd behavior where the aforementioned Attributes, are wrongfully Coupled, is observed.
An example to showcase the issue:
```
AthenaOperator(
...
retries=3,
max_tries=30,
...)
```
Related Documentation states:
* retries: Number of retries that should be performed before failing the task
* max_tries: Number of times to poll for query state before function exits
Regardless of the above specification `max_tries=30`, inspection of related _Task Instance Details_ shows that the Value of both Attributes is **3**
### What you think should happen instead
_No response_
### How to reproduce
_No response_
### Operating System
Ubuntu 20.04.3 LTS
### Versions of Apache Airflow Providers
apache-airflow-providers-amazon==3.0.0
### Deployment
Official Apache Airflow Helm Chart
### Deployment details
_No response_
### Anything else
Imagine a Query, executed on an hourly basis, with a varying scope, causing it to 'organically' execute for anywhere between 5 - 10 minutes. This Query Task should Fail after 3 execution attempts.
In such cases, we would like to poll the state of the Query frequently (every 15 seconds), in order to avoid redundant idle time for downstream Tasks.
A configuration matching the above description:
```
AthenaOperator(
...
retry_delay=15,
retries=3,
max_tries=40, # 40 polls * 15 seconds delay between polls = 10 minutes
...)
```
When deployed, `retries == max_tries == 3`, thus causing the Task to terminate after 45 seconds
In order to quickly avert this situation where our ETL breaks, we are using the following configuration:
```
AthenaOperator(
...
retry_delay=15,
retries=40,
max_tries=40,
...)
```
With the last configuration, our Task does not terminate preemptively but will retry **40 times** before failing - which causes an issue with downstream Tasks SLA, at the very least (that is before weighing in the waste of time and operational costs)
### Are you willing to submit PR?
- [ ] Yes I am willing to submit a PR!
### Code of Conduct
- [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] boring-cyborg[bot] commented on issue #22381: AthenaOperator retries max_tries mix-up
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on issue #22381:
URL: https://github.com/apache/airflow/issues/22381#issuecomment-1073202145
Thanks for opening your first issue here! Be sure to follow the issue template!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SamWheating edited a comment on issue #22381: AthenaOperator retries max_tries mix-up
Posted by GitBox <gi...@apache.org>.
SamWheating edited a comment on issue #22381:
URL: https://github.com/apache/airflow/issues/22381#issuecomment-1074587165
I think that this was introduced in https://github.com/apache/airflow/pull/21018, so its only included in 2.2.4:
Specifically this addition of this line to the TaskInstance constructor:
https://github.com/apache/airflow/blob/d6031f97ab9fd2666cfbcf5a851b26e2ceb010c6/airflow/models/taskinstance.py#L484
As well as the removal of the `max_tries` field from `refresh_from_task`, which means the the max_tries field is only set based on the DB value, which refers to the task_instance max_retries.
It looks like the only operators which use an argument called `max_tries` are the AthenaOperator.
I think that the easiest way to fix this would be to just rename the `max_tries` parameter on those operators (and start the deprecation of the `max_tries` parameter). Thoughts?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on issue #22381: AthenaOperator retries max_tries mix-up
Posted by GitBox <gi...@apache.org>.
eladkal commented on issue #22381:
URL: https://github.com/apache/airflow/issues/22381#issuecomment-1075297363
This happens because `AthenaOperator` has [`max_tries`](https://github.com/apache/airflow/blob/fa655fe53dcf34a7b581fe3e881d770921dfc7a0/airflow/providers/amazon/aws/operators/athena.py#L72) parameter which conflicts with the one added in https://github.com/apache/airflow/pull/21018 by @yuqian90
To fix this we probably need to make a breaking change to `AthenaOperator` and rename the parameter @potiuk 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] yuqian90 commented on issue #22381: AthenaOperator retries max_tries mix-up
Posted by GitBox <gi...@apache.org>.
yuqian90 commented on issue #22381:
URL: https://github.com/apache/airflow/issues/22381#issuecomment-1081260212
@eladkal `max_tries` is an attribute of `TaskInstance`. #21018 is basically fixing a bug of the web UI to not reset its value by mistake when someone opens the Task instance web page. It's not clear to me how it conflicts with `AthenaOperator` which is a `BaseOperator` instead of `TaskInstance`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #22381: AthenaOperator retries max_tries mix-up
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #22381:
URL: https://github.com/apache/airflow/issues/22381#issuecomment-1073228758
Would you like to fix the problem maybe by creating a PR @vladyslavnsa ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SamWheating commented on issue #22381: AthenaOperator retries max_tries mix-up
Posted by GitBox <gi...@apache.org>.
SamWheating commented on issue #22381:
URL: https://github.com/apache/airflow/issues/22381#issuecomment-1074587165
I think that this was introduced in https://github.com/apache/airflow/pull/21018, so its only included in 2.2.4:
Specifically this addition of this line to the TaskInstance constructor:
https://github.com/apache/airflow/blob/d6031f97ab9fd2666cfbcf5a851b26e2ceb010c6/airflow/models/taskinstance.py#L484
It looks like the only operators which use an argument called `max_tries` are the AthenaOperator.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SamWheating edited a comment on issue #22381: AthenaOperator retries max_tries mix-up
Posted by GitBox <gi...@apache.org>.
SamWheating edited a comment on issue #22381:
URL: https://github.com/apache/airflow/issues/22381#issuecomment-1074587165
I think that this was introduced in https://github.com/apache/airflow/pull/21018, so its only included in 2.2.4:
Specifically this addition of this line to the TaskInstance constructor:
https://github.com/apache/airflow/blob/d6031f97ab9fd2666cfbcf5a851b26e2ceb010c6/airflow/models/taskinstance.py#L484
As well as the removal of the `max_tries` field from `refresh_from_task`, which means the the max_tries field is only set based on the DB value, which refers to the task_instance max_retries.
It looks like the only operators which use an argument called `max_tries` are the AthenaOperator.
I think that the easiest way to fix this would be to just rename the `max_tried` parameter on those operator (and start the deprecation of the `max_tries` parameter). Thoughts?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org