[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11977 ) Change subject: IMPALA-6924: Add child queries to profile in compute stats .. Patch Set 7: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 7 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 05 Dec 2018 00:32:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11977 ) Change subject: IMPALA-6924: Add child queries to profile in compute stats .. IMPALA-6924: Add child queries to profile in compute stats COMPUTE STATS triggers two child queries which do the actual stats calculation. This patch fetches the profiles for the child queries and adds them to the profile for the COMPUTE STATS to make it easier to debug issues with the child queries. To enable this, this patch also adds a 'format' parameter to GetRuntimeProfile(), which allows clients to retrieve the profile as either a pretty printed string (currently the only option), as a base64 encoded string, or as a thrift structure. This allows the child query to add the profile directly as a child of the parent query's profile. Note that the 'format' parameter is only available for the HiveServer2 client and not for Beeswax. This is because Thrift does not appear to have proper support for default parameters to service methods, so adding the 'format' parameter would not be backwards compatible with existing Impala Beeswax clients. This does not affect HiveServer2 as it takes a 'request' struct as its only parameter and Thrift does support default values for struct fields. This patch also fixes a bug where '__isset' was not being set in the Thrift runtime profile for the exec summry. Testing: - Ran COMPUTE STATS and verified that the profile contains the expected output both when the child queries succeed and when they fail. - Added an e2e test that runs a COMPUTE STATS and checks that there are three unique query ids in the profile. - Added a BE test that verifies the archive string (de)serialization functions work. Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Reviewed-on: http://gerrit.cloudera.org:8080/11977 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/service/child-query.cc M be/src/service/child-query.h M be/src/service/client-request-state.cc M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-http-handler.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/util/runtime-profile-test.cc M be/src/util/runtime-profile.cc M be/src/util/runtime-profile.h M common/thrift/ImpalaService.thrift M common/thrift/RuntimeProfile.thrift M tests/query_test/test_observability.py 14 files changed, 178 insertions(+), 20 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 8 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11977 ) Change subject: IMPALA-6924: Add child queries to profile in compute stats .. Patch Set 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3522/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 7 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 04 Dec 2018 20:43:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11977 ) Change subject: IMPALA-6924: Add child queries to profile in compute stats .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 7 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 04 Dec 2018 20:43:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11977 ) Change subject: IMPALA-6924: Add child queries to profile in compute stats .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1533/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 6 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 04 Dec 2018 20:30:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11977 ) Change subject: IMPALA-6924: Add child queries to profile in compute stats .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 6 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 04 Dec 2018 19:57:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/11977 ) Change subject: IMPALA-6924: Add child queries to profile in compute stats .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/11977/5/be/src/service/impala-hs2-server.cc File be/src/service/impala-hs2-server.cc: http://gerrit.cloudera.org:8080/#/c/11977/5/be/src/service/impala-hs2-server.cc@907 PS5, Line 907: DCHECK(request.format == TRuntimeProfileFormat::STRING > line too long (111 > 90) Done -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 6 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 04 Dec 2018 19:55:31 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Hello Andrew Sherman, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11977 to look at the new patch set (#6). Change subject: IMPALA-6924: Add child queries to profile in compute stats .. IMPALA-6924: Add child queries to profile in compute stats COMPUTE STATS triggers two child queries which do the actual stats calculation. This patch fetches the profiles for the child queries and adds them to the profile for the COMPUTE STATS to make it easier to debug issues with the child queries. To enable this, this patch also adds a 'format' parameter to GetRuntimeProfile(), which allows clients to retrieve the profile as either a pretty printed string (currently the only option), as a base64 encoded string, or as a thrift structure. This allows the child query to add the profile directly as a child of the parent query's profile. Note that the 'format' parameter is only available for the HiveServer2 client and not for Beeswax. This is because Thrift does not appear to have proper support for default parameters to service methods, so adding the 'format' parameter would not be backwards compatible with existing Impala Beeswax clients. This does not affect HiveServer2 as it takes a 'request' struct as its only parameter and Thrift does support default values for struct fields. This patch also fixes a bug where '__isset' was not being set in the Thrift runtime profile for the exec summry. Testing: - Ran COMPUTE STATS and verified that the profile contains the expected output both when the child queries succeed and when they fail. - Added an e2e test that runs a COMPUTE STATS and checks that there are three unique query ids in the profile. - Added a BE test that verifies the archive string (de)serialization functions work. Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 --- M be/src/service/child-query.cc M be/src/service/child-query.h M be/src/service/client-request-state.cc M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-http-handler.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/util/runtime-profile-test.cc M be/src/util/runtime-profile.cc M be/src/util/runtime-profile.h M common/thrift/ImpalaService.thrift M common/thrift/RuntimeProfile.thrift M tests/query_test/test_observability.py 14 files changed, 178 insertions(+), 20 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/11977/6 -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 6 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11977 ) Change subject: IMPALA-6924: Add child queries to profile in compute stats .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1532/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 5 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 04 Dec 2018 19:36:09 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11977 ) Change subject: IMPALA-6924: Add child queries to profile in compute stats .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/11977/5/be/src/service/impala-hs2-server.cc File be/src/service/impala-hs2-server.cc: http://gerrit.cloudera.org:8080/#/c/11977/5/be/src/service/impala-hs2-server.cc@907 PS5, Line 907: DCHECK(request.format == TRuntimeProfileFormat::STRING || request.format == TRuntimeProfileFormat::BASE64); line too long (111 > 90) -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 5 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 04 Dec 2018 19:04:42 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Hello Andrew Sherman, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11977 to look at the new patch set (#5). Change subject: IMPALA-6924: Add child queries to profile in compute stats .. IMPALA-6924: Add child queries to profile in compute stats COMPUTE STATS triggers two child queries which do the actual stats calculation. This patch fetches the profiles for the child queries and adds them to the profile for the COMPUTE STATS to make it easier to debug issues with the child queries. To enable this, this patch also adds a 'format' parameter to GetRuntimeProfile(), which allows clients to retrieve the profile as either a pretty printed string (currently the only option), as a base64 encoded string, or as a thrift structure. This allows the child query to add the profile directly as a child of the parent query's profile. Note that the 'format' parameter is only available for the HiveServer2 client and not for Beeswax. This is because Thrift does not appear to have proper support for default parameters to service methods, so adding the 'format' parameter would not be backwards compatible with existing Impala Beeswax clients. This does not affect HiveServer2 as it takes a 'request' struct as its only parameter and Thrift does support default values for struct fields. This patch also fixes a bug where '__isset' was not being set in the Thrift runtime profile for the exec summry. Testing: - Ran COMPUTE STATS and verified that the profile contains the expected output both when the child queries succeed and when they fail. - Added an e2e test that runs a COMPUTE STATS and checks that there are three unique query ids in the profile. - Added a BE test that verifies the archive string (de)serialization functions work. Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 --- M be/src/service/child-query.cc M be/src/service/child-query.h M be/src/service/client-request-state.cc M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-http-handler.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/util/runtime-profile-test.cc M be/src/util/runtime-profile.cc M be/src/util/runtime-profile.h M common/thrift/ImpalaService.thrift M common/thrift/RuntimeProfile.thrift M tests/query_test/test_observability.py 14 files changed, 177 insertions(+), 20 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/11977/5 -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 5 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/11977 ) Change subject: IMPALA-6924: Add child queries to profile in compute stats .. Patch Set 5: (5 comments) http://gerrit.cloudera.org:8080/#/c/11977/4/be/src/service/client-request-state.cc File be/src/service/client-request-state.cc: http://gerrit.cloudera.org:8080/#/c/11977/4/be/src/service/client-request-state.cc@572 PS4, Line 572: child_queries.emplace_back(compute_stats_params.tbl_stats_query, this, > I know you didn't write this line, but I see some people advocating that em Obviously not a big deal since this isn't a super perf critical piece of code, but you're right so I went ahead and made the change. http://gerrit.cloudera.org:8080/#/c/11977/4/be/src/service/impala-hs2-server.cc File be/src/service/impala-hs2-server.cc: http://gerrit.cloudera.org:8080/#/c/11977/4/be/src/service/impala-hs2-server.cc@907 PS4, Line 907: DCHECK(request.format == TRuntimeProfileFormat::STRING || request.format == TRuntimeProfileFormat::BASE64); > So this covers both the string and base64 cases? Right, in the base64 case we're still just returning a string, so I'm using the same out parameter to GetRuntimeProfileOutput() 'ss' for both cases. Obviously a bit messy/confusing but I didn't see a better way to rewrite GetRuntimeProfileOutput(). I added a DCHECK to make it clearer. http://gerrit.cloudera.org:8080/#/c/11977/4/be/src/util/runtime-profile-test.cc File be/src/util/runtime-profile-test.cc: http://gerrit.cloudera.org:8080/#/c/11977/4/be/src/util/runtime-profile-test.cc@93 PS4, Line 93: EXPECT_TRUE(deserialized_profile->GetCounter("Not there") == nullptr); > == nullptr Done http://gerrit.cloudera.org:8080/#/c/11977/4/be/src/util/runtime-profile.cc File be/src/util/runtime-profile.cc: http://gerrit.cloudera.org:8080/#/c/11977/4/be/src/util/runtime-profile.cc@888 PS4, Line 888: uint32_t deserialized_len = static_cast(result_len); > Would there be any advantage to using a cast here? My understanding is that it doesn't make a difference to how things run, but it does make the operation more explicit, which is helpful, so I went ahead and did it. http://gerrit.cloudera.org:8080/#/c/11977/4/tests/query_test/test_observability.py File tests/query_test/test_observability.py: http://gerrit.cloudera.org:8080/#/c/11977/4/tests/query_test/test_observability.py@265 PS4, Line 265: # Search for all query ids (max length 33) in the profile. > So 33 is the length of a query id? That made me think. Added a comment -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 5 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 04 Dec 2018 19:04:12 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/11977 ) Change subject: IMPALA-6924: Add child queries to profile in compute stats .. Patch Set 4: Code-Review+1 (5 comments) This all looks good to me. I have comments that are more like questions so I can learn things. http://gerrit.cloudera.org:8080/#/c/11977/4/be/src/service/client-request-state.cc File be/src/service/client-request-state.cc: http://gerrit.cloudera.org:8080/#/c/11977/4/be/src/service/client-request-state.cc@572 PS4, Line 572: child_queries.push_back(ChildQuery(compute_stats_params.tbl_stats_query, this, I know you didn't write this line, but I see some people advocating that emplace_back is better than push_back as it can avoid a copy? http://gerrit.cloudera.org:8080/#/c/11977/4/be/src/service/impala-hs2-server.cc File be/src/service/impala-hs2-server.cc: http://gerrit.cloudera.org:8080/#/c/11977/4/be/src/service/impala-hs2-server.cc@907 PS4, Line 907: return_val.__set_profile(ss.str()); So this covers both the string and base64 cases? http://gerrit.cloudera.org:8080/#/c/11977/4/be/src/util/runtime-profile-test.cc File be/src/util/runtime-profile-test.cc: http://gerrit.cloudera.org:8080/#/c/11977/4/be/src/util/runtime-profile-test.cc@93 PS4, Line 93: EXPECT_TRUE(deserialized_profile->GetCounter("Not there") == NULL); == nullptr http://gerrit.cloudera.org:8080/#/c/11977/4/be/src/util/runtime-profile.cc File be/src/util/runtime-profile.cc: http://gerrit.cloudera.org:8080/#/c/11977/4/be/src/util/runtime-profile.cc@888 PS4, Line 888: uint32_t deserialized_len = result_len; Would there be any advantage to using a cast here? http://gerrit.cloudera.org:8080/#/c/11977/4/tests/query_test/test_observability.py File tests/query_test/test_observability.py: http://gerrit.cloudera.org:8080/#/c/11977/4/tests/query_test/test_observability.py@265 PS4, Line 265: matches = re.findall("Query \(id=.{,33}\)", results.runtime_profile) So 33 is the length of a query id? That made me think. -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 04 Dec 2018 17:41:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11977 ) Change subject: IMPALA-6924: Add child queries to profile in compute stats .. Patch Set 4: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 04 Dec 2018 03:57:57 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11977 ) Change subject: IMPALA-6924: Add child queries to profile in compute stats .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1490/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Sat, 01 Dec 2018 00:06:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/11977 ) Change subject: IMPALA-6924: Add child queries to profile in compute stats .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/11977/3/common/thrift/ImpalaService.thrift File common/thrift/ImpalaService.thrift: http://gerrit.cloudera.org:8080/#/c/11977/3/common/thrift/ImpalaService.thrift@443 PS3, Line 443: 3: optional RuntimeProfile.TRuntimeProfileFormat format = > line too long (103 > 90) Done -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 30 Nov 2018 23:28:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Hello Andrew Sherman, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11977 to look at the new patch set (#4). Change subject: IMPALA-6924: Add child queries to profile in compute stats .. IMPALA-6924: Add child queries to profile in compute stats COMPUTE STATS triggers two child queries which do the actual stats calculation. This patch fetches the profiles for the child queries and adds them to the profile for the COMPUTE STATS to make it easier to debug issues with the child queries. To enable this, this patch also adds a 'format' parameter to GetRuntimeProfile(), which allows clients to retrieve the profile as either a pretty printed string (currently the only option), as a base64 encoded string, or as a thrift structure. This allows the child query to add the profile directly as a child of the parent query's profile. Note that the 'format' parameter is only available for the HiveServer2 client and not for Beeswax. This is because Thrift does not appear to have proper support for default parameters to service methods, so adding the 'format' parameter would not be backwards compatible with existing Impala Beeswax clients. This does not affect HiveServer2 as it takes a 'request' struct as its only parameter and Thrift does support default values for struct fields. This patch also fixes a bug where '__isset' was not being set in the Thrift runtime profile for the exec summry. Testing: - Ran COMPUTE STATS and verified that the profile contains the expected output both when the child queries succeed and when they fail. - Added an e2e test that runs a COMPUTE STATS and checks that there are three unique query ids in the profile. - Added a BE test that verifies the archive string (de)serialization functions work. Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 --- M be/src/service/child-query.cc M be/src/service/child-query.h M be/src/service/client-request-state.cc M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-http-handler.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/util/runtime-profile-test.cc M be/src/util/runtime-profile.cc M be/src/util/runtime-profile.h M common/thrift/ImpalaService.thrift M common/thrift/RuntimeProfile.thrift M tests/query_test/test_observability.py 14 files changed, 175 insertions(+), 20 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/11977/4 -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11977 ) Change subject: IMPALA-6924: Add child queries to profile in compute stats .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1485/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 30 Nov 2018 23:25:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/11977 ) Change subject: IMPALA-6924: Add child queries to profile in compute stats .. Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/11977/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11977/2//COMMIT_MSG@9 PS2, Line 9: COMPUTE STATS triggers two child queries which do the actual stats > Should this be "triggers some child queries"? Done http://gerrit.cloudera.org:8080/#/c/11977/2/be/src/service/child-query.h File be/src/service/child-query.h: http://gerrit.cloudera.org:8080/#/c/11977/2/be/src/service/child-query.h@129 PS2, Line 129: ImpalaServer* parent_server_; > Do you want a /// comment here? Done http://gerrit.cloudera.org:8080/#/c/11977/2/be/src/service/child-query.cc File be/src/service/child-query.cc: http://gerrit.cloudera.org:8080/#/c/11977/2/be/src/service/child-query.cc@104 PS2, Line 104: is_running_ = false; > You're right. I think if we wanted to go that way it might be not that bad I think having the mode flag to GetRuntimeProfile is a nice addition, so I went ahead and did it. -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 30 Nov 2018 22:59:48 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11977 ) Change subject: IMPALA-6924: Add child queries to profile in compute stats .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/11977/3/common/thrift/ImpalaService.thrift File common/thrift/ImpalaService.thrift: http://gerrit.cloudera.org:8080/#/c/11977/3/common/thrift/ImpalaService.thrift@443 PS3, Line 443: 3: optional RuntimeProfile.TRuntimeProfileFormat format = RuntimeProfile.TRuntimeProfileFormat.STRING line too long (103 > 90) -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 30 Nov 2018 23:00:32 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Hello Andrew Sherman, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11977 to look at the new patch set (#3). Change subject: IMPALA-6924: Add child queries to profile in compute stats .. IMPALA-6924: Add child queries to profile in compute stats COMPUTE STATS triggers two child queries which do the actual stats calculation. This patch fetches the profiles for the child queries and adds them to the profile for the COMPUTE STATS to make it easier to debug issues with the child queries. To enable this, this patch also adds a 'format' parameter to GetRuntimeProfile(), which allows clients to retrieve the profile as either a pretty printed string (currently the only option), as a base64 encoded string, or as a thrift structure. This allows the child query to add the profile directly as a child of the parent query's profile. Note that the 'format' parameter is only available for the HiveServer2 client and not for Beeswax. This is because Thrift does not appear to have proper support for default parameters to service methods, so adding the 'format' parameter would not be backwards compatible with existing Impala Beeswax clients. This does not affect HiveServer2 as it takes a 'request' struct as its only parameter and Thrift does support default values for struct fields. This patch also fixes a bug where '__isset' was not being set in the Thrift runtime profile for the exec summry. Testing: - Ran COMPUTE STATS and verified that the profile contains the expected output both when the child queries succeed and when they fail. - Added an e2e test that runs a COMPUTE STATS and checks that there are three unique query ids in the profile. - Added a BE test that verifies the archive string (de)serialization functions work. Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 --- M be/src/service/child-query.cc M be/src/service/child-query.h M be/src/service/client-request-state.cc M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-http-handler.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/util/runtime-profile-test.cc M be/src/util/runtime-profile.cc M be/src/util/runtime-profile.h M common/thrift/ImpalaService.thrift M common/thrift/RuntimeProfile.thrift M tests/query_test/test_observability.py 14 files changed, 174 insertions(+), 20 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/11977/3 -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11977 ) Change subject: IMPALA-6924: Add child queries to profile in compute stats .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/11977/2/be/src/service/child-query.cc File be/src/service/child-query.cc: http://gerrit.cloudera.org:8080/#/c/11977/2/be/src/service/child-query.cc@104 PS2, Line 104: profile_->AddInfoString("Profile", get_profile_resp.profile); > Do we expose any way to get the actual Thrift profile with a client? I thou You're right. I think if we wanted to go that way it might be not that bad to plumb through - we could add an optional mode flag in TRuntimeProfileRequest and an optional TRuntimeProfileTree to TRuntimeProfileResponse. Or we could add a new method to ImpalaServer directly. Anyway, that's only relevant if we want to go down that path. I don't want the perfect to be the enemy of the useful here. -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 29 Nov 2018 01:38:22 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/11977 ) Change subject: IMPALA-6924: Add child queries to profile in compute stats .. Patch Set 2: (2 comments) Looks like a useful change http://gerrit.cloudera.org:8080/#/c/11977/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11977/2//COMMIT_MSG@9 PS2, Line 9: COMPUTE STATS triggers to child queries which do the actual stats Should this be "triggers some child queries"? Or "triggers two child queries"? http://gerrit.cloudera.org:8080/#/c/11977/2/be/src/service/child-query.h File be/src/service/child-query.h: http://gerrit.cloudera.org:8080/#/c/11977/2/be/src/service/child-query.h@129 PS2, Line 129: RuntimeProfile* profile_; Do you want a /// comment here? -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 29 Nov 2018 01:07:12 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/11977 ) Change subject: IMPALA-6924: Add child queries to profile in compute stats .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/11977/2/be/src/service/child-query.cc File be/src/service/child-query.cc: http://gerrit.cloudera.org:8080/#/c/11977/2/be/src/service/child-query.cc@104 PS2, Line 104: profile_->AddInfoString("Profile", get_profile_resp.profile); > Did you think about splicing the profiles together with AddChild(). It feel Do we expose any way to get the actual Thrift profile with a client? I thought we only exposed it as a string, eg. that's what GetRuntimeProfile() returns -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 29 Nov 2018 00:27:44 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11977 ) Change subject: IMPALA-6924: Add child queries to profile in compute stats .. Patch Set 2: (1 comment) This will be very useful. I had one high level question about the approach. Otherwise the code looked good. http://gerrit.cloudera.org:8080/#/c/11977/2/be/src/service/child-query.cc File be/src/service/child-query.cc: http://gerrit.cloudera.org:8080/#/c/11977/2/be/src/service/child-query.cc@104 PS2, Line 104: profile_->AddInfoString("Profile", get_profile_resp.profile); Did you think about splicing the profiles together with AddChild(). It feels a bit wrong putting the text version of the profile in the machine-readable profile since it mixes the two formats and prevents tools from working properly. Or is the idea that a profile analysis tool should be able to link up the queries itself if needed based on the query ids? And this is just for human convenience? If we're going down that path maybe it would make sense to add machine-readable metadata to the thrift struct with the child query ids - it would be good to have a clear story about how a tool should use this. -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 28 Nov 2018 00:34:31 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11977 ) Change subject: IMPALA-6924: Add child queries to profile in compute stats .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1427/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Wed, 21 Nov 2018 23:55:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11977 ) Change subject: IMPALA-6924: Add child queries to profile in compute stats .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1426/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Wed, 21 Nov 2018 23:44:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/11977 ) Change subject: IMPALA-6924: Add child queries to profile in compute stats .. Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/11977/1/be/src/service/client-request-state.cc File be/src/service/client-request-state.cc: http://gerrit.cloudera.org:8080/#/c/11977/1/be/src/service/client-request-state.cc@563 PS1, Line 563: RuntimeProfile* child_profile = > line too long (92 > 90) Done http://gerrit.cloudera.org:8080/#/c/11977/1/be/src/service/client-request-state.cc@568 PS1, Line 568: if (compute_stats_params.__isset.tbl_stats_query) { > line too long (92 > 90) Done http://gerrit.cloudera.org:8080/#/c/11977/1/be/src/service/client-request-state.cc@571 PS1, Line 571: child_profile->AddChild(profile); > line too long (91 > 90) Done http://gerrit.cloudera.org:8080/#/c/11977/1/be/src/service/client-request-state.cc@574 PS1, Line 574: } > line too long (93 > 90) Done http://gerrit.cloudera.org:8080/#/c/11977/1/be/src/service/client-request-state.cc@577 PS1, Line 577: RuntimeProfile::Create(_pool_, "Column Stats Query"); > line too long (91 > 90) Done -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Wed, 21 Nov 2018 23:16:40 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Hello Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11977 to look at the new patch set (#2). Change subject: IMPALA-6924: Add child queries to profile in compute stats .. IMPALA-6924: Add child queries to profile in compute stats COMPUTE STATS triggers to child queries which do the actual stats calculation. This patch fetches the profiles for the child queries and adds them to the profile for the COMPUTE STATS to make it easier to debug issues with the child queries. Testing: - Ran COMPUTE STATS and verified that the profile contains the expected output both when the child queries succeed and when they fail. - Added a test that runs a COMPUTE STATS and checks that there are three unique query ids in the profile. Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 --- M be/src/service/child-query.cc M be/src/service/child-query.h M be/src/service/client-request-state.cc M tests/query_test/test_observability.py 4 files changed, 51 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/11977/2 -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11977 ) Change subject: IMPALA-6924: Add child queries to profile in compute stats .. Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/11977/1/be/src/service/client-request-state.cc File be/src/service/client-request-state.cc: http://gerrit.cloudera.org:8080/#/c/11977/1/be/src/service/client-request-state.cc@563 PS1, Line 563: RuntimeProfile* child_profile = RuntimeProfile::Create(_pool_, "Child Queries"); line too long (92 > 90) http://gerrit.cloudera.org:8080/#/c/11977/1/be/src/service/client-request-state.cc@568 PS1, Line 568: RuntimeProfile* profile = RuntimeProfile::Create(_pool_, "Table Stats Query"); line too long (92 > 90) http://gerrit.cloudera.org:8080/#/c/11977/1/be/src/service/client-request-state.cc@571 PS1, Line 571: ChildQuery(compute_stats_params.tbl_stats_query, this, parent_server_, profile)); line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/11977/1/be/src/service/client-request-state.cc@574 PS1, Line 574: RuntimeProfile* profile = RuntimeProfile::Create(_pool_, "Column Stats Query"); line too long (93 > 90) http://gerrit.cloudera.org:8080/#/c/11977/1/be/src/service/client-request-state.cc@577 PS1, Line 577: ChildQuery(compute_stats_params.col_stats_query, this, parent_server_, profile)); line too long (91 > 90) -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 21 Nov 2018 23:14:55 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6924: Add child queries to profile in compute stats
Thomas Marshall has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11977 Change subject: IMPALA-6924: Add child queries to profile in compute stats .. IMPALA-6924: Add child queries to profile in compute stats COMPUTE STATS triggers to child queries which do the actual stats calculation. This patch fetches the profiles for the child queries and adds them to the profile for the COMPUTE STATS to make it easier to debug issues with the child queries. Testing: - Ran COMPUTE STATS and verified that the profile contains the expected output both when the child queries succeed and when they fail. - Added a test that runs a COMPUTE STATS and checks that there are three unique query ids in the profile. Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 --- M be/src/service/child-query.cc M be/src/service/child-query.h M be/src/service/client-request-state.cc M tests/query_test/test_observability.py 4 files changed, 46 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/11977/1 -- To view, visit http://gerrit.cloudera.org:8080/11977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I5006c3b366d381eed4687e550cdfc463be3d1350 Gerrit-Change-Number: 11977 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Marshall