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
Expose the flag to disable Ømq copy buffers #595
Comments
Yep. Agreed. Can you please elaborate what is the issue? |
There's nothing new to report: better throughput, in exchange for less predictable memory consumption, is a good trade-off for us, and probably for others, so it should be available in the factory methods. |
While we're talking, though, it seems like the factory methods only work seamlessly if on whatever storage Uber happens to use (guessing HDFS, given |
We're considering dropping down to the |
Your diagnosis is correct. We did not use petastorm with s3 internally, hence this is not a polished feature. I can help if you can point me to the issues you run into. I did verify basic functionality but not more then that. |
I saw that you created #594. If this is not sufficient (curious what else would we need), perhaps we can have |
We built up a bit of a backlog while waiting on legal to approve submitting PRs back, but they're all in there, now, so you see everything that's been needed, so far (except for I think passing the |
#590 was actually the real killer, but that addresses every problem we've seen, so far, except this one, so we could get it all done in 2 PRs, probably. |
For your part, having it be parameterized instead of discovered, makes it so that you can route defects to those project ( |
So if I understand correctly if we land all PRs you have put up we would solve your outstanding problems, correct? |
Yes, but we have a private fork we're working off of, so are not blocked. I'm really interested in the future-proofing idea of adding a If you want to merge these and then discuss a refactoring, great, but I'm assuming you don't want to add parameters to the factory methods and then remove them. |
I would image that |
Makes sense. I'll update that PR with release notes for you to merge. |
One of our engineers found an optimization involving disabling ZeroMQ copy buffers in the
ProcessWorker
, but this is not exposed in the top-level factory methods,make_reader
andmake_batch_reader
. It's useful, and probably should be.The text was updated successfully, but these errors were encountered: