Skip to content

Commit

Permalink
Revert "[apache#772] fix(kerberos): cache proxy user ugi to avoid mem…
Browse files Browse the repository at this point in the history
…ory leak (apache#773)"

This reverts commit 89c2b92.
  • Loading branch information
jerqi authored and xianjingfeng committed Jun 20, 2023
1 parent b6a725b commit e36e3a3
Showing 1 changed file with 1 addition and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,16 @@

import java.io.IOException;
import java.security.PrivilegedExceptionAction;
import java.util.Map;
import java.util.concurrent.Callable;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;

import com.google.common.annotations.VisibleForTesting;
import org.apache.commons.lang3.StringUtils;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.security.UserGroupInformation;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import org.apache.uniffle.common.util.JavaUtils;
import org.apache.uniffle.common.util.ThreadUtils;

public class HadoopSecurityContext implements SecurityContext {
Expand All @@ -41,12 +38,6 @@ public class HadoopSecurityContext implements SecurityContext {
private UserGroupInformation loginUgi;
private ScheduledExecutorService refreshScheduledExecutor;

// 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.
private Map<String, UserGroupInformation> proxyUserUgiPool;

public HadoopSecurityContext(
String krb5ConfPath,
String keytabFile,
Expand Down Expand Up @@ -81,7 +72,6 @@ public HadoopSecurityContext(
refreshIntervalSec,
refreshIntervalSec,
TimeUnit.SECONDS);
proxyUserUgiPool = JavaUtils.newConcurrentMap();
}

private void authRefresh() {
Expand All @@ -101,10 +91,8 @@ public <T> T runSecured(String user, Callable<T> securedCallable) throws Excepti

// Run with the proxy user.
if (!user.equals(loginUgi.getShortUserName())) {
UserGroupInformation proxyUserUgi =
proxyUserUgiPool.computeIfAbsent(user, x -> UserGroupInformation.createProxyUser(x, loginUgi));
return executeWithUgiWrapper(
proxyUserUgi,
UserGroupInformation.createProxyUser(user, loginUgi),
securedCallable
);
}
Expand All @@ -122,20 +110,10 @@ private <T> T executeWithUgiWrapper(UserGroupInformation ugi, Callable<T> callab
return ugi.doAs((PrivilegedExceptionAction<T>) callable::call);
}

// Only for tests
@VisibleForTesting
Map<String, UserGroupInformation> getProxyUserUgiPool() {
return proxyUserUgiPool;
}

@Override
public void close() throws IOException {
if (refreshScheduledExecutor != null) {
refreshScheduledExecutor.shutdown();
}
if (proxyUserUgiPool != null) {
proxyUserUgiPool.clear();
proxyUserUgiPool = null;
}
}
}

0 comments on commit e36e3a3

Please sign in to comment.