Sailesh Mukil has posted comments on this change. Change subject: IMPALA-4410: Safer tear-down of RuntimeState ......................................................................
Patch Set 1: Code-Review+1 (2 comments) http://gerrit.cloudera.org:8080/#/c/4893/1/be/src/runtime/runtime-state.cc File be/src/runtime/runtime-state.cc: PS1, Line 300: RuntimeState::Close() Nothing very pressing but wanted to bring this up: Having this generic Close() call for the RuntimeState means one would expect it to be called at the end of every RuntimeState's lifetime. However, we have this RuntimeState in fe-support that looks like a straggler now, since we don't call Close() on it (we don't need to as well, because none of the things being closed here are even set up for that object). Line 304: ExecEnv::GetInstance()->thread_mgr()->UnregisterPool(resource_pool_); Any reason for changing the order in which you are closing now? (UnregisterPool() was after UnregisterReaderContexts() before this patch). If yes, might be good to briefly comment why. -- To view, visit http://gerrit.cloudera.org:8080/4893 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie416e4d57240142bf685385299b749c3a6792c45 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry Robinson <he...@cloudera.com> Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com> Gerrit-HasComments: Yes
