Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/13307 )
Change subject: WIP IMPALA-8536: Add Scalable Pool Configuration to Admission Controller. ...................................................................... Patch Set 1: (6 comments) Thanks for the early feedback. Patch set 2 has some updates based on feedback but still is not complete. http://gerrit.cloudera.org:8080/#/c/13307/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13307/1//COMMIT_MSG@21 PS1, Line 21: + Max Memory Multiple - the Multiple of the current total number of > I had also thought about specifying memory as a % of the available memory ( I did think about this but I welcome suggestions and/or decisions from more experienced folk. On the one hand, percentage of memory is a natural way to think about splitting up memory resources. On the other hand (1) having the memory be configured as a multiple of nodes is consistent with the way the other two scalable parameters work (2) sysadmins may have to decide whether to use the old max memory or the new parameter, including possibly switching between the two, so having them both use the same units has some advantage. In a management system like CM I would expect to see a choice between these tow parameter, so having the same units for each will be less confusing. One thing I should have made clearer is that if the scalable parameter (e.g. Max Memory Multiple) is set, then any value set for the corresponding unscalable parameter is ignored (e.g. Max Memory) http://gerrit.cloudera.org:8080/#/c/13307/1/be/src/scheduling/admission-controller-test.cc File be/src/scheduling/admission-controller-test.cc: http://gerrit.cloudera.org:8080/#/c/13307/1/be/src/scheduling/admission-controller-test.cc@285 PS1, Line 285: FLAGS_fair_scheduler_allocation_path = GetResourceFile("fair-scheduler-test2.xml"); > Can we use ScopedFlagSetter for setting these flags? Otherwise they could a Thanks! http://gerrit.cloudera.org:8080/#/c/13307/1/be/src/scheduling/admission-controller.cc File be/src/scheduling/admission-controller.cc: http://gerrit.cloudera.org:8080/#/c/13307/1/be/src/scheduling/admission-controller.cc@1401 PS1, Line 1401: return pool_config.max_running_queries_multiple > We should think carefully about rounding here. I think this is implicitly t Agreed, I added a simple unit test and updated the descriptions: + Max Running Queries Multiple - this floating point number is multiplied by the current total number of running Impalads at runtime to give the maximum number of concurrently running queries allowed in the pool. This calculation is rounded up to the nearest integer so the result will always be at least one as long as the parameter is non-zero. + Max Queued Queries Multiple - this floating point number is multiplied by the current total number of running Impalads at runtime to give the maximum number of queries that can be queued in the pool. This calculation is rounded up to the nearest integer so the result will always be at least one as long as the parameter is non-zero. http://gerrit.cloudera.org:8080/#/c/13307/1/common/thrift/ImpalaInternalService.thrift File common/thrift/ImpalaInternalService.thrift: http://gerrit.cloudera.org:8080/#/c/13307/1/common/thrift/ImpalaInternalService.thrift@682 PS1, Line 682: > line has trailing whitespace Done http://gerrit.cloudera.org:8080/#/c/13307/1/common/thrift/ImpalaInternalService.thrift@686 PS1, Line 686: > line has trailing whitespace Done http://gerrit.cloudera.org:8080/#/c/13307/1/fe/src/main/java/org/apache/impala/util/RequestPoolService.java File fe/src/main/java/org/apache/impala/util/RequestPoolService.java: http://gerrit.cloudera.org:8080/#/c/13307/1/fe/src/main/java/org/apache/impala/util/RequestPoolService.java@415 PS1, Line 415: result.setMax_running_queries_multiple(getLlamaPoolConfigValue( > Idle thought - maybe we should clean up the LLAMA references in this file w I cleaned up the file -- To view, visit http://gerrit.cloudera.org:8080/13307 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If47508728124076f3b9200c27cffc989f7a4f188 Gerrit-Change-Number: 13307 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Sherman <asher...@cloudera.com> Gerrit-Reviewer: Andrew Sherman <asher...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Mon, 13 May 2019 18:13:34 +0000 Gerrit-HasComments: Yes
