Add catalyst executor server side to catalyst - #3088
Conversation
| std::fprintf(stderr, "[%s] executor ready, waiting for next connection\n", | ||
| Label.c_str()); |
There was a problem hiding this comment.
it exits immediately, and doesn't wait for anything?
There was a problem hiding this comment.
Yes, it's the thing that listed in TODO:
// TODO: This is a temporary solution to initialize the catalyst CTX.
// Done per-connection so each circuit owns its own context.
So what the issue on that executor: exit(0) skips destructors and finalize, the child never explicitly tears down CTX or device state for now. Memory and FDs are reclaimed by the OS, state may linger until the driver notices the process died, so the next connection can occasionally need a reset (It's fine I think if we now what we are doing)
I think it's low priority for now, we can revise it later, it need more careful lifetime control over the system.
| int32_t _catalyst_remote_store_asset(std::vector<char> bytes, std::string name) | ||
| { | ||
| namespace fs = std::filesystem; | ||
| fs::path dst = fs::temp_directory_path() / "catalyst-assets" / fs::path(name).filename(); |
There was a problem hiding this comment.
should we put it in a subdir with PID so multiple writes won't overwrite/collide?
| std::strerror(errno)); | ||
| return -1; | ||
| } | ||
| size_t wrote = std::fwrite(bytes.data(), 1, bytes.size(), f); |
There was a problem hiding this comment.
JB: Partial-write file left on disk. On a short fwrite, the function returns -1 but leaves the truncated file in place and does not check fclose for flush errors. A later load_asset could pick up a corrupt file. Remove the file on failure and check fclose.
josephleekl
left a comment
josephleekl
left a comment
There was a problem hiding this comment.
Thanks @mehrdad2m . Do you think we should add tests for this? up to you
multiphaseCFD
left a comment
multiphaseCFD
left a comment
There was a problem hiding this comment.
LGTM! Thanks @mehrdad2m
Co-authored-by: Joseph Lee <40768758+josephleekl@users.noreply.github.com>
4 participants
Context:
Catalyst already has the host side of the remote-executor connection which is
rt_executorthat exposes the C ABI a compiled program calls, andcatalyst_executor_sessionwhich is the ORC client that holds the connection.Description of the Change:
This PR adds the node end the ORC server that receives objects, maps them and calls them.
Benefits:
Possible Drawbacks:
Related GitHub Issues: