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

Colin Patrick McCabe commented on HADOOP-11238:
-----------------------------------------------

[~chrilisf], thanks for working on this.  Can you update the JIRA title and 
description to better describe the patch here?  It seems like the approach here 
is the same as on HADOOP-10077... refreshing the groups cache in the 
background.  Normally, the description and title of a JIRA should refer to the 
patch.  It is nice to have detailed information about exactly what happened on 
a particular cluster, but we generally put this in the first comment.  That 
way, it is easier to browse through JIRAs to see what each JIRA implemented.

The patch on HADOOP-10077 would need to be updated to handle negative caching 
and warningDeltaMs, which didn't exist back when I wrote it.  So I feel that 
the patch on this JIRA is more fresh.  If this JIRA gets committed, we should 
definitely mark HADOOP-10077 as a duplicate of it.

{code}
+    this.cache = CacheBuilder.newBuilder()
+      .refreshAfterWrite(cacheTimeout, TimeUnit.MILLISECONDS)
+      .ticker(new TimerToTickerAdapter(timer))
+      .build(new GroupCacheLoader());
{code}

Do we need to set the ticker here?  It seems that the default ticker uses 
{{System.nanoTime}}, which is the same monotonic time source that Hadoop's 
{{Timer}} uses.

Can we add a comment to {{GroupCacheLoader}} explaining that this code runs in 
the background if the cache entry is expired and there is an existing cache 
entry?  I don't think that would be completely clear to someone reading this 
for the first time.  And perhaps a similar comment to {{Groups#getGroups}} 
explaining that the access may block, but only if there is no existing entry.

bq. Benoy wrote: In the new implementation, it looks like a user's group will 
be refreshed periodically forever even if the user accessed Hadoop only once. 
Is there a way to prevent that ?

This is a good point.  The code currently never shrinks the cache, only grows 
it.  Why don't we use Guava's {{expireAfterWrite}} option to remove entries 
from the cache after a certain timeout?  I guess this could be a separate 
configuration option, or we could just use 10 * cacheTimeout.

> Group cache expiry causes namenode slowdown
> -------------------------------------------
>
>                 Key: HADOOP-11238
>                 URL: https://issues.apache.org/jira/browse/HADOOP-11238
>             Project: Hadoop Common
>          Issue Type: Bug
>    Affects Versions: 2.5.1
>            Reporter: Chris Li
>            Assignee: Chris Li
>            Priority: Minor
>         Attachments: HADOOP-11238.patch
>
>
> Our namenode pauses for 12-60 seconds several times every hour. During these 
> pauses, no new requests can come in.
> Around the time of pauses, we have log messages such as:
> 2014-10-22 13:24:22,688 WARN org.apache.hadoop.security.Groups: Potential 
> performance problem: getGroups(user=xxxxx) took 34507 milliseconds.
> The current theory is:
> 1. Groups has a cache that is refreshed periodically. Each entry has a cache 
> expiry.
> 2. When a cache entry expires, multiple threads can see this expiration and 
> then we have a thundering herd effect where all these threads hit the wire 
> and overwhelm our LDAP servers (we are using ShellBasedUnixGroupsMapping with 
> sssd, how this happens has yet to be established)
> 3. group resolution queries begin to take longer, I've observed it taking 1.2 
> seconds instead of the usual 0.01-0.03 seconds when measuring in the shell 
> `time groups myself`
> 4. If there is mutual exclusion somewhere along this path, a 1 second pause 
> could lead to a 60 second pause as all the threads compete for the resource. 
> The exact cause hasn't been established
> Potential solutions include:
> 1. Increasing group cache time, which will make the issue less frequent
> 2. Rolling evictions of the cache so we prevent the large spike in LDAP 
> queries
> 3. Gate the cache refresh so that only one thread is responsible for 
> refreshing the cache



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

Reply via email to