Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5164: Fix flaky benchmarks
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/6935/2/be/src/util/benchmark.cc
File be/src/util/benchmark.cc:

Line 62:       throw std::runtime_error("Benchmark failed to complete due to 
context switching.");
> That is true, but having an error message and call graph doesn't help here.
Sounds like a Status might be a better fit then.


Line 69:   }
> I thought so as well, but there is no reset method currently, so this actua
You could maybe create a new stopwatch in that case?


PS2, Line 94: 10
> Arbitrary flake factor.  If we lose 90% of measurements, we bail.  This par
Can you add a comment along those lines for future reference then?


-- 
To view, visit http://gerrit.cloudera.org:8080/6935
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7e0ddfaa9bd1340990ae2b6ff946d72bbd26c274
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <[email protected]>
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Zach Amsden <[email protected]>
Gerrit-HasComments: Yes

Reply via email to