[
https://issues.apache.org/jira/browse/TEZ-1233?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14313160#comment-14313160
]
Siddharth Seth commented on TEZ-1233:
-------------------------------------
Approach looks good to me. Comments on the patch.
- Rename methods to just setConf (instead of setDAGConf, setVertexConf) ?
They're used in the context of the vertex / dag anyway
- {code}+ Scope scope = TezConfiguration.PropertyScope.get(property);
+ if (scope == null) {
+ throw new IllegalStateException("property " + property + " has no
ConfigurationScope annotation, it is not supported");
+ }{code}
This limits the set to the properties in TezConfiguration. Instead, can we
validate properties which exist in TezConfiguration - and let all others
through ? That allows passing Configuration information to pluggable components
within the AM.
- {code}VERTEX, // can been set at AM/DAG/VERTEX level{code}
Do we expect any properties which may make sense only at the Vertex level, but
not at the DAG level ? Similarly for DAG implying AM. AM is probably a special
case, since it isn't directly tied to the execution work.
- PropertyScope should not be public in TezConfiguration. It can be changed by
user code. Instead, TezConfiguration should just expose a method to validate a
property that is passed in.
- Construction of PropertyScope - is this reflection based approach expensive ?
- Add a test similar to TestRuntimeConfiguration which validates that all
properties have an annotation on them.
- In DAGImpl, new Configuration is not required, that's already handled in the
DAGAppMaster. Also, is validation required for the dag conf property being set ?
- DAG.java should not modify dagConf that is passed in. That's the same
configuration instance that was used to create the TezClient.
- Also, If the TezClient configuration is available in the AM - then only
properties which are set at the dag level should be sent to the AM via RPC.
Otherwise, the list ends up being way too large. This is already handled for
the VertexConf.
> Allow configuration of framework parameters per vertex
> ------------------------------------------------------
>
> Key: TEZ-1233
> URL: https://issues.apache.org/jira/browse/TEZ-1233
> Project: Apache Tez
> Issue Type: Improvement
> Reporter: Siddharth Seth
> Assignee: Jeff Zhang
> Attachments: TEZ-1233-1.patch
>
>
> Currently, configuration properties specific in AMConfiguration are used to
> configure tasks - e.g. memory configs, process tree, etc. These should be
> configurable at a Vertex level.
> Also, tasks end up reading these configs from the dist cache each time -
> would be simpler to send over the wire as part of the TaskSpec.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
