You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@nifi.apache.org by rickysaltzer <gi...@git.apache.org> on 2015/09/29 20:23:16 UTC

[GitHub] nifi pull request: NIFI-997: Periodically Renew Kerberos Tickets

GitHub user rickysaltzer opened a pull request:

    https://github.com/apache/nifi/pull/97

    NIFI-997: Periodically Renew Kerberos Tickets

    Adding a patch to renew ticket every 4 hours to avoid inactive Kerberos tickets. This was an issue found when running Kerberos enabled Hadoop processors for a long period of time. This technically _should_ have been handled by the Hadoop library, but due to unknown issues, the renewal thread inside of Hadoop doesn't seem to be doing that. 
    
    This patch is fairly simplistic, and applies to all Hadoop processors as it's implemented at on the AbstractHadoopProcessor. The kerberos ticket age is checked against a threshold (4 hours is a safe bet) when getFileSystem() is called. If the age exceeds the threshold, we re-login using the UserGroupInformation class before passing back the filesystem. 

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/rickysaltzer/nifi kerberos-renewal

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/nifi/pull/97.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #97
    
----
commit b2eb61dca1204afb317bd40346065aa6a0e97647
Author: ricky <ri...@cloudera.com>
Date:   2015-09-25T18:15:09Z

    NIFI-997: Periodically Renew Kerberos Tickets
    
    - Renew ticket every 4 hours to avoid inactive Kerberos tickets.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-997: Periodically Renew Kerberos Tickets

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on the pull request:

    https://github.com/apache/nifi/pull/97#issuecomment-150413539
  
    The good news. It seems to work when I tested it.
    
    One comment on style:
    Having a tuple in a tuple for what I think is a triple for hdfsResources was a little bit hard to read. Has it gotten big enough to be its own class with getters for readability?
    
    I really had to think about what this was doing, and whether the first getValue could return null
    ```
    hdfsResources.get().getValue().getValue())
    ```
    
    this is very evident here where you created a method to read the usergroupinfo and then replicate the work of the method the line below. 
    ``` 
        if (getUserGroupInformation() != null && isTicketOld()) {
            renewKerberosTicket(hdfsResources.get().getValue().getValue());
        }
    ...
    
        protected UserGroupInformation getUserGroupInformation() {
            return hdfsResources.get().getValue().getValue();
        }
    ```
    
    Other comments:
    
    I was trying to reason about your error handling if ugi.checkTGTAndReloginFromKeytab threw an IOException. If this was greenfield code, I'd say that behavior I'd expect is that getFileSystem should throw an IOException, but that would change the method signature and change all the processors that use it (and be a breaking api change). Swallowing the exception, then exponentially backing off on a retry almost seems more logical, but I think it is worth discussing.
    
    Is there a reason you divide the millisecond times by 1000 for comparisons with seconds instead of multiplying the seconds by 1? You're losing precision on the time stamp.
    
    Did you think about trying to get the retry interval be a property instead of being hardcoded?
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-997: Periodically Renew Kerberos Tickets

Posted by rickysaltzer <gi...@git.apache.org>.
Github user rickysaltzer commented on the pull request:

    https://github.com/apache/nifi/pull/97#issuecomment-150675281
  
    I don't think we would have NPE'd because of the order that getFileSystem() ends up being called, but it's good to take care of anyway just in case. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-997: Periodically Renew Kerberos Tickets

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on the pull request:

    https://github.com/apache/nifi/pull/97#issuecomment-150882926
  
    It looks like due to a bug in hadoop 2.6.0 client code (on a java 8 JVM only?) your code doesn't try to reload the ticket:
    
    What `checkTGTAndReloginFromKeytab()` does:
    
    ```
      public synchronized void checkTGTAndReloginFromKeytab() throws IOException {
        if (!isSecurityEnabled()
            || user.getAuthenticationMethod() != AuthenticationMethod.KERBEROS
            || !isKeytab)
          return;
    ```
    
    Here is how isKeyTab is set in 2.6.0
    https://github.com/apache/hadoop/blob/release-2.6.0/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java#L597
    
    This returns false for me, always!
    
    Here is what 2.6.1 is doing
    https://github.com/apache/hadoop/blob/e286512a7143427f2975ec92cdc4fad0a093a456/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java#L623
    
    I'm going to try a java 7 vm.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-997: Periodically Renew Kerberos Tickets

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on the pull request:

    https://github.com/apache/nifi/pull/97#issuecomment-150736369
  
    My kerberos hadoop vm has gone south. I've got to rebuild it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-997: Periodically Renew Kerberos Tickets

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/nifi/pull/97


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-997: Periodically Renew Kerberos Tickets

Posted by rickysaltzer <gi...@git.apache.org>.
Github user rickysaltzer commented on the pull request:

    https://github.com/apache/nifi/pull/97#issuecomment-150674086
  
    Good catch, I'll fix that. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-997: Periodically Renew Kerberos Tickets

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on the pull request:

    https://github.com/apache/nifi/pull/97#issuecomment-150889085
  
    Also of note, hadoop 2.6 really doesn't like when (max_life < 10m) in your kdc.conf (looks like 2.7 may be better). 
    https://github.com/apache/hadoop/blob/release-2.6.1/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java#L1154
    
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-997: Periodically Renew Kerberos Tickets

Posted by rickysaltzer <gi...@git.apache.org>.
Github user rickysaltzer commented on the pull request:

    https://github.com/apache/nifi/pull/97#issuecomment-150593630
  
    @trkurc would you mind testing this latest commit, as it makes the renewal period configurable now. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-997: Periodically Renew Kerberos Tickets

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on the pull request:

    https://github.com/apache/nifi/pull/97#issuecomment-150883155
  
    And it totally works in java 7.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-997: Periodically Renew Kerberos Tickets

Posted by rickysaltzer <gi...@git.apache.org>.
Github user rickysaltzer commented on the pull request:

    https://github.com/apache/nifi/pull/97#issuecomment-150593718
  
    I will squash these changes before we commit. Keeping a revision history in this pull request for clarity. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-997: Periodically Renew Kerberos Tickets

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on the pull request:

    https://github.com/apache/nifi/pull/97#issuecomment-150673073
  
    @rickysaltzer - I don't have my kerberos hadoop running atm, but here are some comments:
    
    This variable should be camelCase, rather than ALL_CAPS.
    
    ```
    private long TICKET_RENEWAL_THRESHOLD_SEC;
    ```
    
    I think `hdfsResources.set(null);` should probably be `hdfsResources.set(new HdfsResources(null, null, null);` .. Walking through the code by hand...won't  getUserGroupInformation in getFileSystem NPE when you're not using kerberos?
    
    Will send more when I have a chance to build and run.
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-997: Periodically Renew Kerberos Tickets

Posted by rickysaltzer <gi...@git.apache.org>.
Github user rickysaltzer commented on the pull request:

    https://github.com/apache/nifi/pull/97#issuecomment-150578022
  
    I agree with you on the tuple within a tuple, it is pretty confusing. I'll work on getting these moved into its own class. You're right, error handling is a bit tricky in this area of the code in order to avoid breaking API changes. I did the division by 1000 since the threshold is in seconds, but I could change that. Precision isn't a big deal here since the renewal isn't millisecond (or even second) time sensitive. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-997: Periodically Renew Kerberos Tickets

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on the pull request:

    https://github.com/apache/nifi/pull/97#issuecomment-149424193
  
    I've started digging into this. Testing is the major challenge


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-997: Periodically Renew Kerberos Tickets

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on the pull request:

    https://github.com/apache/nifi/pull/97#issuecomment-150826424
  
    @rickysaltzer  one thing I missed is that the HDFSResources class should be static


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-997: Periodically Renew Kerberos Tickets

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on the pull request:

    https://github.com/apache/nifi/pull/97#issuecomment-150826350
  
    well, think I discovered an (unrelated) bug in the hdfs PutFile processor that will pass a flow file on to success even if the write fails due to SASL problems. 
    
    https://issues.apache.org/jira/browse/NIFI-1062


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---