Tim Armstrong has posted comments on this change. Change subject: IMPALA-4410: Safer tear-down of RuntimeState ......................................................................
Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/4893/2/be/src/runtime/test-env.cc File be/src/runtime/test-env.cc: Line 109: for (shared_ptr<RuntimeState>& runtime_state : runtime_states_) { This calculation is wrong unless you only created one RuntimeState per query (which was the original intent of the interface). http://gerrit.cloudera.org:8080/#/c/4893/2/be/src/runtime/test-env.h File be/src/runtime/test-env.h: PS2, Line 42: CreateRuntimeState The old name was deliberate, since it's meant to provide a simple interface to set up the minimal set of query-wide objects for a test. I.e. once there is actually a query-wide state, this should create it. The new name seems ok, but there doesn't seem to be much point in changing it if it's going to get changed back after IMPALA-4014 -- 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: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry Robinson <he...@cloudera.com> Gerrit-Reviewer: Henry Robinson <he...@cloudera.com> Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-HasComments: Yes
