Skip to content

Commit 91ee987

Browse files
JustinPan-googOrbax Authors
authored andcommitted
Internal.
PiperOrigin-RevId: 935681934
1 parent cff966f commit 91ee987

3 files changed

Lines changed: 39 additions & 6 deletions

File tree

checkpoint/orbax/checkpoint/_src/path/atomicity_defaults.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
from orbax.checkpoint._src.path import atomicity
2424
from orbax.checkpoint._src.path import atomicity_types
2525
from orbax.checkpoint._src.path import gcs_utils
26+
from orbax.checkpoint._src.path import utils
2627

2728

2829
def get_item_default_temporary_path_class(

checkpoint/orbax/checkpoint/_src/path/deleter.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
from orbax.checkpoint._src.path import step as step_lib
3535
from orbax.checkpoint._src.path import utils as path_utils
3636

37+
3738
urlparse = parse.urlparse
3839
PurePosixPath = pathlib.PurePosixPath
3940

@@ -403,6 +404,7 @@ def __init__(
403404
num_threads=num_threads,
404405
)
405406
self._delete_queue = queue.Queue()
407+
self._exception = None
406408
# Turn on daemon=True so the thread won't block the main thread and die
407409
# when the program exits.
408410
self._delete_thread = threading.Thread(
@@ -417,10 +419,14 @@ def __init__(
417419
def _delete_thread_run(self) -> None:
418420
logging.info('Delete thread has started.')
419421
while True:
420-
step = self._delete_queue.get(block=True)
421-
if step < 0:
422+
try:
423+
step = self._delete_queue.get(block=True)
424+
if step < 0:
425+
break
426+
self._standard_deleter.delete(step)
427+
except Exception as e: # pylint: disable=broad-exception-caught
428+
self._exception = e
422429
break
423-
self._standard_deleter.delete(step)
424430
logging.info('Delete thread exited.')
425431

426432
def delete(self, step: int) -> None:
@@ -437,6 +443,9 @@ def close(self) -> None:
437443
self._delete_queue.put(-1)
438444
self._delete_thread.join()
439445

446+
if self._exception:
447+
raise self._exception
448+
440449
self._standard_deleter.close()
441450

442451

checkpoint/orbax/checkpoint/_src/path/deleter_test.py

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,7 @@ def _get_save_diretory(self, step: int, directory: epath.Path) -> epath.Path:
3939
threaded=(False, True),
4040
todelete_subdir=(None, 'some_delete_dir'),
4141
)
42-
def test_checkpoint_deleter_delete(
43-
self, threaded, todelete_subdir
44-
):
42+
def test_checkpoint_deleter_delete(self, threaded, todelete_subdir):
4543
"""Test regular CheckpointDeleter."""
4644
deleter = deleter_lib.create_checkpoint_deleter(
4745
self.ckpt_dir,
@@ -158,6 +156,30 @@ def test_checkpoint_deleter_auto_threads_gcs(self):
158156
self.assertEqual(num_threads, 1)
159157

160158

159+
160+
class ThreadedCheckpointDeleterExceptionTest(parameterized.TestCase):
161+
162+
def test_exception_bubbling_on_close(self):
163+
"""Verifies exceptions in background thread are raised on close()."""
164+
mock_standard_deleter = mock.MagicMock()
165+
error_msg = 'Background deletion failed'
166+
mock_standard_deleter.delete.side_effect = ValueError(error_msg)
167+
168+
with mock.patch.object(
169+
deleter_lib,
170+
'StandardCheckpointDeleter',
171+
return_value=mock_standard_deleter,
172+
):
173+
deleter = deleter_lib.ThreadedCheckpointDeleter(
174+
epath.Path('/tmp'),
175+
name_format=step_lib.standard_name_format(),
176+
)
177+
deleter.delete(1)
178+
179+
with self.assertRaisesRegex(ValueError, error_msg):
180+
deleter.close()
181+
182+
161183
class GcsRenameTest(unittest.TestCase):
162184

163185
@mock.patch('orbax.checkpoint._src.path.deleter.epath.Path')
@@ -203,5 +225,6 @@ def test_gcs_rename_logic_directly(self, mock_epath_constructor):
203225
# Verify the Rename was actually called
204226
mock_step_path.rename.assert_called_with(mock_final_dest)
205227

228+
206229
if __name__ == '__main__':
207230
absltest.main()

0 commit comments

Comments
 (0)