feature: support RetryStrategy for training jobs #2316

spanghal03 · 2021-04-30T17:48:29Z

Issue #, if available:

Description of changes:
Today when a customer’s training job fails with an ISE, they have to manually retry the job using their custom retry strategy. With Resilient Training feature(RTJ), sagemaker will retry customers job on ISE’s.

Adding max_retry_attempts in CreateTrainingJob and DescribeTrainingJob
Allow setting max_wait when max_retry_attempts is set

Testing done:

Added Unit and updated an Integ test

Merge Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.

General

[ x] I have read the CONTRIBUTING doc
[ x] I used the commit message format described in CONTRIBUTING
I have passed the region in to all S3 and STS clients that I've initialized as part of this change.
I have updated any necessary documentation, including READMEs and API docs (if appropriate)

Tests

[ x] I have added tests that prove my fix is effective or that my feature works (if appropriate)
[ x] I have checked that my tests are not configured for a specific region or account (if appropriate)
I have used unique_name_from_base to create resource names in integ tests (if appropriate)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

sagemaker-bot · 2021-04-30T17:51:15Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: 206c807
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

icywang86rui · 2021-04-30T17:52:43Z

src/sagemaker/estimator.py

+        if estimator.max_retry_attempts is not None:
+            train_args["retry_strategy"] = {"MaximumRetryAttempts": estimator.max_retry_attempts}
+        else:
+            train_args["retry_strategy"] = None


is it necessary to set this to None? Can we just leave it out

As per this test, if I have to assert that retry_strategy is None when max_retry_attempts are not set, then yes we have to explicitly set to None https://github.com/aws/sagemaker-python-sdk/blob/master/tests/unit/test_estimator.py#L1060 . given this my understanding is that we have to set it to None explicitly

icywang86rui · 2021-04-30T17:53:42Z

src/sagemaker/estimator.py

+                You can specify between 1 and 30 attempts.
+                If the value of attempts is greater than one, the job is retried on InternalServerFailure the same number of attempts as the value.
+                You can cap the total duration for your job by setting ``max_wait`` and ``max_run``
+                (default: ``None``)


Please explain here that if this is set to None there will be no retries.

i am wondering if we should set the default to 3 or 2.

We can set the default to 3 or 2 as we want to customers to explicitly opt-in. However I think I need to re-word it like " If the value of attempts is greater than zero, the job is retried on InternalServerFailure the same number of attempts as the value.
You can cap the total duration for your job by setting max_wait and max_run"