From 6b6ef7b8e35af9bd325f6ff670cdbd0fb1265c7d Mon Sep 17 00:00:00 2001 From: Arun Murthy Date: Thu, 8 Sep 2011 18:40:27 +0000 Subject: [PATCH] MAPREDUCE-2947. Fixed race condition in AuxiliaryServices. Contributed by Vinod K V. git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1166849 13f79535-47bb-0310-9956-ffa450edef68 --- hadoop-mapreduce-project/CHANGES.txt | 3 ++ .../impl/pb/StartContainerResponsePBImpl.java | 32 +++++++++---------- .../containermanager/AuxServices.java | 10 +++++- 3 files changed, 28 insertions(+), 17 deletions(-) diff --git a/hadoop-mapreduce-project/CHANGES.txt b/hadoop-mapreduce-project/CHANGES.txt index 236c08daaf..8e0f622b0c 100644 --- a/hadoop-mapreduce-project/CHANGES.txt +++ b/hadoop-mapreduce-project/CHANGES.txt @@ -1230,6 +1230,9 @@ Release 0.23.0 - Unreleased MAPREDUCE-2942. TestNMAuditLogger.testNMAuditLoggerWithIP failing (Thomas Graves via mahadev) + MAPREDUCE-2947. Fixed race condition in AuxiliaryServices. (vinodkv via + acmurthy) + Release 0.22.0 - Unreleased INCOMPATIBLE CHANGES diff --git a/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/protocolrecords/impl/pb/StartContainerResponsePBImpl.java b/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/protocolrecords/impl/pb/StartContainerResponsePBImpl.java index 4fbdf97c7c..a01b11bac1 100644 --- a/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/protocolrecords/impl/pb/StartContainerResponsePBImpl.java +++ b/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/protocolrecords/impl/pb/StartContainerResponsePBImpl.java @@ -48,20 +48,20 @@ public StartContainerResponsePBImpl(StartContainerResponseProto proto) { viaProto = true; } - public StartContainerResponseProto getProto() { + public synchronized StartContainerResponseProto getProto() { mergeLocalToProto(); proto = viaProto ? proto : builder.build(); viaProto = true; return proto; } - private void mergeLocalToBuilder() { + private synchronized void mergeLocalToBuilder() { if (this.serviceResponse != null) { addServiceResponseToProto(); } } - private void mergeLocalToProto() { + private synchronized void mergeLocalToProto() { if (viaProto) { maybeInitBuilder(); } @@ -70,7 +70,7 @@ private void mergeLocalToProto() { viaProto = true; } - private void maybeInitBuilder() { + private synchronized void maybeInitBuilder() { if (viaProto || builder == null) { builder = StartContainerResponseProto.newBuilder(proto); } @@ -79,17 +79,17 @@ private void maybeInitBuilder() { @Override - public Map getAllServiceResponse() { + public synchronized Map getAllServiceResponse() { initServiceResponse(); return this.serviceResponse; } @Override - public ByteBuffer getServiceResponse(String key) { + public synchronized ByteBuffer getServiceResponse(String key) { initServiceResponse(); return this.serviceResponse.get(key); } - private void initServiceResponse() { + private synchronized void initServiceResponse() { if (this.serviceResponse != null) { return; } @@ -103,14 +103,14 @@ private void initServiceResponse() { } @Override - public void addAllServiceResponse(final Map serviceResponse) { + public synchronized void addAllServiceResponse(final Map serviceResponse) { if (serviceResponse == null) return; initServiceResponse(); this.serviceResponse.putAll(serviceResponse); } - private void addServiceResponseToProto() { + private synchronized void addServiceResponseToProto() { maybeInitBuilder(); builder.clearServiceResponse(); if (serviceResponse == null) @@ -118,24 +118,24 @@ private void addServiceResponseToProto() { Iterable iterable = new Iterable() { @Override - public Iterator iterator() { + public synchronized Iterator iterator() { return new Iterator() { Iterator keyIter = serviceResponse.keySet().iterator(); @Override - public void remove() { + public synchronized void remove() { throw new UnsupportedOperationException(); } @Override - public StringBytesMapProto next() { + public synchronized StringBytesMapProto next() { String key = keyIter.next(); return StringBytesMapProto.newBuilder().setKey(key).setValue(convertToProtoFormat(serviceResponse.get(key))).build(); } @Override - public boolean hasNext() { + public synchronized boolean hasNext() { return keyIter.hasNext(); } }; @@ -144,17 +144,17 @@ public boolean hasNext() { builder.addAllServiceResponse(iterable); } @Override - public void setServiceResponse(String key, ByteBuffer val) { + public synchronized void setServiceResponse(String key, ByteBuffer val) { initServiceResponse(); this.serviceResponse.put(key, val); } @Override - public void removeServiceResponse(String key) { + public synchronized void removeServiceResponse(String key) { initServiceResponse(); this.serviceResponse.remove(key); } @Override - public void clearServiceResponse() { + public synchronized void clearServiceResponse() { initServiceResponse(); this.serviceResponse.clear(); } diff --git a/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/AuxServices.java b/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/AuxServices.java index 254ff2a671..647f56a80b 100644 --- a/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/AuxServices.java +++ b/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/AuxServices.java @@ -23,6 +23,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; +import java.util.Map.Entry; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -72,7 +73,14 @@ Collection getServices() { * the the name of the service as defined in the configuration. */ public Map getMeta() { - return Collections.unmodifiableMap(serviceMeta); + Map metaClone = new HashMap( + serviceMeta.size()); + synchronized (serviceMeta) { + for (Entry entry : serviceMeta.entrySet()) { + metaClone.put(entry.getKey(), entry.getValue().duplicate()); + } + } + return metaClone; } @Override