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

Gregory Chanan commented on HADOOP-11157:
-----------------------------------------

Thanks for the patch, Arun.

Some notes:
- I haven't looked at the renew logic in depth and will probably skip it as 
outside the scope of this JIRA.
- I haven't looked at the new cancel logic in depth, but I will soon.
- The new tests look good

Other things:
{code}  private static final int NUM_ZK_RETRIES = 3;
+  private static final int ZK_RETRY_WAIT_MS = 200;
{code}
These don't seem to be used.

{code}
+        +        ZK_DTSM_ZK_SHUTDOWN_TIMEOUT_DEFAULT);
{code}
extra plus?

{code}    allKeys.put(key.getKeyId(), key);{code}
does this need to be synchronized?  Why does the currentToken stuff need to be, 
but this not?

{code}
      LOG.debug("Thread interrupted while performing token counter increment", 
e);
      LOG.debug("Thread interrupted while performing keyId increment", e);
{code}
Set the interrupt flag in both these cases?

{code}
  public ExecutorService getListenerThreadPool() {
{code}
@VisibleForTesting?

{code}
    Thread.sleep(5000);
{code}
Maybe make a constant (it's used a couple times) and reduce the time?  Seems 
like a second should be sufficient

{code}
    // Create a new token on ne ZKDTSM
{code}
ne?

{code}
  protected void verifyDestroy(DelegationTokenManager tm, Configuration conf)
{code}
rename to destroyAndVerify?

Some idents seem to be 2 spaces, some 4.

> ZKDelegationTokenSecretManager never shuts down listenerThreadPool
> ------------------------------------------------------------------
>
>                 Key: HADOOP-11157
>                 URL: https://issues.apache.org/jira/browse/HADOOP-11157
>             Project: Hadoop Common
>          Issue Type: Bug
>          Components: security
>    Affects Versions: 2.6.0
>            Reporter: Gregory Chanan
>            Assignee: Arun Suresh
>         Attachments: HADOOP-11157.2.patch, HADOOP-11157.3.patch, 
> HADOOP-11157.patch, HADOOP-11157.patch
>
>
> I'm trying to integrate Solr with the DelegationTokenAuthenticationFilter and 
> running into this issue.  The solr unit tests look for leaked threads and 
> when I started using the ZKDelegationTokenSecretManager it started reporting 
> leaks.  Shuting down the listenerThreadPool after the objects that use it 
> resolves the leak threads errors.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to