From 44a003d8eca613ce51a63f55c575b1d00bdd0d6e Mon Sep 17 00:00:00 2001 From: Vinod Kumar Vavilapalli Date: Wed, 29 Feb 2012 04:55:29 +0000 Subject: [PATCH] MAPREDUCE-3931. Changed PB implementation of LocalResource to take locks so that race conditions don't fail tasks by inadvertantly changing the timestamps. Contributed by Siddarth Seth. git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1294970 13f79535-47bb-0310-9956-ffa450edef68 --- hadoop-mapreduce-project/CHANGES.txt | 4 ++ .../records/impl/pb/LocalResourcePBImpl.java | 58 ++++++++----------- 2 files changed, 29 insertions(+), 33 deletions(-) diff --git a/hadoop-mapreduce-project/CHANGES.txt b/hadoop-mapreduce-project/CHANGES.txt index d293b52b3f..8d965b5c59 100644 --- a/hadoop-mapreduce-project/CHANGES.txt +++ b/hadoop-mapreduce-project/CHANGES.txt @@ -207,6 +207,10 @@ Release 0.23.2 - UNRELEASED task attempt without an assigned container. (Robert Joseph Evans via sseth) + MAPREDUCE-3931. Changed PB implementation of LocalResource to take locks + so that race conditions don't fail tasks by inadvertantly changing the + timestamps. (Siddarth Seth via vinodkv) + Release 0.23.1 - 2012-02-17 INCOMPATIBLE CHANGES diff --git a/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/impl/pb/LocalResourcePBImpl.java b/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/impl/pb/LocalResourcePBImpl.java index 4dec36b570..dd57f309b9 100644 --- a/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/impl/pb/LocalResourcePBImpl.java +++ b/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/impl/pb/LocalResourcePBImpl.java @@ -32,14 +32,14 @@ -public class LocalResourcePBImpl extends ProtoBase implements LocalResource { +public class LocalResourcePBImpl extends ProtoBase + implements LocalResource { LocalResourceProto proto = LocalResourceProto.getDefaultInstance(); LocalResourceProto.Builder builder = null; boolean viaProto = false; - + private URL url = null; - - + public LocalResourcePBImpl() { builder = LocalResourceProto.newBuilder(); } @@ -48,60 +48,55 @@ public LocalResourcePBImpl(LocalResourceProto proto) { this.proto = proto; viaProto = true; } - - public LocalResourceProto getProto() { - mergeLocalToProto(); + + public synchronized LocalResourceProto getProto() { + mergeLocalToBuilder(); proto = viaProto ? proto : builder.build(); viaProto = true; return proto; } - private void mergeLocalToBuilder() { - if (this.url != null) { + private synchronized void mergeLocalToBuilder() { + LocalResourceProtoOrBuilder l = viaProto ? proto : builder; + if (this.url != null + && !(l.getResource().equals(((URLPBImpl) url).getProto()))) { + maybeInitBuilder(); + l = builder; builder.setResource(convertToProtoFormat(this.url)); } } - private void mergeLocalToProto() { - if (viaProto) - maybeInitBuilder(); - mergeLocalToBuilder(); - proto = builder.build(); - viaProto = true; - } - - private void maybeInitBuilder() { + private synchronized void maybeInitBuilder() { if (viaProto || builder == null) { builder = LocalResourceProto.newBuilder(proto); } viaProto = false; } - - + @Override - public long getSize() { + public synchronized long getSize() { LocalResourceProtoOrBuilder p = viaProto ? proto : builder; return (p.getSize()); } @Override - public void setSize(long size) { + public synchronized void setSize(long size) { maybeInitBuilder(); builder.setSize((size)); } @Override - public long getTimestamp() { + public synchronized long getTimestamp() { LocalResourceProtoOrBuilder p = viaProto ? proto : builder; return (p.getTimestamp()); } @Override - public void setTimestamp(long timestamp) { + public synchronized void setTimestamp(long timestamp) { maybeInitBuilder(); builder.setTimestamp((timestamp)); } @Override - public LocalResourceType getType() { + public synchronized LocalResourceType getType() { LocalResourceProtoOrBuilder p = viaProto ? proto : builder; if (!p.hasType()) { return null; @@ -110,7 +105,7 @@ public LocalResourceType getType() { } @Override - public void setType(LocalResourceType type) { + public synchronized void setType(LocalResourceType type) { maybeInitBuilder(); if (type == null) { builder.clearType(); @@ -119,7 +114,7 @@ public void setType(LocalResourceType type) { builder.setType(convertToProtoFormat(type)); } @Override - public URL getResource() { + public synchronized URL getResource() { LocalResourceProtoOrBuilder p = viaProto ? proto : builder; if (this.url != null) { return this.url; @@ -132,14 +127,14 @@ public URL getResource() { } @Override - public void setResource(URL resource) { + public synchronized void setResource(URL resource) { maybeInitBuilder(); if (resource == null) builder.clearResource(); this.url = resource; } @Override - public LocalResourceVisibility getVisibility() { + public synchronized LocalResourceVisibility getVisibility() { LocalResourceProtoOrBuilder p = viaProto ? proto : builder; if (!p.hasVisibility()) { return null; @@ -148,7 +143,7 @@ public LocalResourceVisibility getVisibility() { } @Override - public void setVisibility(LocalResourceVisibility visibility) { + public synchronized void setVisibility(LocalResourceVisibility visibility) { maybeInitBuilder(); if (visibility == null) { builder.clearVisibility(); @@ -180,7 +175,4 @@ private LocalResourceVisibilityProto convertToProtoFormat(LocalResourceVisibilit private LocalResourceVisibility convertFromProtoFormat(LocalResourceVisibilityProto e) { return ProtoUtils.convertFromProtoFormat(e); } - - - }