Marcel Kornacker has posted comments on this change. Change subject: IMPALA-4890/5143: Coordinator race involving TearDown() ......................................................................
Patch Set 1: (1 comment) > (2 comments) > > Where does ReleaseResources() get called for DML queries? It used > to be in ClientRequestState::Done(), I think, but I don't see where > it gets called now (it must be, because tests are passing). This is now done implicitly as part of Cancel(). UnregisterQuery() calls CRS::Cancel (which in turn calls Coordinator::Cancel) before it calls Done(). http://gerrit.cloudera.org:8080/#/c/6897/1/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: PS1, Line 113: if (query_state_ != nullptr) { : ExecEnv::GetInstance()->query_exec_mgr()->ReleaseQueryState(query_state_); : } > I thought that in general we wanted to move away from work done in d'tors? we want to move away from release resources in d'tors, and from tearing down control structures prematurely (as was done in teardown()). the coordinator now has access to the query state until the coordinator itself goes away. -- To view, visit http://gerrit.cloudera.org:8080/6897 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I457a6424a0255c137336c4bc01a6e7ed830d18c7 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Marcel Kornacker <mar...@cloudera.com> Gerrit-Reviewer: Henry Robinson <he...@cloudera.com> Gerrit-Reviewer: Marcel Kornacker <mar...@cloudera.com> Gerrit-HasComments: Yes
