[GitHub] [incubator-uniffle] advancedxy commented on a diff in pull request #773: [#772] fix(kerberos): cache proxy user ugi to avoid memory leak

["advancedxy (via GitHub)" <gi...@apache.org>]

advancedxy commented on code in PR #773:
URL: https://github.com/apache/incubator-uniffle/pull/773#discussion_r1150338889


##########
common/src/main/java/org/apache/uniffle/common/security/HadoopSecurityContext.java:
##########
@@ -39,6 +42,12 @@ public class HadoopSecurityContext implements SecurityContext {
   private UserGroupInformation loginUgi;
   private ScheduledExecutorService refreshScheduledExecutor;
 
+  // The cache of proxy user ugi is to avoid creating the different cache keys of
+  // Hadoop filesystem for the same user, scheme and authority, this will cache too
+  // much unnecessary filesystem instances in memory and then cause memory leak.

Review Comment:
   `too much unnecessary ` seems a bit weird...
   
   You could try the polished version from ChatGPT, 😄 
   ```
   // The purpose of the proxy user ugi cache is to prevent the creation of multiple cache keys for the same user, scheme, and authority in the Hadoop filesystem. Without this cache, large amounts of unnecessary filesystem instances could be stored in memory, leading to potential memory leaks. For more information on this issue, refer to #706.
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org