Matthew Jacobs has posted comments on this change. Change subject: IMPALA-4833: Compute precise per-host reservation size, pt2 ......................................................................
Patch Set 1: > This looks good to me, but let's still discuss. I made some notes from a back-channel communication. Potential improvements/changes: 1) Consider renaming min_reservation_bytes -> initial_reservation_bytes? This isn't obviously the right thing to unless we also change resource_profile_.min_reservation, and I don't think anyone feels strongly about that. 2) look into removing initial_reservation_bytes on the thrift structures. I.e. because it's a basically trivially calculation, compute it in the BE. This is interesting to me because the concept doesn't really make sense outside of the implementation in initial-reservation.cc so it's not ideal to keep in the very visible thrift structures. That said, Tim has pointed out that it does make sense for the FE to compute as much as possible, and for the BE to plumb it through. I'm on the fence, but I'll leave it for now and just try to improve the comments we have. -- To view, visit http://gerrit.cloudera.org:8080/7681 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1391d99ca15d2ebcaabd564fbe1242806be09f72 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-HasComments: No
