Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16020 )
Change subject: IMPALA-9318: Add admission control setting to cap MT_DOP ...................................................................... Patch Set 3: Code-Review+1 (3 comments) Had a few very minor things in addition to bikram's comments. http://gerrit.cloudera.org:8080/#/c/16020/3/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/16020/3/be/src/service/impala-server.cc@914 PS3, Line 914: EnforceMaxMtDop > since the config values can change while the query is in admission controll I think we should be clear about the expected behaviour. I think the actual decision we make is probably inconsequential in practice http://gerrit.cloudera.org:8080/#/c/16020/3/common/thrift/ImpalaInternalService.thrift File common/thrift/ImpalaInternalService.thrift: http://gerrit.cloudera.org:8080/#/c/16020/3/common/thrift/ImpalaInternalService.thrift@594 PS3, Line 594: 26: optional i32 overridden_mt_dop_value = -1; Doesn't matter too much, but would probably be more consistent with other options like transaction_id to use the __isset bit to indicate whether it was valid or not. http://gerrit.cloudera.org:8080/#/c/16020/3/testdata/workloads/functional-query/queries/QueryTest/max-mt-dop.test File testdata/workloads/functional-query/queries/QueryTest/max-mt-dop.test: http://gerrit.cloudera.org:8080/#/c/16020/3/testdata/workloads/functional-query/queries/QueryTest/max-mt-dop.test@7 PS3, Line 7: select count(*) from tpch.lineitem; > nit: we can probably just use "select 1" here and below and save a few seco tpch_parquet.lineitem would also be significantly faster, since it turns into just a footer read. -- To view, visit http://gerrit.cloudera.org:8080/16020 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3affb127a5dca517591323f2b1c880aa4b38badd Gerrit-Change-Number: 16020 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Wed, 03 Jun 2020 00:03:40 +0000 Gerrit-HasComments: Yes
