You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Henry Robinson (Code Review)" <ge...@cloudera.org> on 2016/09/17 21:48:50 UTC

[Impala-ASF-CR] Remove Llama support.

Henry Robinson has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/4445

Change subject: Remove Llama support.
......................................................................

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.

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/ImpalaInternalService.thrift
D common/thrift/Llama.thrift
D common/thrift/ResourceBrokerService.thrift
M common/thrift/metrics.json
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
73 files changed, 117 insertions(+), 4,296 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/4445/1
-- 
To view, visit http://gerrit.cloudera.org:8080/4445
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Icfb14209e31f6608bb7b8a33789e00411a6447ef
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>

[Impala-ASF-CR] IMPALA-4160: Remove Llama support.

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Hello Marcel Kornacker, Matthew Jacobs,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/4445

to look at the new patch set (#5).

Change subject: IMPALA-4160: Remove Llama support.
......................................................................

IMPALA-4160: 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 (see IMPALA-4159)
* 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/create-test-configuration.sh
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
76 files changed, 181 insertions(+), 4,392 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/4445/5
-- 
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: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>

[Impala-ASF-CR] Remove Llama support.

Posted by "Marcel Kornacker (Code Review)" <ge...@cloudera.org>.
Marcel Kornacker has posted comments on this change.

Change subject: Remove Llama support.
......................................................................


Patch Set 1:

(5 comments)

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?

have you done a global search for vcores?


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 "
what about this? also, it looks like the header has a bunch of llama references that should be removed.


Line 136:   jstring llama_site_path =
?


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?


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


-- 
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 Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4160: Remove Llama support.

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has uploaded a new patch set (#3).

Change subject: IMPALA-4160: Remove Llama support.
......................................................................

IMPALA-4160: 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 (see IMPALA-4159)
* 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, 127 insertions(+), 4,323 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/4445/3
-- 
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: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>

[Impala-ASF-CR] Remove Llama support.

Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.

Change subject: Remove Llama support.
......................................................................


Patch Set 1:

(2 comments)

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 "
> what about this? also, it looks like the header has a bunch of llama refere
This file is unfortunately still referenced by admission control even when there is no real llama service (CM generates it) for a few per-pool configurations. On our AC/RM roadmap is cleaning up the configuration story.


Line 136:   jstring llama_site_path =
> ?
same thing I mentioned above


-- 
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 Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4160: Remove Llama support.

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Hello Matthew Jacobs,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/4445

to look at the new patch set (#4).

Change subject: IMPALA-4160: Remove Llama support.
......................................................................

IMPALA-4160: 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 (see IMPALA-4159)
* 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/create-test-configuration.sh
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
76 files changed, 182 insertions(+), 4,393 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/4445/4
-- 
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: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>

[Impala-ASF-CR] Remove Llama support.

Posted by "Marcel Kornacker (Code Review)" <ge...@cloudera.org>.
Marcel Kornacker has posted comments on this change.

Change subject: Remove Llama support.
......................................................................


Patch Set 1:

(1 comment)

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 "
> This file is unfortunately still referenced by admission control even when 
good to know. please leave todo with explanatory comment.

does this need to be a c6 cleanup item?


-- 
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 Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4610: Remove Llama support.

Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-4610: Remove Llama support.
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4445/1//COMMIT_MSG
Commit Message:

Line 7: Remove Llama support.
> please add a JIRA
You beat me to it in patch set #2


-- 
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 Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4160: Remove Llama support.

Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-4160: Remove Llama support.
......................................................................


Patch Set 3: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4445/3/be/src/scheduling/request-pool-service.cc
File be/src/scheduling/request-pool-service.cc:

Line 47: // TODO: Rename / cleanup now that Llama is removed.
reference IMPALA-4159


-- 
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: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4160: Remove Llama support.

Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has posted comments on this change.

Change subject: IMPALA-4160: Remove Llama support.
......................................................................


Patch Set 5: Verified+1

-- 
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: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4160: Remove Llama support.

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.

Change subject: IMPALA-4160: Remove Llama support.
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4445/1/be/src/scheduling/query-schedule.cc
File be/src/scheduling/query-schedule.cc:

PS1, Line 112: bool ignored;
> can we print "deprecated" somewhere?
There's not an obvious place to do that; the only warning mechanism I can see is the RuntimeState's log which we don't have access to here. Printing a warning here to the process log wouldn't really help (users don't look at logs, operations staff do). Returning a bad status from parsing the query options would abort everything.  Perhaps the best thing to do is to somehow hide the query options from clients (so they can still be set, but not shown).


http://gerrit.cloudera.org:8080/#/c/4445/4/common/thrift/Frontend.thrift
File common/thrift/Frontend.thrift:

Line 390:   // TODO: Remove this and associated code in Planner.
> also add a todo in Planner.java, it's too easy to overlook here
Done


-- 
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: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-HasComments: Yes

Re: [Impala-ASF-CR] IMPALA-4160: Remove Llama support.

Posted by Henry Robinson <he...@cloudera.com>.
Huh, the GVO failed when the same job passed just fine when run manually.
Looks possibly like the machine had trouble, but looking into it.

On 19 September 2016 at 19:43, Internal Jenkins (Code Review) <
gerrit@cloudera.org> wrote:

> Internal Jenkins has posted comments on this change.
>
> Change subject: IMPALA-4160: Remove Llama support.
> ......................................................................
>
>
> Patch Set 5: Verified-1
>
> Build failed: http://sandbox.jenkins.cloudera.com/job/impala-
> external-gerrit-verify-merge-ASF/216/
>
> --
> 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: 5
> Gerrit-Project: Impala-ASF
> Gerrit-Branch: master
> Gerrit-Owner: Henry Robinson <he...@cloudera.com>
> Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
> Gerrit-Reviewer: Internal Jenkins
> Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
> Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
> Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
> Gerrit-HasComments: No
>



-- 
Henry Robinson
Software Engineer
Cloudera
415-994-6679

[Impala-ASF-CR] IMPALA-4160: Remove Llama support.

Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has posted comments on this change.

Change subject: IMPALA-4160: Remove Llama support.
......................................................................


Patch Set 5: Verified-1

Build failed: http://sandbox.jenkins.cloudera.com/job/impala-external-gerrit-verify-merge-ASF/216/

-- 
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: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4160: Remove Llama support.

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.

Change subject: IMPALA-4160: Remove Llama support.
......................................................................


Patch Set 5: Code-Review+2

Carry +2.

-- 
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: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4610: Remove Llama support.

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
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 Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4610: Remove Llama support.

Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
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:   <property>
            :     <name>yarn.scheduler.fair.continuous-scheduling-enabled</name>
            :     <value>true</value>
            :   </property>
            : 
            :   <property>
            :     <name>yarn.scheduler.fair.assignmultiple</name>
            :     <value>true</value>
            :   </property>
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 Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4160: Remove Llama support.

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.

Change subject: IMPALA-4160: Remove Llama support.
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4445/3/be/src/scheduling/request-pool-service.cc
File be/src/scheduling/request-pool-service.cc:

Line 47: // TODO: Rename / cleanup now that Llama is removed.
> reference IMPALA-4159
Done


-- 
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: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] Remove Llama support.

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.

Change subject: Remove Llama support.
......................................................................


Patch Set 1:

Passed a core test run.

-- 
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 Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4160: Remove Llama support.

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.

Change subject: IMPALA-4160: Remove Llama support.
......................................................................


Patch Set 1:

(13 comments)

http://gerrit.cloudera.org:8080/#/c/4445/1//COMMIT_MSG
Commit Message:

Line 7: Remove Llama support.
> You beat me to it in patch set #2
Done


PS1, Line 31: * Remove RM flags (--enable_rm etc.)
            : * Remove RM query options
            : * Changes to RequestPoolService.
> reference https://issues.cloudera.org/browse/IMPALA-4159
Done


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?
I will clang-format this patch separately.


PS1, Line 87: MemTracker::MemTracker(UIntGauge* consumption_metric,
            :     int64_t byte_limit, const string& label)
> 1 line?
See above.


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
Done


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.
Done


PS1, Line 190: new limit=" << limit << " prev=" << limit;
> new/prev isn't interesting anymore, this is enough:
Done


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'
I don't think we can or should remove the query option now, as that would break some clients I think. I'll leave a TODO to remove this logic if admission control depends on this. I could see someone using this option to force admission control behavior by capping the mem estimate.


PS1, Line 120: FLAGS_rm_default_memory
> same as above.
Done


PS1, Line 125:   // TODO: Get this limit from Llama (Yarn sets it).
> remove
Done


http://gerrit.cloudera.org:8080/#/c/4445/2/be/src/scheduling/query-schedule.h
File be/src/scheduling/query-schedule.h:

PS2, Line 88:   int64_t GetClusterMemoryEstimate() const;
> I think this can be removed
Done


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: 
> We can remove the VCores stuff now, though some users may be relying on the
Decided against this - users might rely on this for capacity estimations etc. MT might completely change the logic, in which case we can remove then.


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:   <property>
            :     <name>yarn.scheduler.fair.continuous-scheduling-enabled</name>
            :     <value>true</value>
            :   </property>
            : 
            :   <property>
            :     <name>yarn.scheduler.fair.assignmultiple</name>
            :     <value>true</value>
            :   </property>
> I think a lot of this is stuff that was needed for Llama. Can you leave a T
Done. I think we are very close to being able to remove 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: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4610: Remove Llama support.

Posted by "Taras Bobrovytsky (Code Review)" <ge...@cloudera.org>.
Taras Bobrovytsky has posted comments on this change.

Change subject: IMPALA-4610: Remove Llama support.
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4445/2//COMMIT_MSG
Commit Message:

PS2, Line 7: 4610
Wrong jira number.


-- 
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 Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-HasComments: Yes

Re: [Impala-ASF-CR] IMPALA-4160: Remove Llama support.

Posted by Matthew Jacobs <mj...@cloudera.com>.
Yup, sure thing

On Mon, Sep 19, 2016 at 4:54 PM, Henry Robinson <he...@cloudera.com> wrote:
> Sounds good - if it's ok by you, will do in a separate commit since the
> first one is well into GVO-land...
>
> On 19 September 2016 at 16:19, Matthew Jacobs (Code Review)
> <ge...@cloudera.org> wrote:
>>
>> Matthew Jacobs has posted comments on this change.
>>
>> Change subject: IMPALA-4160: Remove Llama support.
>> ......................................................................
>>
>>
>> Patch Set 5:
>>
>> (3 comments)
>>
>> a few more small things to remove
>>
>> http://gerrit.cloudera.org:8080/#/c/4445/5/testdata/cluster/admin
>> File testdata/cluster/admin:
>>
>> PS5, Line 82: export LLAMA_WEBUI_PORT=1501  # same as default
>> remove
>>
>>
>> PS5, Line 262:     # Escape the first : to workaround
>> https://jira.cloudera.com/browse/CDH-16840
>>              :
>> LLAMA_PORT_MAPPINGS+="$HADOOP_HOSTNAME\\:$DATANODE_PORT="
>>              :
>> LLAMA_PORT_MAPPINGS+="$HADOOP_HOSTNAME:$NODEMANAGER_PORT
>> We can probably remove these
>>
>>
>> PS5, Line 266: LLAMA_PORT_MAPPINGS
>> this too
>>
>>
>> --
>> 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: 5
>> Gerrit-Project: Impala-ASF
>> Gerrit-Branch: master
>> Gerrit-Owner: Henry Robinson <he...@cloudera.com>
>> Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
>> Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
>> Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
>> Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
>> Gerrit-HasComments: Yes
>
>
>
>
> --
> Henry Robinson
> Software Engineer
> Cloudera
> 415-994-6679

Re: [Impala-ASF-CR] IMPALA-4160: Remove Llama support.

Posted by Henry Robinson <he...@cloudera.com>.
Sounds good - if it's ok by you, will do in a separate commit since the
first one is well into GVO-land...

On 19 September 2016 at 16:19, Matthew Jacobs (Code Review) <
gerrit@cloudera.org> wrote:

> Matthew Jacobs has posted comments on this change.
>
> Change subject: IMPALA-4160: Remove Llama support.
> ......................................................................
>
>
> Patch Set 5:
>
> (3 comments)
>
> a few more small things to remove
>
> http://gerrit.cloudera.org:8080/#/c/4445/5/testdata/cluster/admin
> File testdata/cluster/admin:
>
> PS5, Line 82: export LLAMA_WEBUI_PORT=1501  # same as default
> remove
>
>
> PS5, Line 262:     # Escape the first : to workaround
> https://jira.cloudera.com/browse/CDH-16840
>              :     LLAMA_PORT_MAPPINGS+="$HADOOP_
> HOSTNAME\\:$DATANODE_PORT="
>              :     LLAMA_PORT_MAPPINGS+="$HADOOP_
> HOSTNAME:$NODEMANAGER_PORT
> We can probably remove these
>
>
> PS5, Line 266: LLAMA_PORT_MAPPINGS
> this too
>
>
> --
> 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: 5
> Gerrit-Project: Impala-ASF
> Gerrit-Branch: master
> Gerrit-Owner: Henry Robinson <he...@cloudera.com>
> Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
> Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
> Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
> Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
> Gerrit-HasComments: Yes
>



-- 
Henry Robinson
Software Engineer
Cloudera
415-994-6679

[Impala-ASF-CR] IMPALA-4160: Remove Llama support.

Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-4160: Remove Llama support.
......................................................................


Patch Set 5:

(3 comments)

a few more small things to remove

http://gerrit.cloudera.org:8080/#/c/4445/5/testdata/cluster/admin
File testdata/cluster/admin:

PS5, Line 82: export LLAMA_WEBUI_PORT=1501  # same as default
remove


PS5, Line 262:     # Escape the first : to workaround https://jira.cloudera.com/browse/CDH-16840
             :     LLAMA_PORT_MAPPINGS+="$HADOOP_HOSTNAME\\:$DATANODE_PORT="
             :     LLAMA_PORT_MAPPINGS+="$HADOOP_HOSTNAME:$NODEMANAGER_PORT
We can probably remove these


PS5, Line 266: LLAMA_PORT_MAPPINGS
this too


-- 
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: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] Remove Llama support.

Posted by "Marcel Kornacker (Code Review)" <ge...@cloudera.org>.
Marcel Kornacker has posted comments on this change.

Change subject: Remove Llama support.
......................................................................


Patch Set 1:

there's also related fe code. would be good to cover that as well.

-- 
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 Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4160: Remove Llama support.

Posted by "Marcel Kornacker (Code Review)" <ge...@cloudera.org>.
Marcel Kornacker has posted comments on this change.

Change subject: IMPALA-4160: Remove Llama support.
......................................................................


Patch Set 4: Code-Review+2

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4445/1/be/src/scheduling/query-schedule.cc
File be/src/scheduling/query-schedule.cc:

PS1, Line 112: bool ignored;
> I don't think we can or should remove the query option now, as that would b
can we print "deprecated" somewhere?


http://gerrit.cloudera.org:8080/#/c/4445/4/common/thrift/Frontend.thrift
File common/thrift/Frontend.thrift:

Line 390:   // TODO: Remove this and associated code in Planner.
also add a todo in Planner.java, it's too easy to overlook here


-- 
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: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4610: Remove Llama support.

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
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 Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>

[Impala-ASF-CR] IMPALA-4160: Remove Llama support.

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has submitted this change and it was merged.

Change subject: IMPALA-4160: Remove Llama support.
......................................................................


IMPALA-4160: 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 (see IMPALA-4159)
* Remove estimates of VCores / memory from plan

Change-Id: Icfb14209e31f6608bb7b8a33789e00411a6447ef
Reviewed-on: http://gerrit.cloudera.org:8080/4445
Tested-by: Internal Jenkins
Reviewed-by: Henry Robinson <he...@cloudera.com>
---
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/create-test-configuration.sh
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
76 files changed, 181 insertions(+), 4,392 deletions(-)

Approvals:
  Henry Robinson: Looks good to me, approved
  Internal Jenkins: Verified



-- 
To view, visit http://gerrit.cloudera.org:8080/4445
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Icfb14209e31f6608bb7b8a33789e00411a6447ef
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>