Repository navigation
SageMakerTrainingOperator gets ThrottlingException when listing training jobs #16299
Description
Activity
Thanks for opening your first issue here! Be sure to follow the issue template!
I believe (and this is somewhat opinionated) that the SageMakerTrainingOperator should not be responsible for renaming jobs
The reason for renaming is explained in #7598
Do you have alternative solution?I believe (and this is somewhat opinionated) that the SageMakerTrainingOperator should not be responsible for renaming jobs
The reason for renaming is explained in #7598
Do you have alternative solution?I guess the solution depends on one's opinion: as I mentioned above I believe the operator should not rename the training job it is creating, and making sure the name being used is unique should be done elsewhere (and is the responsibility of whoever is using the operator). Exactly how they ensure this uniqueness (and listing existing jobs is one solution among others) is up to them. If that's out of the question, then perhaps a switch allowing the user to decide whether to check for uniqueness or not would do.
Perhaps the author of the original PR (@BasPH) has an opinion? Did you deal with a situation where the AWS account used by SageMaker had hundreds of existing jobs and the listing operation was throttled?
@eladkal I'm happy to take a shot at opening a PR for this, but before I do I'd like to validate my idea.
Removing the uniqueness check entirely might cause issues for users who rely on it (whether they know that they rely on it or not), so the better solution, even though it bloats the code, is probably to add a parameter to the operator to indicate whether it should perform this check or not.
I would imagine it to be simply something like a
check_job_name_uniquenessparameter (or something more concise), which essentially skips this whole block if set to true.Does that make sense to you?
- added a commit that references this issue
on Jun 16, 2021 @olivermeyer - you described precisely the way we work and the problem we are now facing trying to migrate to airflow 2.0
not sure how this didn't resonate more - probably not many use airflow with sagemaker.
regarding the single responsibility of the operator - i could not agree more , and in fact we are working like this for 3 years now, creating our own semi-random job names, and its fine - this definitely should not be implemented within the operator.
(specially not by checking existing jobs...)we are facing the same for other operators as well - like processing jobs and transform jobs.
do you know if there any plans improving them as well ?regards
Shlomiprobably not many use airflow with sagemaker.
Many use sagemaker. It's popular service.
i'd like to contribute here as well - do you have some kind of a one pager how to start ? regards Shlomi
https://github.com/apache/airflow/blob/main/CONTRIBUTORS_QUICK_START.rst
@eladkal - many use sagemaker , but i guess not as much through airflow as they do with simple python scripts.
otherwise i don't understand how some basic stuff that is not working properly is not handled.otherwise i don't understand how some basic stuff that is not working properly is not handled.
There is not much to understand. Airflow has almost 1900 contributors. Some of them - like you @shlomiken use and contribute specific features. Airflow has > 70 integrations and SageMaker is probably less than 0.1% of Airflow use. So it's possible it has some missing features.
Users like you are the best to verify, update, tests and provide PRs to improve this part. This is a free software, that you get for free without any guarantees that all the details are working as you want, but also you get the powers, freedom and possibility to pay back for the free software by contributing back in the area that you have expertise and possibility to test it.Lookinf forward to your contributions!
Apache Airflow version: 1.10.15 (also applies to 2.X)
Environment:
What happened:
I am currently upgrading an Airflow deployment from 1.10.15 to 2.1.0. While doing so, I switched over from
airflow.contrib.operators.sagemaker_training_operator.SageMakerTrainingOperatortoairflow.providers.amazon.aws.operators.sagemaker_training.SageMakerTrainingOperator, and found that some DAGs started failing at every run after that.I dug into the issue a bit, and found that the problem comes from the operator listing existing training jobs (here). This method calls
boto3'slist_training_jobsover and over again, enough times to get rate limited with a single operator if the number of existing training jobs is high enough - AWS does not allow to delete existing jobs, so these can easily be in the hundreds if not more. Since the operator does not allow to passmax_resultsto the hook's method (although the method can take it), the default page size is used (10) and the number of requests can explode. With a single operator, I was able to mitigate the issue by using the standard retry handler (instead of the legacy handler) - see doc. However, even the standard retry handler does not help in our case, where we have a dozen operators firing at the same time. All of them got rate limited again, and I was unable to make the job succeed.One technical fix would be to use a dedicated pool with 1 slot, thereby effectively running all training jobs sequentially. However that won't do in the real world: SageMaker jobs are often long-running, and we cannot afford to go from 1-2 hours when executing them in parallel, to 10-20 hours sequentially.
I believe (and this is somewhat opinionated) that the
SageMakerTrainingOperatorshould not be responsible for renaming jobs, for two reasons: (1) single responsibility principle (my operator should trigger a SageMaker job, not figure out the correct name + trigger it); (2) alignment between operators and the systems they interact with: running this list operation is, until AWS allows to somehow delete old jobs and/or dramatically increases rate limits, not aligned with the way AWS works.What you expected to happen:
The
SageMakerTrainingOperatorshould not be limited in parallelism by the number of existing training jobs in AWS. This limitation is a side-effect of listing existing training jobs. Therefore, theSageMakerTrainingOperatorshould not list existing training jobs.Anything else we need to know:
x.log
Traceback (most recent call last): File "/usr/local/lib/python3.7/site-packages/airflow/models/taskinstance.py", line 984, in _run_raw_task result = task_copy.execute(context=context) File "/usr/local/lib/python3.7/site-packages/airflow/providers/amazon/aws/operators/sagemaker_training.py", line 97, in execute training_jobs = self.hook.list_training_jobs(name_contains=training_job_name) File "/usr/local/lib/python3.7/site-packages/airflow/providers/amazon/aws/hooks/sagemaker.py", line 888, in list_training_jobs list_training_jobs_request, "TrainingJobSummaries", max_results=max_results File "/usr/local/lib/python3.7/site-packages/airflow/providers/amazon/aws/hooks/sagemaker.py", line 945, in _list_request response = partial_func(**kwargs) File "/usr/local/lib/python3.7/site-packages/botocore/client.py", line 337, in _api_call return self._make_api_call(operation_name, kwargs) File "/usr/local/lib/python3.7/site-packages/botocore/client.py", line 656, in _make_api_call raise error_class(parsed_response, operation_name) botocore.exceptions.ClientError: An error occurred (ThrottlingException) when calling the ListTrainingJobs operation (reached max retries: 9): Rate exceeded