YARN-9958. Remove the invalid lock in ContainerExecutor (#1704)

This commit is contained in:
Wanqiang Ji 2019-12-04 13:35:41 +08:00 committed by Akira Ajisaka
parent ccca5f4a40
commit c48de9aa2d

View File

@ -34,8 +34,6 @@
import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock.ReadLock;
import java.util.concurrent.locks.ReentrantReadWriteLock.WriteLock;
import org.apache.hadoop.hdfs.protocol.datatransfer.IOStreamPair; import org.apache.hadoop.hdfs.protocol.datatransfer.IOStreamPair;
import org.slf4j.Logger; import org.slf4j.Logger;
@ -101,8 +99,6 @@ public abstract class ContainerExecutor implements Configurable {
private final ConcurrentMap<ContainerId, Path> pidFiles = private final ConcurrentMap<ContainerId, Path> pidFiles =
new ConcurrentHashMap<>(); new ConcurrentHashMap<>();
private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
private final ReadLock readLock = lock.readLock();
private final WriteLock writeLock = lock.writeLock();
private String[] whitelistVars; private String[] whitelistVars;
@Override @Override
@ -570,12 +566,7 @@ protected void logOutput(String output) {
* @return the path of the pid-file for the given containerId. * @return the path of the pid-file for the given containerId.
*/ */
protected Path getPidFilePath(ContainerId containerId) { protected Path getPidFilePath(ContainerId containerId) {
readLock.lock(); return this.pidFiles.get(containerId);
try {
return (this.pidFiles.get(containerId));
} finally {
readLock.unlock();
}
} }
/** /**
@ -721,12 +712,7 @@ protected String[] getRunCommandForOther(String command,
* @return true if the container is active * @return true if the container is active
*/ */
protected boolean isContainerActive(ContainerId containerId) { protected boolean isContainerActive(ContainerId containerId) {
readLock.lock(); return this.pidFiles.containsKey(containerId);
try {
return (this.pidFiles.containsKey(containerId));
} finally {
readLock.unlock();
}
} }
@VisibleForTesting @VisibleForTesting
@ -742,12 +728,7 @@ protected String getNMEnvVar(String varname) {
* of the launched process * of the launched process
*/ */
public void activateContainer(ContainerId containerId, Path pidFilePath) { public void activateContainer(ContainerId containerId, Path pidFilePath) {
writeLock.lock(); this.pidFiles.put(containerId, pidFilePath);
try {
this.pidFiles.put(containerId, pidFilePath);
} finally {
writeLock.unlock();
}
} }
// LinuxContainerExecutor overrides this method and behaves differently. // LinuxContainerExecutor overrides this method and behaves differently.
@ -778,12 +759,7 @@ public static String[] getLocalIpAndHost(Container container) {
* @param containerId the container ID * @param containerId the container ID
*/ */
public void deactivateContainer(ContainerId containerId) { public void deactivateContainer(ContainerId containerId) {
writeLock.lock(); this.pidFiles.remove(containerId);
try {
this.pidFiles.remove(containerId);
} finally {
writeLock.unlock();
}
} }
/** /**