Skip to content

[Core] Make Task Exit with System Error When free_objects Receives IOError #48636

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rynewang merged 10 commits into from
Nov 13, 2024
Merged

[Core] Make Task Exit with System Error When free_objects Receives IOError #48636

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9d57b29
Make core worker exit with system error when receiving IOError from w…
MengjinYan Nov 7, 2024
2029d36
Update the code to throw system exception upon IOError in free_objects
MengjinYan Nov 8, 2024
89dfa7c
fix lint
MengjinYan Nov 8, 2024
d654c2f
revert comment change
MengjinYan Nov 8, 2024
8148938
fix lint
MengjinYan Nov 9, 2024
eeba114
make time calculation in test_disable_driver_logs_breakpoint more acc…
MengjinYan Nov 12, 2024
01f5f11
Move the error check to core_worker.cc
MengjinYan Nov 12, 2024
f202ac7
fix lint
MengjinYan Nov 13, 2024
07d4cf1
Merge branch 'master' into issue-48628
MengjinYan Nov 13, 2024
76fd34d
Merge branch 'master' into issue-48628
MengjinYan Nov 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions python/ray/_raylet.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3831,8 +3831,11 @@ cdef class CoreWorker:
c_vector[CObjectID] free_ids = ObjectRefsToVector(object_refs)

with nogil:
check_status(CCoreWorkerProcess.GetCoreWorker().Delete(
free_ids, local_only))
status = CCoreWorkerProcess.GetCoreWorker().Delete(free_ids, local_only)
if status.IsIOError():
check_status(CRayStatus.UnexpectedSystemExit(status.ToString()))
Copy link
Contributor

@rynewang rynewang Nov 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of inspecting it here, can we do if is IOError then UnexpectedSystemExit, else as-is in CoreWorker::Delete ?

Copy link
Contributor

@rynewang rynewang Nov 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts: if there's another code path calling Delete and get broken pipe, it should all be considered as UnexpectedSystemExit

Copy link
Contributor Author

@MengjinYan MengjinYan Nov 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Synced offline. It is better to add the logic in the CoreWorker::Delete because it can cover the case for other languages as well. The CoreWorker::Delete function is intended to be open to all languages to call and is not called in other ray internal code paths.

else:
check_status(status)

def get_local_ongoing_lineage_reconstruction_tasks(self):
cdef:
Expand Down
6 changes: 6 additions & 0 deletions src/ray/raylet_client/raylet_client.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,12 @@ Status raylet::RayletClient::Disconnect(
auto status = conn_->WriteMessage(MessageType::DisconnectClient, &fbb);
// Don't be too strict for disconnection errors.
// Just create logs and prevent it from crash.
// TODO (myan): In the current implementation, if raylet is already terminated in the
// "WriteMessage" function above, the worker process will exit early in the function
// and will not reach here. However, the code path here is shared between graceful
// shutdown and force termination. We need to make sure the above early exit
// shouldn't happen during the graceful shutdown scenario and there shouldn't be any
// leak if early exit is triggered
if (!status.ok()) {
RAY_LOG(WARNING)
<< status.ToString()
Expand Down