Re: [PR] Fix EMR Serverless task failure on transient AWS throttling errors [airflow]
vincbeck commented on code in PR #67222:
URL: https://github.com/apache/airflow/pull/67222#discussion_r3320758928
##
providers/amazon/src/airflow/providers/amazon/aws/utils/waiter_with_logging.py:
##
@@ -87,11 +112,23 @@ def wait(
and isinstance(last_response.get("Error"), dict)
and "Code" in last_response.get("Error")
):
-raise AirflowException(f"{failure_message}: {error}")
+error_code = last_response["Error"]["Code"]
+if error_code not in RETRIABLE_ERROR_CODES:
+raise AirflowException(f"{failure_message}: {error}")
+
+log.info(
+"Waiter encountered retriable error: %s. Retrying (attempt
%d/%d)...",
+error_code,
+attempt + 1,
+waiter_max_attempts,
+)
+# Don't increment attempt counter for retriable errors;
continue looping
Review Comment:
@Subham-KRLX would you able to fix 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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Fix EMR Serverless task failure on transient AWS throttling errors [airflow]
ferruzzi commented on code in PR #67222:
URL: https://github.com/apache/airflow/pull/67222#discussion_r3320708911
##
providers/amazon/src/airflow/providers/amazon/aws/utils/waiter_with_logging.py:
##
@@ -87,11 +112,23 @@ def wait(
and isinstance(last_response.get("Error"), dict)
and "Code" in last_response.get("Error")
):
-raise AirflowException(f"{failure_message}: {error}")
+error_code = last_response["Error"]["Code"]
+if error_code not in RETRIABLE_ERROR_CODES:
+raise AirflowException(f"{failure_message}: {error}")
+
+log.info(
+"Waiter encountered retriable error: %s. Retrying (attempt
%d/%d)...",
+error_code,
+attempt + 1,
+waiter_max_attempts,
+)
+# Don't increment attempt counter for retriable errors;
continue looping
Review Comment:
Yeah, looks like if the service keeps returning a retriable error then this
will never exit.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Fix EMR Serverless task failure on transient AWS throttling errors [airflow]
o-nikolas commented on code in PR #67222:
URL: https://github.com/apache/airflow/pull/67222#discussion_r3320644077
##
providers/amazon/src/airflow/providers/amazon/aws/utils/waiter_with_logging.py:
##
@@ -87,11 +112,23 @@ def wait(
and isinstance(last_response.get("Error"), dict)
and "Code" in last_response.get("Error")
):
-raise AirflowException(f"{failure_message}: {error}")
+error_code = last_response["Error"]["Code"]
+if error_code not in RETRIABLE_ERROR_CODES:
+raise AirflowException(f"{failure_message}: {error}")
+
+log.info(
+"Waiter encountered retriable error: %s. Retrying (attempt
%d/%d)...",
+error_code,
+attempt + 1,
+waiter_max_attempts,
+)
+# Don't increment attempt counter for retriable errors;
continue looping
Review Comment:
Couldn't this get us stuck in infinite loops? @ferruzzi you've worked most
on waiters, what are your thoughts 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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Fix EMR Serverless task failure on transient AWS throttling errors [airflow]
eladkal merged PR #67222: URL: https://github.com/apache/airflow/pull/67222 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Fix EMR Serverless task failure on transient AWS throttling errors [airflow]
eladkal commented on code in PR #67222:
URL: https://github.com/apache/airflow/pull/67222#discussion_r327991
##
providers/amazon/src/airflow/providers/amazon/aws/utils/waiter_with_logging.py:
##
@@ -30,6 +30,28 @@
if TYPE_CHECKING:
from botocore.waiter import Waiter
+# Standard throttling and transient error codes to retry on
+# https://docs.aws.amazon.com/general/latest/gr/api-retries.html
+# and https://github.com/boto/botocore/blob/develop/botocore/retryhandler.py
+RETRIABLE_ERROR_CODES = {
Review Comment:
If they are stwble I am fine with it.
Feel free to followup with upstream later if they can manage the list from
thier side
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Fix EMR Serverless task failure on transient AWS throttling errors [airflow]
eladkal commented on code in PR #67222:
URL: https://github.com/apache/airflow/pull/67222#discussion_r327991
##
providers/amazon/src/airflow/providers/amazon/aws/utils/waiter_with_logging.py:
##
@@ -30,6 +30,28 @@
if TYPE_CHECKING:
from botocore.waiter import Waiter
+# Standard throttling and transient error codes to retry on
+# https://docs.aws.amazon.com/general/latest/gr/api-retries.html
+# and https://github.com/boto/botocore/blob/develop/botocore/retryhandler.py
+RETRIABLE_ERROR_CODES = {
Review Comment:
If they are stable I am fine with it.
Feel free to followup with upstream later if they can manage the list from
thier side
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Fix EMR Serverless task failure on transient AWS throttling errors [airflow]
Subham-KRLX commented on code in PR #67222:
URL: https://github.com/apache/airflow/pull/67222#discussion_r3279193691
##
providers/amazon/src/airflow/providers/amazon/aws/utils/waiter_with_logging.py:
##
@@ -30,6 +30,28 @@
if TYPE_CHECKING:
from botocore.waiter import Waiter
+# Standard throttling and transient error codes to retry on
+# https://docs.aws.amazon.com/general/latest/gr/api-retries.html
+# and https://github.com/boto/botocore/blob/develop/botocore/retryhandler.py
+RETRIABLE_ERROR_CODES = {
Review Comment:
These error codes are from AWS documentation and are pretty stable but you
are right that we'd need to maintain the list manually for now I think the
hardcoded approach is fine since these codes change infrequently we could
definitely explore importing them from botocore in a future improvement. Let me
know if you'd prefer I investigate that option for this PR or if the documented
list works for now.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Fix EMR Serverless task failure on transient AWS throttling errors [airflow]
eladkal commented on code in PR #67222:
URL: https://github.com/apache/airflow/pull/67222#discussion_r3279153547
##
providers/amazon/src/airflow/providers/amazon/aws/utils/waiter_with_logging.py:
##
@@ -30,6 +30,28 @@
if TYPE_CHECKING:
from botocore.waiter import Waiter
+# Standard throttling and transient error codes to retry on
+# https://docs.aws.amazon.com/general/latest/gr/api-retries.html
+# and https://github.com/boto/botocore/blob/develop/botocore/retryhandler.py
+RETRIABLE_ERROR_CODES = {
Review Comment:
Must we maintaine the list from our side? If a new code would be introduced
we won't get it.
I wonder if it make sense, maybe, to ask upstream to maintain the list from
thier side and we will import it?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Fix EMR Serverless task failure on transient AWS throttling errors [airflow]
eladkal commented on code in PR #67222:
URL: https://github.com/apache/airflow/pull/67222#discussion_r3279153547
##
providers/amazon/src/airflow/providers/amazon/aws/utils/waiter_with_logging.py:
##
@@ -30,6 +30,28 @@
if TYPE_CHECKING:
from botocore.waiter import Waiter
+# Standard throttling and transient error codes to retry on
+# https://docs.aws.amazon.com/general/latest/gr/api-retries.html
+# and https://github.com/boto/botocore/blob/develop/botocore/retryhandler.py
+RETRIABLE_ERROR_CODES = {
Review Comment:
Must we mantain the list from our side? If a new code would be introduced we
won't get it.
I wonder if it make sense, maybe, to ask upstream to maintain the list from
thier side and we will import it?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Fix EMR Serverless task failure on transient AWS throttling errors [airflow]
Subham-KRLX commented on code in PR #67222: URL: https://github.com/apache/airflow/pull/67222#discussion_r3278135358 ## providers/amazon/src/airflow/providers/amazon/aws/utils/waiter_with_logging.py: ## @@ -30,6 +30,28 @@ if TYPE_CHECKING: from botocore.waiter import Waiter +# Standard throttling and transient error codes to retry on +# https://docs.aws.amazon.com/general/latest/gr/api-retries.html +# and https://github.com/boto/botocore/blob/e25e9be5ea42a1eb5097f48b11c977d4c38d5c4b/botocore/retryhandler.py Review Comment: The link pointed to a stale commit hash so I have updated it to point to the active `develop` branch of `botocore` and pushed the fix. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Fix EMR Serverless task failure on transient AWS throttling errors [airflow]
eladkal commented on code in PR #67222: URL: https://github.com/apache/airflow/pull/67222#discussion_r3272264887 ## providers/amazon/src/airflow/providers/amazon/aws/utils/waiter_with_logging.py: ## @@ -30,6 +30,28 @@ if TYPE_CHECKING: from botocore.waiter import Waiter +# Standard throttling and transient error codes to retry on +# https://docs.aws.amazon.com/general/latest/gr/api-retries.html +# and https://github.com/boto/botocore/blob/e25e9be5ea42a1eb5097f48b11c977d4c38d5c4b/botocore/retryhandler.py Review Comment: link doesn't work -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Fix EMR Serverless task failure on transient AWS throttling errors [airflow]
eladkal commented on PR #67222: URL: https://github.com/apache/airflow/pull/67222#issuecomment-4494881071 > #Thanks @Subham-KRLX for taking this up on priority. We are blocked to release our builds to prod environments due to this issue. Do you have any work-around for this? If this gets merged, how soon it will be rolled out? You are not blocked. see https://github.com/apache/airflow/issues/67178#issuecomment-4494869135 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Fix EMR Serverless task failure on transient AWS throttling errors [airflow]
ROOBALJINDAL commented on PR #67222: URL: https://github.com/apache/airflow/pull/67222#issuecomment-4494767215 Thanks @Subham-KRLX for taking this up on priority. We are blocked to release our builds to prod environments due to this issue. Do you have any work-around for this? If this gets merged, how soon it will be rolled out? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
