impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4410: Safer tear-down of RuntimeState
Date Wed, 09 Nov 2016 01:14:15 GMT
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 <henry@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message