Named pipe based watchdog timer #83695

bchen2020 · 2022-08-18T18:41:00Z

Summary:
This diff implements a named pipe based watchdog timer (FileTimerClient and FileTimerServer). This is similar to the existing LocalTimerClient and LocalTimerServer (https://fburl.com/code/j4b9pyya).

The motivation is from the need of handling various timeout issues. The training process occasionally get stuck. We need a proper watchdog to monitor the liveness of the training processes. This timer allows the TorchElastic agent (as the watchdog) to monitor the progress of the training processes that it spawned. If a timeout occurred, he TorchElastic agent can take some action to kill the stuck process and creating a core dump for it.

LocalTimerClient and LocalTimerServer require a multiprocessing.Queue() to work. So they can only be used between multiprocessing parent and child processes.

FileTimerClient and FileTimerServer does not have such limitation.

Test Plan:

Unit Test

buck test mode/opt caffe2/test/distributed/elastic/timer:file_based_timer_test

RemoteExecution session id: reSessionID-06d70a77-043c-4d9d-b0f2-94c24460740a-tpx
Started reporting to test run: https://www.internalfb.com/intern/testinfra/testrun/844425186732666
    ✓ ListingSuccess: caffe2/test/distributed/elastic/timer:file_based_timer_test : 12 tests discovered (2.177)
    ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_happy_path (file_based_local_timer_test.FileTimerTest) (2.463)
    ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_expired_timers (file_based_local_timer_test.FileTimerServerTest) (1.889)
    ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_send_request_release (file_based_local_timer_test.FileTimerServerTest) (1.700)
    ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_valid_timers (file_based_local_timer_test.FileTimerServerTest) (1.873)
    ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_watchdog_call_count (file_based_local_timer_test.FileTimerServerTest) (1.715)
    ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_watchdog_empty_queue (file_based_local_timer_test.FileTimerServerTest) (1.609)
    ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_exception_propagation (file_based_local_timer_test.FileTimerTest) (1.633)
    ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_multiple_clients_interaction (file_based_local_timer_test.FileTimerTest) (2.189)
    ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_get_timer_recursive (file_based_local_timer_test.FileTimerTest) (2.295)
    ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_no_client (file_based_local_timer_test.FileTimerTest) (1.753)
    ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_timer (file_based_local_timer_test.FileTimerTest) (2.151)
    ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_client_interaction (file_based_local_timer_test.FileTimerTest) (1.895)
Summary
  Pass: 12
  ListingSuccess: 1
Finished test run: https://www.internalfb.com/intern/testinfra/testrun/844425186732666

Differential Revision: D38604238

facebook-github-bot · 2022-08-18T18:41:07Z

🔗 Helpful links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/83695
📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓Need help or want to give feedback on the CI? Visit our office hours

✅ No Failures (3 Pending)

As of commit 051ff2b (more details on the Dr. CI page):

Expand to see more

💚 💚 Looks good so far! There are no failures yet. 💚 💚

This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

facebook-github-bot · 2022-08-18T18:41:27Z

This pull request was exported from Phabricator. Differential Revision: D38604238

facebook-github-bot · 2022-08-19T19:41:27Z

This pull request was exported from Phabricator. Differential Revision: D38604238

facebook-github-bot · 2022-08-22T22:44:51Z

This pull request was exported from Phabricator. Differential Revision: D38604238

d4l3k · 2022-08-23T16:43:57Z

test/distributed/elastic/timer/file_based_local_timer_test.py

+
+
+# timer is not supported on windows or macos
+if not (IS_WINDOWS or IS_MACOS):


generally folks do:

@skipIf(IS_WINDOWS, "...")

I followed the same usage in local_timer_test.py. Does @skipIf work for a code block? I only see it is used on a class or a method.

d4l3k · 2022-08-23T16:44:18Z

torch/distributed/elastic/timer/file_based_local_timer.py

+            )
+        return False
+
+    def toJson(self) -> str:


snake case?

Will change it.

d4l3k · 2022-08-23T16:45:28Z

torch/distributed/elastic/timer/file_based_local_timer.py

+                "expiration_time": self.expiration_time,
+                "signal": self.signal
+            },
+            indent=None


indent defaults to None so this is a noop

Correct. I can remove it.

facebook-github-bot · 2022-08-23T17:17:18Z

This pull request was exported from Phabricator. Differential Revision: D38604238

d4l3k

seems pretty reasonable to me

d4l3k · 2022-08-23T21:54:35Z

cc @kiukchung

Summary: Pull Request resolved: pytorch#83695 This diff implements a named pipe based watchdog timer (`FileTimerClient` and `FileTimerServer`). This is similar to the existing `LocalTimerClient` and `LocalTimerServer` (https://fburl.com/code/j4b9pyya). The motivation is from the need of handling various timeout issues. The training process occasionally get stuck. We need a proper watchdog to monitor the liveness of the training processes. This timer allows the TorchElastic agent (as the watchdog) to monitor the progress of the training processes that it spawned. If a timeout occurred, he TorchElastic agent can take some action to kill the stuck process and creating a core dump for it. `LocalTimerClient` and `LocalTimerServer` require a `multiprocessing.Queue()` to work. So they can only be used between `multiprocessing` parent and child processes. `FileTimerClient` and `FileTimerServer` does not have such limitation. Test Plan: ### Unit Test ``` buck test mode/opt caffe2/test/distributed/elastic/timer:file_based_timer_test ``` ``` RemoteExecution session id: reSessionID-06d70a77-043c-4d9d-b0f2-94c24460740a-tpx Started reporting to test run: https://www.internalfb.com/intern/testinfra/testrun/844425186732666 ✓ ListingSuccess: caffe2/test/distributed/elastic/timer:file_based_timer_test : 12 tests discovered (2.177) ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_happy_path (file_based_local_timer_test.FileTimerTest) (2.463) ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_expired_timers (file_based_local_timer_test.FileTimerServerTest) (1.889) ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_send_request_release (file_based_local_timer_test.FileTimerServerTest) (1.700) ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_valid_timers (file_based_local_timer_test.FileTimerServerTest) (1.873) ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_watchdog_call_count (file_based_local_timer_test.FileTimerServerTest) (1.715) ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_watchdog_empty_queue (file_based_local_timer_test.FileTimerServerTest) (1.609) ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_exception_propagation (file_based_local_timer_test.FileTimerTest) (1.633) ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_multiple_clients_interaction (file_based_local_timer_test.FileTimerTest) (2.189) ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_get_timer_recursive (file_based_local_timer_test.FileTimerTest) (2.295) ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_no_client (file_based_local_timer_test.FileTimerTest) (1.753) ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_timer (file_based_local_timer_test.FileTimerTest) (2.151) ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_client_interaction (file_based_local_timer_test.FileTimerTest) (1.895) Summary Pass: 12 ListingSuccess: 1 Finished test run: https://www.internalfb.com/intern/testinfra/testrun/844425186732666 ``` Reviewed By: d4l3k Differential Revision: D38604238 fbshipit-source-id: f3873658f0329e9f717d37bdd2530ed58eb70685

facebook-github-bot · 2022-08-23T23:20:46Z

This pull request was exported from Phabricator. Differential Revision: D38604238

facebook-github-bot · 2022-08-24T22:14:45Z

@pytorchbot merge

(Initiating merge automatically since Phabricator Diff has merged)

pytorchmergebot · 2022-08-24T22:16:10Z

@pytorchbot successfully started a merge job. Check the current status here.
The merge job was triggered without a flag. This means that your change will be merged once all checks on your PR have passed (ETA: 0-4 Hours). If this is not the intended behavior, feel free to use some of the other merge options in the wiki.
Please reach out to the PyTorch DevX Team with feedback or questions!

github-actions · 2022-08-24T22:16:50Z

Hey @bchen2020.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

Summary: Pull Request resolved: #83695 This diff implements a named pipe based watchdog timer (`FileTimerClient` and `FileTimerServer`). This is similar to the existing `LocalTimerClient` and `LocalTimerServer` (https://fburl.com/code/j4b9pyya). The motivation is from the need of handling various timeout issues. The training process occasionally get stuck. We need a proper watchdog to monitor the liveness of the training processes. This timer allows the TorchElastic agent (as the watchdog) to monitor the progress of the training processes that it spawned. If a timeout occurred, he TorchElastic agent can take some action to kill the stuck process and creating a core dump for it. `LocalTimerClient` and `LocalTimerServer` require a `multiprocessing.Queue()` to work. So they can only be used between `multiprocessing` parent and child processes. `FileTimerClient` and `FileTimerServer` does not have such limitation. Test Plan: ### Unit Test ``` buck test mode/opt caffe2/test/distributed/elastic/timer:file_based_timer_test ``` ``` RemoteExecution session id: reSessionID-06d70a77-043c-4d9d-b0f2-94c24460740a-tpx Started reporting to test run: https://www.internalfb.com/intern/testinfra/testrun/844425186732666 ✓ ListingSuccess: caffe2/test/distributed/elastic/timer:file_based_timer_test : 12 tests discovered (2.177) ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_happy_path (file_based_local_timer_test.FileTimerTest) (2.463) ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_expired_timers (file_based_local_timer_test.FileTimerServerTest) (1.889) ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_send_request_release (file_based_local_timer_test.FileTimerServerTest) (1.700) ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_valid_timers (file_based_local_timer_test.FileTimerServerTest) (1.873) ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_watchdog_call_count (file_based_local_timer_test.FileTimerServerTest) (1.715) ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_watchdog_empty_queue (file_based_local_timer_test.FileTimerServerTest) (1.609) ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_exception_propagation (file_based_local_timer_test.FileTimerTest) (1.633) ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_multiple_clients_interaction (file_based_local_timer_test.FileTimerTest) (2.189) ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_get_timer_recursive (file_based_local_timer_test.FileTimerTest) (2.295) ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_no_client (file_based_local_timer_test.FileTimerTest) (1.753) ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_timer (file_based_local_timer_test.FileTimerTest) (2.151) ✓ Pass: caffe2/test/distributed/elastic/timer:file_based_timer_test - test_client_interaction (file_based_local_timer_test.FileTimerTest) (1.895) Summary Pass: 12 ListingSuccess: 1 Finished test run: https://www.internalfb.com/intern/testinfra/testrun/844425186732666 ``` Reviewed By: satgera, d4l3k Differential Revision: D38604238 fbshipit-source-id: bc8389f8742a02bf87c753748d9861e7046ef0b4

kit1980 · 2022-08-30T00:16:01Z

torch/distributed/elastic/timer/file_based_local_timer.py

+
+    __slots__ = ["version", "worker_pid", "scope_id", "expiration_time", "signal"]
+
+    def __init__(self, worker_pid: int, scope_id: str, expiration_time: float, signal: int = 0) -> None:


@bchen2020 This seems to violate inheritance properties, as parent class TimerRequest has a property not present in this class - worker_id. You should usually call parent's init before own init, but here it will not work because of the inheritance issue.

bchen2020 requested review from mrshenli, pritamdamania87, zhaojuanmao, rohan-varma, H-Huang, awgu and mingzhe09088 as code owners August 18, 2022 18:41

facebook-github-bot added the cla signed label Aug 18, 2022

facebook-github-bot added oncall: distributed Add this issue/PR to distributed oncall triage queue fb-exported labels Aug 18, 2022

bchen2020 force-pushed the export-D38604238 branch from f5241fb to bfba984 Compare August 19, 2022 19:41

satgera requested a review from d4l3k August 22, 2022 21:10

bchen2020 force-pushed the export-D38604238 branch from bfba984 to bbf3739 Compare August 22, 2022 22:44

d4l3k reviewed Aug 23, 2022

View reviewed changes

bchen2020 force-pushed the export-D38604238 branch from bbf3739 to 0e12266 Compare August 23, 2022 17:17

d4l3k approved these changes Aug 23, 2022

View reviewed changes

bchen2020 force-pushed the export-D38604238 branch from 0e12266 to 051ff2b Compare August 23, 2022 23:20

pytorchmergebot added the Merged label Aug 24, 2022

pytorchmergebot closed this in 3b11b80 Aug 24, 2022

kit1980 reviewed Aug 30, 2022

View reviewed changes



		# timer is not supported on windows or macos
		if not (IS_WINDOWS or IS_MACOS):


		__slots__ = ["version", "worker_pid", "scope_id", "expiration_time", "signal"]

		def __init__(self, worker_pid: int, scope_id: str, expiration_time: float, signal: int = 0) -> None:

Named pipe based watchdog timer #83695

Named pipe based watchdog timer #83695

Uh oh!

Conversation

bchen2020 commented Aug 18, 2022

Unit Test

Uh oh!

facebook-github-bot commented Aug 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful links

✅ No Failures (3 Pending)

Uh oh!

facebook-github-bot commented Aug 18, 2022

Uh oh!

facebook-github-bot commented Aug 19, 2022

Uh oh!

facebook-github-bot commented Aug 22, 2022

Uh oh!

d4l3k Aug 23, 2022

Choose a reason for hiding this comment

Uh oh!

bchen2020 Aug 23, 2022

Choose a reason for hiding this comment

Uh oh!

d4l3k Aug 23, 2022

Choose a reason for hiding this comment

Uh oh!

bchen2020 Aug 23, 2022

Choose a reason for hiding this comment

Uh oh!

d4l3k Aug 23, 2022

Choose a reason for hiding this comment

Uh oh!

bchen2020 Aug 23, 2022

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Aug 23, 2022

Uh oh!

d4l3k left a comment

Choose a reason for hiding this comment

Uh oh!

d4l3k commented Aug 23, 2022

Uh oh!

facebook-github-bot commented Aug 23, 2022

Uh oh!

facebook-github-bot commented Aug 24, 2022

Uh oh!

pytorchmergebot commented Aug 24, 2022

Uh oh!

github-actions bot commented Aug 24, 2022

Uh oh!

kit1980 Aug 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

facebook-github-bot commented Aug 18, 2022 •

edited

Loading

kit1980 Aug 30, 2022 •

edited

Loading