[ 
https://issues.apache.org/jira/browse/HADOOP-9747?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16340431#comment-16340431
 ] 

Bharat Viswanadham edited comment on HADOOP-9747 at 1/26/18 1:59 AM:
---------------------------------------------------------------------

Hi [~daryn]

Thank You for the patch.

I am going through the patch, have some questions when I am trying to 
understand the code and also have some review comments.
 * In hasKerberosCredential methods, add user!=null check, as it might throw 
NPE, when user=null.
 * In case of loginuserfromSubject, as we pass params as null to 
doSubjectLogin, the configuration will be OS_SPECIFIC_LOGIN, but if the subject 
passed to this method has keytab and principal, then how login happens. So, in 
this case login fallbacks to Simple Auth login? Is this the expected behavior.
 **  @Override
 public AppConfigurationEntry[] getAppConfigurationEntry(String appName) {
 ArrayList<AppConfigurationEntry> entries = new ArrayList<>();
 // login of external subject passes no params. technically only
 // existing credentials should be used but other components expect
 // the login to succeed with local user fallback if no principal.
 if (params == null || appName.equals(SIMPLE_CONFIG_NAME)) \{ 
entries.add(OS_SPECIFIC_LOGIN); }
 ** So, the AppConfigEntry will have loginModule as OS_LOGIN_MODULE,
 * renewTGT and refreshKrb5Config is not set in IBM_JAVA case, is there any 
reason for this?
 * @Test annotation is missing for tests in TestUGILoginFromKeytab.java
 * Following 3 methods perform login and update the static loginUser. It might 
make sense to add documentation that these update the global loginUser.
 getLoginUser, loginUserFromSubject and loginUserFromKeytab

I am still going through patch, and still reviewing test cases. Will update if 
I have more comments.


was (Author: bharatviswa):
Hi [~daryn]

Thank You for the patch.

I am going through the patch, have some questions when I am trying to 
understand the code and also have some review comments.
 * In hasKerberosCredential methods, add user!=null check, as it might throw 
NPE, when user=null.
 * In case of loginuserfromSubject, as we pass params as null to 
doSubjectLogin, the configuration will be OS_SPECIFIC_LOGIN, but if it is a 
subject which has keytab and principal, then how login happens. So, in this 
case login fallbacks to Simple Auth login? Is this the expected behavior.
 **  @Override
public AppConfigurationEntry[] getAppConfigurationEntry(String appName) {
ArrayList<AppConfigurationEntry> entries = new ArrayList<>();
// login of external subject passes no params. technically only
// existing credentials should be used but other components expect
// the login to succeed with local user fallback if no principal.
if (params == null || appName.equals(SIMPLE_CONFIG_NAME)) {
entries.add(OS_SPECIFIC_LOGIN);
}
 ** So, the AppConfigEntry will have loginModule as OS_LOGIN_MODULE,
 * renewTGT and refreshKrb5Config is not set in IBM_JAVA case, is there any 
reason for this?
 * @Test annotation is missing for tests in TestUGILoginFromKeytab.java
 * Following 3 methods perform login and update the static loginUser. It might 
make sense to add documentation that these update the global loginUser.
getLoginUser, loginUserFromSubject and loginUserFromKeytab

I am still going through patch, and still reviewing test cases. Will update if 
I have more comments.

> Reduce unnecessary UGI synchronization
> --------------------------------------
>
>                 Key: HADOOP-9747
>                 URL: https://issues.apache.org/jira/browse/HADOOP-9747
>             Project: Hadoop Common
>          Issue Type: Bug
>          Components: security
>    Affects Versions: 0.23.0, 2.0.0-alpha, 3.0.0-alpha1
>            Reporter: Daryn Sharp
>            Assignee: Daryn Sharp
>            Priority: Critical
>         Attachments: HADOOP-9747-trunk-03.patch, HADOOP-9747-trunk.01.patch, 
> HADOOP-9747-trunk.02.patch, HADOOP-9747.2.branch-2.patch, 
> HADOOP-9747.2.trunk.patch, HADOOP-9747.branch-2.patch, HADOOP-9747.trunk.patch
>
>
> Jstacks of heavily loaded NNs show up to dozens of threads blocking in the 
> UGI.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

Reply via email to