impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Matthew Jacobs (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4610: Remove Llama support.
Date Mon, 19 Sep 2016 19:39:30 GMT
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 <henry@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tbobrovytsky@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message