[
https://issues.apache.org/jira/browse/TEZ-3404?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15447187#comment-15447187
]
Hitesh Shah commented on TEZ-3404:
----------------------------------
Comments:
{code}
// Merge the dag access controls into AM config.
949 if (dag.getDagAccessControls() != null) {
950
dag.getDagAccessControls().mergeIntoAmAcls(amConfig.getTezConfiguration());
951 }
{code}
- The same tez client if used to submit multiple dags in non-session mode
will pollute the am configs.
DAGImpl.java:
{code}
this.aclManager = new ACLManager(appContext.getAMACLManager(),
dagUGI.getShortUserName(),
this.dagConf);
{code}
- does this need to change?
{code}
if (aclInfo.getUsersWithViewAccessList() != null) {
886
dagAccessControls.setUsersWithViewACLs(aclInfo.getUsersWithViewAccessList());
887 }
888 if (aclInfo.getUsersWithModifyAccessList() != null) {
889
dagAccessControls.setUsersWithModifyACLs(aclInfo.getUsersWithModifyAccessList());
890 }
891 if (aclInfo.getGroupsWithViewAccessList() != null) {
892
dagAccessControls.setGroupsWithViewACLs(aclInfo.getGroupsWithViewAccessList());
893 }
894 if (aclInfo.getGroupsWithModifyAccessList() != null) {
895
dagAccessControls.setGroupsWithModifyACLs(aclInfo.getGroupsWithModifyAccessList());
896 }
{code}
- Are the list objects ever null? Does the proto have a has*() call we can
use? or shoudl we use the count function?
{code}
ACLInfo.Builder builder = ACLInfo.newBuilder();
873
builder.addAllUsersWithViewAccess(dagAccessControls.getUsersWithViewACLs());
874
builder.addAllUsersWithModifyAccess(dagAccessControls.getUsersWithModifyACLs());
875
builder.addAllGroupsWithViewAccess(dagAccessControls.getGroupsWithViewACLs());
876
builder.addAllGroupsWithModifyAccess(dagAccessControls.getGroupsWithModifyACLs());
877 return builder.build();
{code}
- no null check needed?
{code}
14 if (dagId == null || historyACLPolicyManager == null ||
415
!HistoryEventType.isDAGSpecificEvent(historyEvent.getEventType())) {
416 return domainId;
417 }
{code}
- from a code clarity perspective, shouldn't historyACLPolicyManager == null
be in its own if clause and return null?
{code}
createDagDomain(Configuration dagConf, DAGPlan dagPlan, TezDAGID dagId) {
477 if (appContext.isSession()) {
{code}
- code would be cleaner if the non-session mode was handled first in the if
clause. Also, docs need improvement to explain the non session code path. Docs
also reference sessionDomainId in createDAGDomain().
bq. The contract for HistoryACLPolicyManager seems to be too wide
In any case, we should probably clarify the behavior change and also update the
javadocs for the base class in that case. Granted that most impls are internal
only but it would be good to make sure the behavior change is very clearly
documented in this jira.
> 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, TEZ-3404-WIP-05.patch,
> TEZ-3404-WIP-06.patch, TEZ-3404-WIP-07.patch, TEZ-3404-WIP-08.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)
