[Disco] Treat hangup of disco worker process as kShutdown by Lunderberg · Pull Request #16989 · apache/tvm

Lunderberg · 2024-05-13T13:52:09Z

Prior to this commit, each disco worker needed to receive DiscoAction::kShutdown in order to close cleanly. While this is sent from the destructor of ProcessSessionObj, which owns the worker processes, this does not guarantee that the disco workers will receive the shutdown command. For example, the controller process holding the ProcessSessionObj may reach a timeout and be terminated, preventing it from sending the DiscoAction::kShutdown command.

This commit updates the disco worker to check for a closed pipe that occurs between two packets, and to treat this as if the DiscoAction::kShutdown command were received. A closed pipe that occurs at any other location is still treated as an error and reported.

Prior to this commit, each disco worker needed to receive `DiscoAction::kShutdown` in order to close cleanly. While this is sent from the destructor of `ProcessSessionObj`, which owns the worker processes, this does not guarantee that the disco workers will receive the shutdown command. For example, the controller process holding the `ProcessSessionObj` may reach a timeout and be terminated, preventing it from sending the `DiscoAction::kShutdown` command. This commit updates the disco worker to check for a closed pipe that occurs between two packets, and to treat this as if the `DiscoAction::kShutdown` command were received. A closed pipe that occurs at any other location is still treated as an error and reported.

This resolves a conflict between two recent changes. In apache#16989, reads of size zero are used to identify hangups in `ProcessSession`. In apache#16992, reads of size zero are treated as an error to avoid infinite loops while waiting for data to be ready. For a long-term resolution, the `dmlc::Stream` interface will need to be updated, so that the `Write` method returns the number of bytes written, just as the `Read` method currently does. This will allow the calling scope to verify the number of bytes received.

This resolves a conflict between two recent changes. In #16989, reads of size zero are used to identify hangups in `ProcessSession`. In #16992, reads of size zero are treated as an error to avoid infinite loops while waiting for data to be ready. For a long-term resolution, the `dmlc::Stream` interface will need to be updated, so that the `Write` method returns the number of bytes written, just as the `Read` method currently does. This will allow the calling scope to verify the number of bytes received.

Lunderberg · 2024-05-13T13:52:09Z

Prior to this commit, each disco worker needed to receive DiscoAction::kShutdown in order to close cleanly. While this is sent from the destructor of ProcessSessionObj, which owns the worker processes, this does not guarantee that the disco workers will receive the shutdown command. For example, the controller process holding the ProcessSessionObj may reach a timeout and be terminated, preventing it from sending the DiscoAction::kShutdown command.

This commit updates the disco worker to check for a closed pipe that occurs between two packets, and to treat this as if the DiscoAction::kShutdown command were received. A closed pipe that occurs at any other location is still treated as an error and reported.

Prior to this commit, each disco worker needed to receive `DiscoAction::kShutdown` in order to close cleanly. While this is sent from the destructor of `ProcessSessionObj`, which owns the worker processes, this does not guarantee that the disco workers will receive the shutdown command. For example, the controller process holding the `ProcessSessionObj` may reach a timeout and be terminated, preventing it from sending the `DiscoAction::kShutdown` command. This commit updates the disco worker to check for a closed pipe that occurs between two packets, and to treat this as if the `DiscoAction::kShutdown` command were received. A closed pipe that occurs at any other location is still treated as an error and reported.

This resolves a conflict between two recent changes. In apache#16989, reads of size zero are used to identify hangups in `ProcessSession`. In apache#16992, reads of size zero are treated as an error to avoid infinite loops while waiting for data to be ready. For a long-term resolution, the `dmlc::Stream` interface will need to be updated, so that the `Write` method returns the number of bytes written, just as the `Read` method currently does. This will allow the calling scope to verify the number of bytes received.

This resolves a conflict between two recent changes. In #16989, reads of size zero are used to identify hangups in `ProcessSession`. In #16992, reads of size zero are treated as an error to avoid infinite loops while waiting for data to be ready. For a long-term resolution, the `dmlc::Stream` interface will need to be updated, so that the `Write` method returns the number of bytes written, just as the `Read` method currently does. This will allow the calling scope to verify the number of bytes received.

Lunderberg · 2024-05-13T13:52:09Z

Prior to this commit, each disco worker needed to receive DiscoAction::kShutdown in order to close cleanly. While this is sent from the destructor of ProcessSessionObj, which owns the worker processes, this does not guarantee that the disco workers will receive the shutdown command. For example, the controller process holding the ProcessSessionObj may reach a timeout and be terminated, preventing it from sending the DiscoAction::kShutdown command.

This commit updates the disco worker to check for a closed pipe that occurs between two packets, and to treat this as if the DiscoAction::kShutdown command were received. A closed pipe that occurs at any other location is still treated as an error and reported.

Prior to this commit, each disco worker needed to receive `DiscoAction::kShutdown` in order to close cleanly. While this is sent from the destructor of `ProcessSessionObj`, which owns the worker processes, this does not guarantee that the disco workers will receive the shutdown command. For example, the controller process holding the `ProcessSessionObj` may reach a timeout and be terminated, preventing it from sending the `DiscoAction::kShutdown` command. This commit updates the disco worker to check for a closed pipe that occurs between two packets, and to treat this as if the `DiscoAction::kShutdown` command were received. A closed pipe that occurs at any other location is still treated as an error and reported.

This resolves a conflict between two recent changes. In apache#16989, reads of size zero are used to identify hangups in `ProcessSession`. In apache#16992, reads of size zero are treated as an error to avoid infinite loops while waiting for data to be ready. For a long-term resolution, the `dmlc::Stream` interface will need to be updated, so that the `Write` method returns the number of bytes written, just as the `Read` method currently does. This will allow the calling scope to verify the number of bytes received.

This resolves a conflict between two recent changes. In #16989, reads of size zero are used to identify hangups in `ProcessSession`. In #16992, reads of size zero are treated as an error to avoid infinite loops while waiting for data to be ready. For a long-term resolution, the `dmlc::Stream` interface will need to be updated, so that the `Write` method returns the number of bytes written, just as the `Read` method currently does. This will allow the calling scope to verify the number of bytes received.

Lunderberg · 2024-05-13T13:52:09Z

Prior to this commit, each disco worker needed to receive DiscoAction::kShutdown in order to close cleanly. While this is sent from the destructor of ProcessSessionObj, which owns the worker processes, this does not guarantee that the disco workers will receive the shutdown command. For example, the controller process holding the ProcessSessionObj may reach a timeout and be terminated, preventing it from sending the DiscoAction::kShutdown command.

This commit updates the disco worker to check for a closed pipe that occurs between two packets, and to treat this as if the DiscoAction::kShutdown command were received. A closed pipe that occurs at any other location is still treated as an error and reported.

Prior to this commit, each disco worker needed to receive `DiscoAction::kShutdown` in order to close cleanly. While this is sent from the destructor of `ProcessSessionObj`, which owns the worker processes, this does not guarantee that the disco workers will receive the shutdown command. For example, the controller process holding the `ProcessSessionObj` may reach a timeout and be terminated, preventing it from sending the `DiscoAction::kShutdown` command. This commit updates the disco worker to check for a closed pipe that occurs between two packets, and to treat this as if the `DiscoAction::kShutdown` command were received. A closed pipe that occurs at any other location is still treated as an error and reported.

This resolves a conflict between two recent changes. In apache#16989, reads of size zero are used to identify hangups in `ProcessSession`. In apache#16992, reads of size zero are treated as an error to avoid infinite loops while waiting for data to be ready. For a long-term resolution, the `dmlc::Stream` interface will need to be updated, so that the `Write` method returns the number of bytes written, just as the `Read` method currently does. This will allow the calling scope to verify the number of bytes received.

This resolves a conflict between two recent changes. In #16989, reads of size zero are used to identify hangups in `ProcessSession`. In #16992, reads of size zero are treated as an error to avoid infinite loops while waiting for data to be ready. For a long-term resolution, the `dmlc::Stream` interface will need to be updated, so that the `Write` method returns the number of bytes written, just as the `Read` method currently does. This will allow the calling scope to verify the number of bytes received.

Lunderberg · 2024-05-13T13:52:09Z

Prior to this commit, each disco worker needed to receive DiscoAction::kShutdown in order to close cleanly. While this is sent from the destructor of ProcessSessionObj, which owns the worker processes, this does not guarantee that the disco workers will receive the shutdown command. For example, the controller process holding the ProcessSessionObj may reach a timeout and be terminated, preventing it from sending the DiscoAction::kShutdown command.

This commit updates the disco worker to check for a closed pipe that occurs between two packets, and to treat this as if the DiscoAction::kShutdown command were received. A closed pipe that occurs at any other location is still treated as an error and reported.

Prior to this commit, each disco worker needed to receive `DiscoAction::kShutdown` in order to close cleanly. While this is sent from the destructor of `ProcessSessionObj`, which owns the worker processes, this does not guarantee that the disco workers will receive the shutdown command. For example, the controller process holding the `ProcessSessionObj` may reach a timeout and be terminated, preventing it from sending the `DiscoAction::kShutdown` command. This commit updates the disco worker to check for a closed pipe that occurs between two packets, and to treat this as if the `DiscoAction::kShutdown` command were received. A closed pipe that occurs at any other location is still treated as an error and reported.

This resolves a conflict between two recent changes. In apache#16989, reads of size zero are used to identify hangups in `ProcessSession`. In apache#16992, reads of size zero are treated as an error to avoid infinite loops while waiting for data to be ready. For a long-term resolution, the `dmlc::Stream` interface will need to be updated, so that the `Write` method returns the number of bytes written, just as the `Read` method currently does. This will allow the calling scope to verify the number of bytes received.

This resolves a conflict between two recent changes. In #16989, reads of size zero are used to identify hangups in `ProcessSession`. In #16992, reads of size zero are treated as an error to avoid infinite loops while waiting for data to be ready. For a long-term resolution, the `dmlc::Stream` interface will need to be updated, so that the `Write` method returns the number of bytes written, just as the `Read` method currently does. This will allow the calling scope to verify the number of bytes received.

Lunderberg · 2024-05-13T13:52:09Z

Prior to this commit, each disco worker needed to receive DiscoAction::kShutdown in order to close cleanly. While this is sent from the destructor of ProcessSessionObj, which owns the worker processes, this does not guarantee that the disco workers will receive the shutdown command. For example, the controller process holding the ProcessSessionObj may reach a timeout and be terminated, preventing it from sending the DiscoAction::kShutdown command.

This commit updates the disco worker to check for a closed pipe that occurs between two packets, and to treat this as if the DiscoAction::kShutdown command were received. A closed pipe that occurs at any other location is still treated as an error and reported.

Prior to this commit, each disco worker needed to receive `DiscoAction::kShutdown` in order to close cleanly. While this is sent from the destructor of `ProcessSessionObj`, which owns the worker processes, this does not guarantee that the disco workers will receive the shutdown command. For example, the controller process holding the `ProcessSessionObj` may reach a timeout and be terminated, preventing it from sending the `DiscoAction::kShutdown` command. This commit updates the disco worker to check for a closed pipe that occurs between two packets, and to treat this as if the `DiscoAction::kShutdown` command were received. A closed pipe that occurs at any other location is still treated as an error and reported.

This resolves a conflict between two recent changes. In apache#16989, reads of size zero are used to identify hangups in `ProcessSession`. In apache#16992, reads of size zero are treated as an error to avoid infinite loops while waiting for data to be ready. For a long-term resolution, the `dmlc::Stream` interface will need to be updated, so that the `Write` method returns the number of bytes written, just as the `Read` method currently does. This will allow the calling scope to verify the number of bytes received.

This resolves a conflict between two recent changes. In #16989, reads of size zero are used to identify hangups in `ProcessSession`. In #16992, reads of size zero are treated as an error to avoid infinite loops while waiting for data to be ready. For a long-term resolution, the `dmlc::Stream` interface will need to be updated, so that the `Write` method returns the number of bytes written, just as the `Read` method currently does. This will allow the calling scope to verify the number of bytes received.

Lunderberg · 2024-05-13T13:52:09Z

Prior to this commit, each disco worker needed to receive DiscoAction::kShutdown in order to close cleanly. While this is sent from the destructor of ProcessSessionObj, which owns the worker processes, this does not guarantee that the disco workers will receive the shutdown command. For example, the controller process holding the ProcessSessionObj may reach a timeout and be terminated, preventing it from sending the DiscoAction::kShutdown command.

This commit updates the disco worker to check for a closed pipe that occurs between two packets, and to treat this as if the DiscoAction::kShutdown command were received. A closed pipe that occurs at any other location is still treated as an error and reported.

Prior to this commit, each disco worker needed to receive `DiscoAction::kShutdown` in order to close cleanly. While this is sent from the destructor of `ProcessSessionObj`, which owns the worker processes, this does not guarantee that the disco workers will receive the shutdown command. For example, the controller process holding the `ProcessSessionObj` may reach a timeout and be terminated, preventing it from sending the `DiscoAction::kShutdown` command. This commit updates the disco worker to check for a closed pipe that occurs between two packets, and to treat this as if the `DiscoAction::kShutdown` command were received. A closed pipe that occurs at any other location is still treated as an error and reported.

This resolves a conflict between two recent changes. In apache#16989, reads of size zero are used to identify hangups in `ProcessSession`. In apache#16992, reads of size zero are treated as an error to avoid infinite loops while waiting for data to be ready. For a long-term resolution, the `dmlc::Stream` interface will need to be updated, so that the `Write` method returns the number of bytes written, just as the `Read` method currently does. This will allow the calling scope to verify the number of bytes received.

This resolves a conflict between two recent changes. In #16989, reads of size zero are used to identify hangups in `ProcessSession`. In #16992, reads of size zero are treated as an error to avoid infinite loops while waiting for data to be ready. For a long-term resolution, the `dmlc::Stream` interface will need to be updated, so that the `Write` method returns the number of bytes written, just as the `Read` method currently does. This will allow the calling scope to verify the number of bytes received.

Lunderberg · 2024-05-13T13:52:09Z

Prior to this commit, each disco worker needed to receive DiscoAction::kShutdown in order to close cleanly. While this is sent from the destructor of ProcessSessionObj, which owns the worker processes, this does not guarantee that the disco workers will receive the shutdown command. For example, the controller process holding the ProcessSessionObj may reach a timeout and be terminated, preventing it from sending the DiscoAction::kShutdown command.

This commit updates the disco worker to check for a closed pipe that occurs between two packets, and to treat this as if the DiscoAction::kShutdown command were received. A closed pipe that occurs at any other location is still treated as an error and reported.

Prior to this commit, each disco worker needed to receive `DiscoAction::kShutdown` in order to close cleanly. While this is sent from the destructor of `ProcessSessionObj`, which owns the worker processes, this does not guarantee that the disco workers will receive the shutdown command. For example, the controller process holding the `ProcessSessionObj` may reach a timeout and be terminated, preventing it from sending the `DiscoAction::kShutdown` command. This commit updates the disco worker to check for a closed pipe that occurs between two packets, and to treat this as if the `DiscoAction::kShutdown` command were received. A closed pipe that occurs at any other location is still treated as an error and reported.

This resolves a conflict between two recent changes. In apache#16989, reads of size zero are used to identify hangups in `ProcessSession`. In apache#16992, reads of size zero are treated as an error to avoid infinite loops while waiting for data to be ready. For a long-term resolution, the `dmlc::Stream` interface will need to be updated, so that the `Write` method returns the number of bytes written, just as the `Read` method currently does. This will allow the calling scope to verify the number of bytes received.

This resolves a conflict between two recent changes. In #16989, reads of size zero are used to identify hangups in `ProcessSession`. In #16992, reads of size zero are treated as an error to avoid infinite loops while waiting for data to be ready. For a long-term resolution, the `dmlc::Stream` interface will need to be updated, so that the `Write` method returns the number of bytes written, just as the `Read` method currently does. This will allow the calling scope to verify the number of bytes received.

masahi approved these changes May 13, 2024
View reviewed changes