Dan Hecht has posted comments on this change. Change subject: IMPALA-3201: reservation implementation for new buffer pool ......................................................................
Patch Set 9: (1 comment) http://gerrit.cloudera.org:8080/#/c/3993/8/be/src/bufferpool/reservation-tracker-counters.h File be/src/bufferpool/reservation-tracker-counters.h: PS8, Line 35: > Will change to the subclass. I see. We could hide this detail inside of this class by defining an SetReservation(), SetUsedReservation(), and then it's moved out of the way of the ReservationTracker code which is good since that's where the interesting stuff happens. And it's one less invariant that it has to check. Also, now that we never have NULL counters, why not getting rid of the reservation_, used_reservation_ and reservation_limit_ fields of ReservationTracker() and instead call into this class to get them. That would eliminate another invariant to verify and streamline the amount of state further. What do you think? -- To view, visit http://gerrit.cloudera.org:8080/3993 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I35cc89e863efb4cc506657bfdaaaf633a10bbab6 Gerrit-PatchSet: 9 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
