Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14104 )
Change subject: IMPALA-8803: Coordinator should release admitted memory per-backend ...................................................................... Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/14104/1/be/src/runtime/coordinator-backend-state.cc File be/src/runtime/coordinator-backend-state.cc: http://gerrit.cloudera.org:8080/#/c/14104/1/be/src/runtime/coordinator-backend-state.cc@846 PS1, Line 846: last_backend || coordinator_last_backend > Fair point. I suppose SetNonErrorTerminalState() may not be the right funct Agree that it might be clearer to add a new state to the state machine to make it more explicit (I'm ok with deferring this though). Most of the state transitions are to terminal states, so it would require some refactoring, since most of the helpers are oriented around terminal states. I think there could be up to three states - when all non-coordinator backends are finished, when all results have been sent to the PRS, and when all rows are fetched from the PRS (at which point the coordinator fragment can be torn down). -- To view, visit http://gerrit.cloudera.org:8080/14104 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88bb11e0ede7574568020e0277dd8ac8d2586dc9 Gerrit-Change-Number: 14104 Gerrit-PatchSet: 1 Gerrit-Owner: Sahil Takiar <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Sahil Takiar <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Fri, 23 Aug 2019 16:07:32 +0000 Gerrit-HasComments: Yes
