[
https://issues.apache.org/jira/browse/TEZ-3404?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15439987#comment-15439987
]
Hitesh Shah commented on TEZ-3404:
----------------------------------
Comments:
"dag.getDagAccessControls().mergeIntoAMConf(amConfig.getTezConfiguration());" -
why is the function called mergeIntoAMConf? maybe mergeToConf instead of there
is nothing AM specific about the params? Would be good to add a comment on why
this is being done here.
"public static DAGAccessControls deserializeFromConfiguration(Configuration
conf)" - is there any reason why configs are being used to pass this info from
client to AM instead of an explicit field as part of the dag plan proto?
{code}
Configuration conf = HistoryEventType.DAG_SUBMITTED ==
historyEvent.getEventType()
392 ? ((DAGSubmittedEvent)historyEvent).getConf()
393 : appContext.getCurrentDAG().getConf();
{code}
- In what scenario does the submit event not have a config?
" // Returns null historyACLPolicyManager return null, this will disable ACLs.
" - might be good to fix the docs as this line is a bit confusing. Also not
sure I understand why some code will disable ACLs?
{code}
4 DAGAccessControls dagAccessControls =
DAGAccessControls.deserializeFromConfiguration(dagConf);
465 try {
466 Map<String, String> acls =
historyACLPolicyManager.setupSessionDAGACLs(
467 dagConf, appContext.getApplicationID(),
Integer.toString(dagId.getId()),
468 dagAccessControls);
469 if (acls != null) {
470 return acls.get(TezConfiguration.YARN_ATS_ACL_DAG_DOMAIN_ID);
471 }
{code}
- The var name acls in "Map<String, String> acls" seems very confusing. This
map tracks the domain info and has nothing to do with acls as far as I can
understand.
{code}
409 if (appContext.isSession()) {
410 acls = historyACLPolicyManager.setupSessionACLs(getConfig(),
appContext.getApplicationID());
411 } else {
412 acls = historyACLPolicyManager.setupNonSessionACLs(getConfig(),
413 appContext.getApplicationID(), null);
{code}
- given the client side merge of acls in the non-session case, why is there a
need to call setupNonSessionACLs() ?
" @Test(timeout=5000)" - may want to bump this up to 10 secs esp as the test
does an internal sleep of 2.5 seconds to be on the safer side.
More comments after the test failures are addressed.
> Remove blocking call for ATS domain creation from client side to AM
> -------------------------------------------------------------------
>
> Key: TEZ-3404
> URL: https://issues.apache.org/jira/browse/TEZ-3404
> Project: Apache Tez
> Issue Type: Improvement
> Reporter: Harish Jaiprakash
> Assignee: Harish Jaiprakash
> Attachments: TEZ-3404-WIP-01.patch, TEZ-3404-WIP-02.patch,
> TEZ-3404-WIP-03.patch, TEZ-3404-WIP-04.patch
>
>
> Today, there is a blocking call for creating the domain on the client side
> which blocks hive query throughput. We need to move this to the AM and still
> retain the same security guarantees.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
