-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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.
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
Inline objects #3756
Inline objects #3756
Conversation
Test PASSed. |
Test PASSed. |
Test PASSed. |
Test PASSed. |
Test PASSed. |
Test PASSed. |
Travis is failing with
Can you reproduce the failing of object_manager_test locally? |
maybe valgrind gives us some hint on what's going wrong here: https://travis-ci.com/ray-project/ray/jobs/170048915 |
also some minor linting: https://travis-ci.com/ray-project/ray/jobs/170048914 |
io_service_.post([callback, object_id, locations, has_been_created]() { | ||
callback(object_id, locations, has_been_created); | ||
bool inline_object_flag = it->second.inline_object_flag; | ||
std::vector<uint8_t> inline_object_data = it->second.inline_object_data; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will do a copy, it might be better to do const auto& inline_object_data = ...
Test PASSed. |
Test FAILed. |
Test PASSed. |
Test PASSed. |
Test PASSed. |
Test PASSed. |
Test PASSed. |
test/runtest.py
Outdated
def get(self): | ||
return | ||
|
||
def flush(actor): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The release delay is gone as of apache/arrow#3124, so we can get rid of this, yay!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah great, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm actually, if I remove the flush
call, this test sometimes fails on my laptop. Do you know why that is (not sure about the semantics of ray.internal.free
for plasma)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably a race condition (ray.internal.free is using the plasma client in the raylet which is different from the one in the worker that get is called on).
One way around that is to use ray.workers.global_worker.plasma_client.delete, which uses the same plasma client that will be used for the get, so that should work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm this still seems to fail occasionally with plasma_client.delete
(maybe 1/100 times). Any idea why or do you think it's a bug somewhere else?
if (!client_ids.empty()) { | ||
const std::unordered_set<ClientID> &client_ids, | ||
bool inline_object_flag, | ||
const std::vector<uint8_t> inline_object_data, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not const std::vector<uint8_t>& inline_object_data here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, thank you!
// Inline object is not in the local object store. Create it from | ||
// inline_object_data, and inline_object_metadata, respectively. | ||
// | ||
// Since this function is called on notification or when reading the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call!
Test PASSed. |
Test PASSed. |
Test PASSed. |
Test FAILed. |
This reverts commit f987572.
What do these changes do?
This pull request adds object data to the object entry in GCS, for small objects. This helps with performance optimization, and fault tolerance, as there is no need to reconstruct an object as long as its entry is in GCS.