avantgardnerio commented on PR #156: URL: https://github.com/apache/arrow-ballista/pull/156#issuecomment-1232372088
I'm sorry, I looked this PR over a couple times, and I just don't understand enough about Ballista (yet) to contribute a meaningful review. I suppose I can at least try to explain what I think I understand: 1. it looks like in January we added the ability to push work to executors, rather than having them poll? 2. it looks like the executors know how many CPU resources or memory or whatever, and those are advertised as ReservationOfferings to the scheduler when they connect? 3. and now there's a config setting for whether to push to executors or have them poll, presumably because we will eventually switch over to always push, but we aren't confident enough in that functionality yet? 4. but ebay china has been using the push method on their fork for some time now? 5. because of some simplification in the previous PR, this one was able to eliminate an event loop and associated channel? I think analyzing these PRs is probably hard because there isn't up-to-date documentation, or documentation in the level of depth of channels, what messages go over them, and who sends and receives them. I will try to piece this together from the code, but it will take me a little while to get up to speed as I'm also having to contribute some things myself. Hopefully by at least asking these questions I will learn something. It sounds like @andygrove is willing to merge as long as there is no opposition, and I think that strategy makes sense if this code has been used in production and is just now making its way upstream. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
