IMPALA-5487: Fix race in RuntimeProfile::toThrift() node.num_children got initialized before children_ was locked. This could lead to node.num_children < children_.size(), which made the node tree in the resulting thrift profiles not deserialize properly.
To fix this, node.num_children needs to be initialized after children_ has been locked. I tested this by running queries on a private cluster for a while, making sure that the thrift profiles of running queries could be parsed correctly. Change-Id: I7fad4ee2eee1f73e387c1e90a3db265b19b3a0c6 Reviewed-on: http://gerrit.cloudera.org:8080/7154 Reviewed-by: Dan Hecht <[email protected]> Reviewed-by: Tim Armstrong <[email protected]> Reviewed-by: Sailesh Mukil <[email protected]> Tested-by: Impala Public Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/58baca74 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/58baca74 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/58baca74 Branch: refs/heads/master Commit: 58baca7455fb5ff153455fe7b53292ab5e431371 Parents: 9fd46f7 Author: Lars Volker <[email protected]> Authored: Sun Jun 11 22:01:34 2017 -0700 Committer: Impala Public Jenkins <[email protected]> Committed: Tue Jun 13 00:34:21 2017 +0000 ---------------------------------------------------------------------- be/src/util/runtime-profile.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/58baca74/be/src/util/runtime-profile.cc ---------------------------------------------------------------------- diff --git a/be/src/util/runtime-profile.cc b/be/src/util/runtime-profile.cc index 9efe556..5ff9d28 100644 --- a/be/src/util/runtime-profile.cc +++ b/be/src/util/runtime-profile.cc @@ -761,13 +761,10 @@ void RuntimeProfile::ToThrift(TRuntimeProfileTree* tree) const { } void RuntimeProfile::ToThrift(vector<TRuntimeProfileNode>* nodes) const { - nodes->reserve(nodes->size() + children_.size()); - int index = nodes->size(); nodes->push_back(TRuntimeProfileNode()); TRuntimeProfileNode& node = (*nodes)[index]; node.name = name_; - node.num_children = children_.size(); node.metadata = metadata_; node.indent = true; @@ -839,6 +836,7 @@ void RuntimeProfile::ToThrift(vector<TRuntimeProfileNode>* nodes) const { { lock_guard<SpinLock> l(children_lock_); children = children_; + node.num_children = children_.size(); } for (int i = 0; i < children.size(); ++i) { int child_idx = nodes->size();
