Stop vendoring pyarrow #7233
Stop vendoring pyarrow #7233
Conversation
|
Can one of the admins verify this patch? |
| _exit(kRayletStoreErrorExitCode); | ||
| if (exit_on_error_) { | ||
| // When shutting down a cluster, it's possible that the plasma store is killed | ||
| // earlier than raylet, in this case we don't want raylet to crash, we instead |
There was a problem hiding this comment.
| // earlier than raylet, in this case we don't want raylet to crash, we instead | |
| // earlier than raylet. In this case we don't want raylet to crash, we instead |
| << "Problem communicating with the object store from raylet, check logs or " | ||
| << "dmesg for previous errors: " << boost_to_ray_status(error).ToString(); | ||
| if (exit_on_error_) { | ||
| RAY_LOG(FATAL) |
There was a problem hiding this comment.
Is there a reason we RAY_LOG(FATAL) here, but _exit in the other code block?
There was a problem hiding this comment.
i don't know the reason
| @@ -71,6 +77,8 @@ class ObjectStoreNotificationManager { | |||
| int64_t num_removes_processed_; | |||
| std::vector<uint8_t> notification_; | |||
| local_stream_protocol::socket socket_; | |||
|
|
|||
| bool exit_on_error_; | |||
There was a problem hiding this comment.
Please document (what happens if this is false?).
| // Exit raylet process. | ||
| _exit(kRayletStoreErrorExitCode); | ||
| } else { | ||
| return; |
There was a problem hiding this comment.
Should we log an error message here?
| ObjectStoreNotificationManager(boost::asio::io_service &io_service, | ||
| const std::string &store_socket_name); | ||
| const std::string &store_socket_name, | ||
| bool exit_on_error = true); |
There was a problem hiding this comment.
Not a strong preference, but I think it would be good to make this an explicit argument instead of optional.
There was a problem hiding this comment.
yeah this is a special case I want to keep backward compatibility.
|
Test FAILed. |
|
@stephanie-wang addressed |
|
Test FAILed. |
|
Test memory scheduling seems to be flaky on master |
This will be the final revert.
Check the diff here f5d1571