Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/8997 )
Change subject: IMPALA-6338: Fix flaky test_profile_fragment_instances ...................................................................... Patch Set 4: (1 comment) > (1 comment) > > The change LGTM. > > The only other thing I want to bring up is that we're preferring a > full profile over a quick teardown of a query. From a correctness > point of view, that's sound. However, that may regress short > queries in terms of total query time taken. But that's probably a > topic for a much larger discussion that can be addressed as part of > IMPALA-2990. > > Since this change changes the lifecycle of a query a bit, it would > be better if someone else had a look too. Agree its important to thing about perf impact here, esp. for small queries. I think ultimately we'll want to always end up constructing the complete runtime profile, since having full and accurate info in the profile is important for analysis, but that we may want to change the behavior where Fetch() blocks until the profile is complete. http://gerrit.cloudera.org:8080/#/c/8997/3/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: http://gerrit.cloudera.org:8080/#/c/8997/3/be/src/runtime/coordinator.cc@462 PS3, Line 462: St > nit: Not your change, but we can get rid of this bad indent. Done -- To view, visit http://gerrit.cloudera.org:8080/8997 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I77773a1e3c62952003f37f88fe2b662bb11889ed Gerrit-Change-Number: 8997 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com> Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-Comment-Date: Wed, 31 Jan 2018 18:37:50 +0000 Gerrit-HasComments: Yes
