[Impala-ASF-CR] IMPALA-4610: Remove Llama support.
Matthew Jacobs has posted comments on this change. Change subject: IMPALA-4610: Remove Llama support. .. Patch Set 2: (13 comments) http://gerrit.cloudera.org:8080/#/c/4445/1//COMMIT_MSG Commit Message: Line 7: IMPALA-4610: Remove Llama support. please add a JIRA PS1, Line 31: * Remove RM flags (--enable_rm etc.) : * Remove RM query options : * Changes to RequestPoolService. reference https://issues.cloudera.org/browse/IMPALA-4159 http://gerrit.cloudera.org:8080/#/c/4445/1/be/src/runtime/mem-tracker.cc File be/src/runtime/mem-tracker.cc: PS1, Line 69: RuntimeProfile* profile, int64_t byte_limit, : const std::string& label, MemTracker* parent) 1 line? PS1, Line 87: MemTracker::MemTracker(UIntGauge* consumption_metric, : int64_t byte_limit, const string& label) 1 line? http://gerrit.cloudera.org:8080/#/c/4445/1/be/src/runtime/mem-tracker.h File be/src/runtime/mem-tracker.h: PS1, Line 174: // TODO: This may not be right if more than one tracker can actually change its : // RM reservation limit. remove PS1, Line 179: // TODO: this doesn't roll it back completely since the max values for : // the updated trackers aren't decremented. The max values are only used : // for error reporting so this is probably okay. Rolling those back is : // pretty hard; we'd need something like 2PC. I think this was talking about the RM case so this can be removed. PS1, Line 190: new limit=" << limit << " prev=" << limit; new/prev isn't interesting anymore, this is enough: << " limit=" << limit; http://gerrit.cloudera.org:8080/#/c/4445/1/be/src/scheduling/query-schedule.cc File be/src/scheduling/query-schedule.cc: PS1, Line 112: per_host_mem = query_options_.rm_initial_mem; I'd be fine with taking out this case and removing the query option- I don't think we ever tell any admission control users to use this. That said, if it's not getting in the way let's leave it as deprecated until 3.0 so it doesn't break anyone that happens to be using it for now. PS1, Line 120: FLAGS_rm_default_memory same as above. PS1, Line 125: // TODO: Get this limit from Llama (Yarn sets it). remove http://gerrit.cloudera.org:8080/#/c/4445/2/be/src/scheduling/query-schedule.h File be/src/scheduling/query-schedule.h: PS2, Line 88: void GetResourceHostport(const TNetworkAddress& src, TNetworkAddress* dst); I think this can be removed http://gerrit.cloudera.org:8080/#/c/4445/2/fe/src/main/java/com/cloudera/impala/planner/Planner.java File fe/src/main/java/com/cloudera/impala/planner/Planner.java: PS2, Line 295: TODO: Revisit and possibly remove during MT work. We can remove the VCores stuff now, though some users may be relying on the mem estimates (despite our guidance & efforts). We should try to get rid of it for 3.0 though. http://gerrit.cloudera.org:8080/#/c/4445/2/testdata/cluster/node_templates/cdh5/etc/hadoop/conf/yarn-site.xml.tmpl File testdata/cluster/node_templates/cdh5/etc/hadoop/conf/yarn-site.xml.tmpl: PS2, Line 35: : yarn.scheduler.fair.continuous-scheduling-enabled : true : : : : yarn.scheduler.fair.assignmultiple : true : I think a lot of this is stuff that was needed for Llama. Can you leave a TODO to simplify this file? -- To view, visit http://gerrit.cloudera.org:8080/4445 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icfb14209e31f6608bb7b8a33789e00411a6447ef Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry RobinsonGerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Taras Bobrovytsky Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4610: Remove Llama support.
Henry Robinson has posted comments on this change. Change subject: IMPALA-4610: Remove Llama support. .. Patch Set 1: (4 comments) Matt's going to weigh in on what parts of the memory estimation path we can remove, depending on what admission control depends on. http://gerrit.cloudera.org:8080/#/c/4445/1/be/src/scheduling/query-schedule.h File be/src/scheduling/query-schedule.h: Line 86: int16_t GetPerHostVCores() const; > what about this? This is an unused method declaration. As discussed, we still use VCores to generate estimates in the plan / profile, and since those are user-facing we should phase them out on a different schedule. Left a TODO. http://gerrit.cloudera.org:8080/#/c/4445/1/be/src/scheduling/request-pool-service.cc File be/src/scheduling/request-pool-service.cc: Line 47: DEFINE_string(llama_site_path, "", "Path to the Llama configuration file " > good to know. please leave todo with explanatory comment. Done http://gerrit.cloudera.org:8080/#/c/4445/1/be/src/scheduling/simple-scheduler.cc File be/src/scheduling/simple-scheduler.cc: Line 292 > what's going on here? The 'offline' mode was used only when the local NodeManager went offline. Since that can't happen, this is an unused and untested code path. We'll likely want to redo this anyhow when we implement decommissioning. http://gerrit.cloudera.org:8080/#/c/4445/1/be/src/service/impala-server.h File be/src/service/impala-server.h: Line 243 > unclear how this is related to llama See other comment wrt when this was used. -- To view, visit http://gerrit.cloudera.org:8080/4445 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icfb14209e31f6608bb7b8a33789e00411a6447ef Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry RobinsonGerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Matthew Jacobs Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4610: Remove Llama support.
Henry Robinson has uploaded a new patch set (#2). Change subject: IMPALA-4610: Remove Llama support. .. IMPALA-4610: Remove Llama support. Alas, poor Llama! I knew him, Impala: a system of infinite jest, of most excellent fancy: we hath borne him on our back a thousand times; and now, how abhorred in my imagination it is! Done: * Removed QueryResourceMgr, ResourceBroker, CGroupsMgr * Removed untested 'offline' mode and NM failure detection from ImpalaServer * Removed all Llama-related Thrift files * Removed RM-related arguments to MemTracker constructors * Deprecated all RM-related flags, printing a warning if enable_rm is set * Removed expansion logic from MemTracker * Removed VCore logic from QuerySchedule * Removed all reservation-related logic from Scheduler * Removed RM metric descriptions * Various misc. small class changes Not done: * Remove RM flags (--enable_rm etc.) * Remove RM query options * Changes to RequestPoolService. * Remove estimates of VCores / memory from plan Change-Id: Icfb14209e31f6608bb7b8a33789e00411a6447ef --- M be/CMakeLists.txt M be/generated-sources/gen-cpp/CMakeLists.txt M be/src/bufferpool/reservation-tracker-test.cc M be/src/exec/blocking-join-node.cc M be/src/exec/data-sink.cc M be/src/exec/exec-node.cc M be/src/exec/hash-join-node.cc M be/src/exec/hdfs-scan-node-base.cc M be/src/exec/hdfs-scan-node.cc M be/src/exec/kudu-scan-node-test.cc M be/src/exec/kudu-scan-node.cc M be/src/exec/kudu-table-sink-test.cc M be/src/exprs/expr-test.cc D be/src/resourcebroker/CMakeLists.txt D be/src/resourcebroker/resource-broker.cc D be/src/resourcebroker/resource-broker.h M be/src/runtime/buffered-block-mgr-test.cc M be/src/runtime/buffered-block-mgr.cc M be/src/runtime/collection-value-builder-test.cc M be/src/runtime/coordinator.cc M be/src/runtime/data-stream-recvr.cc M be/src/runtime/data-stream-test.cc M be/src/runtime/disk-io-mgr-test.cc M be/src/runtime/disk-io-mgr.cc M be/src/runtime/exec-env.cc M be/src/runtime/exec-env.h M be/src/runtime/mem-pool-test.cc M be/src/runtime/mem-tracker-test.cc M be/src/runtime/mem-tracker.cc M be/src/runtime/mem-tracker.h M be/src/runtime/plan-fragment-executor.cc M be/src/runtime/runtime-filter-bank.cc M be/src/runtime/runtime-state.cc M be/src/runtime/runtime-state.h M be/src/runtime/test-env.cc M be/src/scheduling/CMakeLists.txt D be/src/scheduling/query-resource-mgr.cc D be/src/scheduling/query-resource-mgr.h M be/src/scheduling/query-schedule.cc M be/src/scheduling/query-schedule.h M be/src/scheduling/request-pool-service.cc M be/src/scheduling/scheduler.h M be/src/scheduling/simple-scheduler-test.cc M be/src/scheduling/simple-scheduler.cc M be/src/scheduling/simple-scheduler.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/service/impalad-main.cc M be/src/service/query-exec-state.cc M be/src/util/CMakeLists.txt D be/src/util/cgroups-mgr.cc D be/src/util/cgroups-mgr.h M be/src/util/debug-util.h D be/src/util/llama-util.cc D be/src/util/llama-util.h M be/src/util/thread-pool.h M be/src/util/thread.cc M be/src/util/thread.h M be/src/util/uid-util.h M bin/bootstrap_toolchain.py M bin/generate_minidump_collection_testdata.py M bin/start-impala-cluster.py M common/thrift/CMakeLists.txt M common/thrift/Frontend.thrift M common/thrift/ImpalaInternalService.thrift D common/thrift/Llama.thrift D common/thrift/ResourceBrokerService.thrift M common/thrift/metrics.json M fe/src/main/java/com/cloudera/impala/planner/Planner.java M testdata/cluster/admin M testdata/cluster/node_templates/cdh5/etc/hadoop/conf/yarn-site.xml.tmpl D testdata/cluster/node_templates/cdh5/etc/init.d/llama-application D testdata/cluster/node_templates/cdh5/etc/llama/conf/llama-log4j.properties.tmpl D testdata/cluster/node_templates/cdh5/etc/llama/conf/llama-site.xml.tmpl M testdata/cluster/node_templates/common/etc/hadoop/conf/core-site.xml.tmpl 75 files changed, 124 insertions(+), 4,301 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/4445/2 -- To view, visit http://gerrit.cloudera.org:8080/4445 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icfb14209e31f6608bb7b8a33789e00411a6447ef Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry RobinsonGerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Matthew Jacobs