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

ASF GitHub Bot commented on HDFS-17813:
---------------------------------------

Hexiaoqiao commented on code in PR #8313:
URL: https://github.com/apache/hadoop/pull/8313#discussion_r3166408381


##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameCache.java:
##########
@@ -147,9 +147,9 @@ void initialized() {
   }
   
   /** Promote a frequently used name to the cache */
-  private void promote(final K name) {
+  private void promote(final K name, final UseCount useCount) {
     transientMap.remove(name);
-    cache.put(name, name);
+    cache.put(name, useCount.value);
     lookups += useThreshold;
   }

Review Comment:
   How about fix it as the following code snippet? Thus we didn't change 
signature and could keep it more simple.
   ```
   private void promote(final K name) {
       UseCount useCount = transientMap.remove(name);
       cache.put(name, useCount.value);
       lookups += useThreshold;
   }
   ```





> NameCache promote wrong name to cache map
> -----------------------------------------
>
>                 Key: HDFS-17813
>                 URL: https://issues.apache.org/jira/browse/HDFS-17813
>             Project: Hadoop HDFS
>          Issue Type: Bug
>          Components: namenode
>            Reporter: khazhen
>            Priority: Minor
>              Labels: pull-request-available
>
> The NameCache class is used to cache frequently used names in namenode, it 
> promotes a name used more than useThreshold to the cache, the promote logic:
> {code:java}
> K put(final K name) {
>   K internal = cache.get(name);
>   if (internal != null) {
>     lookups++;
>     return internal;
>   }
>   // Track the usage count only during initialization
>   if (!initialized) {
>     UseCount useCount = transientMap.get(name);
>     if (useCount != null) {
>       useCount.increment();
>       if (useCount.get() >= useThreshold) {
>         promote(name); // name got promoted
>       }
>       return useCount.value;
>     }
>     useCount = new UseCount(name);
>     transientMap.put(name, useCount);
>   }
>   return null;
> } {code}
> When promoting, the cache stores the `name` parameter from put() instead of 
> the existing useCount.value. This causes the returned value to change after a 
> name is promoted, resulting in memory duplication.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to