Skip to content

Commit 1b72e01

Browse files
lukebaumanncopybara-github
authored andcommitted
Add retry_policy and deprecate max_retries in pathwaysutils.elastic.manager.elastic_retry.
PiperOrigin-RevId: 932676888
1 parent d395a90 commit 1b72e01

1 file changed

Lines changed: 65 additions & 20 deletions

File tree

pathwaysutils/elastic/manager.py

Lines changed: 65 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
import functools
2323
import logging
2424
import threading
25-
from typing import Any, TypeVar
25+
from typing import Any, TypeAlias, TypeVar
2626
import warnings
2727

2828
import jax
@@ -48,6 +48,7 @@ class ScaleUpSignalError(Exception):
4848

4949

5050
_F = TypeVar("_F", bound=Callable[..., Any])
51+
RetryPolicy: TypeAlias = Callable[[int, Exception], bool]
5152

5253

5354
def _elastic_event_cleanup() -> None:
@@ -66,6 +67,19 @@ def _elastic_event_cleanup() -> None:
6667
array.delete()
6768

6869

70+
class ElasticRetryLimit:
71+
"""A retry callback that limits the number of attempts."""
72+
73+
def __init__(self, max_attempts: int):
74+
if max_attempts <= 0:
75+
raise ValueError("max_attempts must be positive.")
76+
self.max_attempts = max_attempts
77+
78+
def __call__(self, attempt: int, error: Exception) -> bool:
79+
del error # Unused
80+
return attempt < self.max_attempts
81+
82+
6983
class Manager:
7084
"""Utility class for elastic training.
7185
@@ -191,12 +205,14 @@ def _monitor_new_slices(
191205

192206
def elastic_retry(
193207
self,
194-
max_retries: int,
208+
# TODO: b/523384843 - Remove max_retries parameter.
209+
max_retries: int | None = None,
195210
minimum_slice_count: int | None = None,
196211
poll_interval: float | int = 10,
197212
timeout: float | None = None,
198213
pre_callback: Callable[..., Any] | None = None,
199214
on_elastic_event_callback: Callable[..., Any] | None = None,
215+
retry_policy: RetryPolicy | None = None,
200216
) -> Callable[[_F], _F]:
201217
"""Retries a function with elasticity fault tolerance.
202218
@@ -224,6 +240,8 @@ def elastic_retry(
224240
225241
Args:
226242
max_retries: The maximum number of times to retry the function.
243+
Deprecated: Use `retry_policy` instead.
244+
TODO: b/523384843 - Remove max_retries parameter.
227245
minimum_slice_count: The minimum number of slices required to run the
228246
function. If None, defaults to the total number of slices.
229247
poll_interval: The number of seconds to wait between activity checks.
@@ -233,6 +251,11 @@ def elastic_retry(
233251
pre_callback: A callback to call before the function is attempted.
234252
on_elastic_event_callback: A callback to call after an elastic failure
235253
occurs.
254+
retry_policy: A policy (callable) to determine if a retry should be
255+
attempted. It accepts the attempt number (1-indexed) and the exception
256+
that triggered the retry. If it returns False, no more retries are
257+
attempted. If neither `retry_policy` nor `max_retries` is specified, it
258+
defaults to unlimited retries.
236259
237260
Returns:
238261
A decorator that retries the wrapped function.
@@ -248,17 +271,24 @@ def elastic_retry(
248271
else minimum_slice_count
249272
)
250273

251-
if max_retries <= 0:
252-
raise ValueError("max_retries must be positive.")
274+
if max_retries is not None and retry_policy is not None:
275+
raise ValueError("Cannot specify both max_retries and retry_policy.")
276+
277+
if retry_policy is None:
278+
if max_retries is None:
279+
# Default to unlimited retries if neither parameter is supplied.
280+
retry_policy = lambda attempt, error: True
281+
else:
282+
if max_retries <= 0:
283+
raise ValueError("max_retries must be positive.")
284+
retry_policy = ElasticRetryLimit(max_retries)
253285

254286
def decorator(func: _F) -> _F:
255287
@functools.wraps(func)
256288
def wrapper(*args: Any, **kwargs: Any) -> Any:
257289

258-
def attempt_execution(retry_index: int) -> Any:
259-
_logger.info(
260-
"Elastic attempt %d out of %d", retry_index + 1, max_retries
261-
)
290+
def attempt_execution(attempt: int) -> Any:
291+
_logger.info("Elastic attempt %d", attempt)
262292
self.active_slice_indices = elastic.wait_for_slices(
263293
slice_count=target_slice_count,
264294
slice_to_devices=self.slice_to_devices,
@@ -289,34 +319,49 @@ def attempt_execution(retry_index: int) -> Any:
289319
if monitor_thread is not None:
290320
monitor_thread.join()
291321

292-
for retry_index in range(max_retries):
322+
attempt = 1
323+
while True:
293324
try:
294-
return attempt_execution(retry_index)
295-
except ScaleUpSignalError:
296-
_logger.info("Scale up requested. Retrying.")
325+
return attempt_execution(attempt)
326+
except ScaleUpSignalError as error:
327+
_logger.info("Scale up requested.")
297328
_elastic_event_cleanup()
298329

299330
if on_elastic_event_callback is not None:
300331
on_elastic_event_callback()
332+
333+
if not retry_policy(attempt, error):
334+
_logger.info(
335+
"Retry policy rejected retry after ScaleUpSignalError."
336+
)
337+
raise ElasticRuntimeError(
338+
f"Elastic attempt {attempt} failed."
339+
) from error
340+
341+
_logger.info("Retrying.")
301342
except jax.errors.JaxRuntimeError as error:
302343
if not elastic.is_error_due_to_slice_down(error):
303344
raise
304345

305346
if self.new_slice_event.is_set():
306-
_logger.info(
307-
"Slice down event and new slice available detected. Retrying."
308-
)
347+
_logger.info("Slice down event and new slice available detected.")
309348
else:
310-
_logger.info("Slice down event detected. Retrying.")
349+
_logger.info("Slice down event detected.")
311350

312351
_elastic_event_cleanup()
313352

314353
if on_elastic_event_callback is not None:
315354
on_elastic_event_callback()
316-
else:
317-
raise ElasticRuntimeError(
318-
f"Elastic attempt {max_retries} out of {max_retries} failed."
319-
)
355+
356+
if not retry_policy(attempt, error):
357+
_logger.info("Retry policy rejected retry after JaxRuntimeError.")
358+
raise ElasticRuntimeError(
359+
f"Elastic attempt {attempt} failed."
360+
) from error
361+
362+
_logger.info("Retrying.")
363+
364+
attempt += 1
320365

321366
return wrapper
322367

0 commit comments

Comments
 (0)