HADOOP-4276. Improve the hashing functions and deserialization of the

mapred ID classes. (omalley)


git-svn-id: https://svn.apache.org/repos/asf/hadoop/core/trunk@707473 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -49,6 +49,9 @@ Trunk (unreleased changes)
     HADOOP-4431. Add versionning/tags to Chukwa Chunk. 
     (Jerome Boulon via Johan)
 
+    HADOOP-4276. Improve the hashing functions and deserialization of the 
+    mapred ID classes. (omalley)
+
   OPTIMIZATIONS
 
   BUG FIXES

src/contrib/capacity-scheduler/src/java/org/apache/hadoop/mapred/CapacityTaskScheduler.java

@@ -665,7 +665,7 @@ class CapacityTaskScheduler extends TaskScheduler {
         t = obtainNewTask(taskTracker, j);
         if (t != null) {
           LOG.debug("Got task from job " + 
-              j.getJobID().toStringWOPrefix() + " in queue " + qsi.queueName);
+                    j.getJobID() + " in queue " + qsi.queueName);
           return t;
         }
       }
@@ -686,7 +686,7 @@ class CapacityTaskScheduler extends TaskScheduler {
         t = obtainNewTask(taskTracker, j);
         if (t != null) {
           LOG.debug("Getting task from job " + 
-              j.getJobID().toStringWOPrefix() + " in queue " + qsi.queueName);
+                    j.getJobID() + " in queue " + qsi.queueName);
           return t;
         }
       }
@@ -705,7 +705,7 @@ class CapacityTaskScheduler extends TaskScheduler {
             t = obtainNewTask(taskTracker, j);
             if (t != null) {
               LOG.debug("Getting task from job " + 
-                  j.getJobID().toStringWOPrefix() + " in queue " + qsi.queueName);
+                        j.getJobID() + " in queue " + qsi.queueName);
               return t;
             }
           }
@@ -718,7 +718,7 @@ class CapacityTaskScheduler extends TaskScheduler {
             t = obtainNewTask(taskTracker, j);
             if (t != null) {
               LOG.debug("Getting task from job " + 
-                  j.getJobID().toStringWOPrefix() + " in queue " + qsi.queueName);
+                        j.getJobID() + " in queue " + qsi.queueName);
               return t;
             }
           }

src/mapred/org/apache/hadoop/mapred/CommitTaskAction.java

@@ -32,6 +32,7 @@ class CommitTaskAction extends TaskTrackerAction {
   
   public CommitTaskAction() {
     super(ActionType.COMMIT_TASK);
+    taskId = new TaskAttemptID();
   }
   
   public CommitTaskAction(TaskAttemptID taskId) {
@@ -48,6 +49,6 @@ class CommitTaskAction extends TaskTrackerAction {
   }
 
   public void readFields(DataInput in) throws IOException {
-    taskId = TaskAttemptID.read(in);
+    taskId.readFields(in);
   }
 }

src/mapred/org/apache/hadoop/mapred/HeartbeatResponse.java

@@ -134,7 +134,8 @@ class HeartbeatResponse implements Writable, Configurable {
     if (size != 0) {
       lastKnownIndexMap = new HashMap<JobID, Integer>(size);
       for (int i = 0; i < size; ++i) {
-        JobID id = JobID.read(in);
+        JobID id = new JobID();
+        id.readFields(in);
         int count = in.readInt();
         lastKnownIndexMap.put(id, count);
       }

src/mapred/org/apache/hadoop/mapred/ID.java

@@ -34,6 +34,7 @@ import org.apache.hadoop.io.WritableComparable;
  * @see TaskAttemptID
  */
 public abstract class ID implements WritableComparable<ID> {
+  protected static final char SEPARATOR = '_';
   protected int id;
 
   /** constructs an ID object from the given int */

src/mapred/org/apache/hadoop/mapred/JVMId.java

@@ -27,7 +27,6 @@ class JVMId extends ID {
   boolean isMap;
   JobID jobId;
   private static final String JVM = "jvm";
-  private static char UNDERSCORE = '_';  
   private static NumberFormat idFormat = NumberFormat.getInstance();
   static {
     idFormat.setGroupingUsed(false);
@@ -44,7 +43,9 @@ class JVMId extends ID {
     this(new JobID(jtIdentifier, jobId), isMap, id);
   }
     
-  private JVMId() { }
+  public JVMId() { 
+    jobId = new JobID();
+  }
   
   public boolean isMapJVM() {
     return isMap;
@@ -73,35 +74,41 @@ class JVMId extends ID {
     if(jobComp == 0) {
       if(this.isMap == that.isMap) {
         return this.id - that.id;
+      } else {
+        return this.isMap ? -1 : 1;
       }
-      else return this.isMap ? -1 : 1;
+    } else {
+      return jobComp;
     }
-    else return jobComp;
   }
   
   @Override
   public String toString() { 
-    StringBuilder builder = new StringBuilder();
-    return builder.append(JVM).append(UNDERSCORE)
-      .append(toStringWOPrefix()).toString();
+    return appendTo(new StringBuilder(JVM)).toString();
   }
 
-  StringBuilder toStringWOPrefix() {
-    StringBuilder builder = new StringBuilder();
-    builder.append(jobId.toStringWOPrefix())
-      .append(isMap ? "_m_" : "_r_");
-    return builder.append(idFormat.format(id));
+  /**
+   * Add the unique id to the given StringBuilder.
+   * @param builder the builder to append to
+   * @return the passed in builder.
+   */
+  protected StringBuilder appendTo(StringBuilder builder) {
+    return jobId.appendTo(builder).
+                 append(SEPARATOR).
+                 append(isMap ? 'm' : 'r').
+                 append(SEPARATOR).
+                 append(idFormat.format(id));
   }
   
   @Override
   public int hashCode() {
-    return toStringWOPrefix().toString().hashCode();
+    return jobId.hashCode() * 11 + id;
   }
   
   @Override
   public void readFields(DataInput in) throws IOException {
     super.readFields(in);
-    this.jobId = JobID.read(in);
+    this.jobId.readFields(in);
     this.isMap = in.readBoolean();
   }
 
@@ -112,12 +119,6 @@ class JVMId extends ID {
     out.writeBoolean(isMap);
   }
   
-  public static JVMId read(DataInput in) throws IOException {
-    JVMId jvmId = new JVMId();
-    jvmId.readFields(in);
-    return jvmId;
-  }
-  
   /** Construct a JVMId object from given string 
    * @return constructed JVMId object or null if the given String is null
    * @throws IllegalArgumentException if the given string is malformed

src/mapred/org/apache/hadoop/mapred/JobID.java

@@ -45,9 +45,8 @@ import org.apache.hadoop.io.Text;
  * @see JobTracker#getStartTime()
  */
 public class JobID extends ID {
-  private static final String JOB = "job";
-  private String jtIdentifier;
-  private static char UNDERSCORE = '_';
+  protected static final String JOB = "job";
+  private Text jtIdentifier = new Text();
   
   private static NumberFormat idFormat = NumberFormat.getInstance();
   static {
@@ -62,13 +61,13 @@ public class JobID extends ID {
    */
   public JobID(String jtIdentifier, int id) {
     super(id);
-    this.jtIdentifier = jtIdentifier;
+    this.jtIdentifier.set(jtIdentifier);
   }
   
-  private JobID() { }
+  public JobID() { }
   
   public String getJtIdentifier() {
-    return jtIdentifier;
+    return jtIdentifier.toString();
   }
   
   @Override
@@ -93,42 +92,47 @@ public class JobID extends ID {
   
   @Override
   public String toString() {
-    StringBuilder builder = new StringBuilder();
-    return builder.append(JOB).append(UNDERSCORE)
-      .append(toStringWOPrefix()).toString();
+    return appendTo(new StringBuilder(JOB)).toString();
   }
-  
-  /** Returns the string representation w/o prefix */
-  StringBuilder toStringWOPrefix() {
-    StringBuilder builder = new StringBuilder();
-    builder.append(jtIdentifier).append(UNDERSCORE)
-    .append(idFormat.format(id)).toString();
+
+  /**
+   * Add the stuff after the "job" prefix to the given builder. This is useful,
+   * because the sub-ids use this substring at the start of their string.
+   * @param builder the builder to append to
+   * @return the builder that was passed in
+   */
+  protected StringBuilder appendTo(StringBuilder builder) {
+    builder.append(SEPARATOR);
+    builder.append(jtIdentifier);
+    builder.append(SEPARATOR);
+    builder.append(idFormat.format(id));
     return builder;
   }
-  
+
   @Override
   public int hashCode() {
-    return toStringWOPrefix().toString().hashCode();
+    return jtIdentifier.hashCode() + id;
   }
   
   @Override
   public void readFields(DataInput in) throws IOException {
     super.readFields(in);
-    this.jtIdentifier = Text.readString(in);
+    jtIdentifier.readFields(in);
   }
 
   @Override
   public void write(DataOutput out) throws IOException {
     super.write(out);
-    Text.writeString(out, jtIdentifier);
+    jtIdentifier.write(out);
   }
   
+  @Deprecated
   public static JobID read(DataInput in) throws IOException {
     JobID jobId = new JobID();
     jobId.readFields(in);
     return jobId;
   }
-  
+
   /** Construct a JobId object from given string 
    * @return constructed JobId object or null if the given String is null
    * @throws IllegalArgumentException if the given string is malformed
@@ -137,7 +141,7 @@ public class JobID extends ID {
     if(str == null)
       return null;
     try {
-      String[] parts = str.split("_");
+      String[] parts = str.split(Character.toString(SEPARATOR));
       if(parts.length == 3) {
         if(parts[0].equals(JOB)) {
           return new JobID(parts[1], Integer.parseInt(parts[2]));
@@ -163,16 +167,23 @@ public class JobID extends ID {
    * @param jobId job number, or null
    * @return a regex pattern matching JobIDs
    */
+  @Deprecated
   public static String getJobIDsPattern(String jtIdentifier, Integer jobId) {
-    StringBuilder builder = new StringBuilder(JOB).append(UNDERSCORE);
+    StringBuilder builder = new StringBuilder(JOB).append(SEPARATOR);
     builder.append(getJobIDsPatternWOPrefix(jtIdentifier, jobId));
     return builder.toString();
   }
   
-  static StringBuilder getJobIDsPatternWOPrefix(String jtIdentifier
-      , Integer jobId) {
-    StringBuilder builder = new StringBuilder()
-      .append(jtIdentifier != null ? jtIdentifier : "[^_]*").append(UNDERSCORE)
+  @Deprecated
+  static StringBuilder getJobIDsPatternWOPrefix(String jtIdentifier,
+                                                Integer jobId) {
+    StringBuilder builder = new StringBuilder();
+    if (jtIdentifier != null) {
+      builder.append(jtIdentifier);
+    } else {
+      builder.append("[^").append(SEPARATOR).append("]*");
+    }
+    builder.append(SEPARATOR)
       .append(jobId != null ? idFormat.format(jobId) : "[0-9]*");
     return builder;
   }

src/mapred/org/apache/hadoop/mapred/JobProfile.java

@@ -43,7 +43,7 @@ public class JobProfile implements Writable {
   }
 
   String user;
-  JobID jobid;
+  final JobID jobid;
   String jobFile;
   String url;
   String name;
@@ -53,6 +53,7 @@ public class JobProfile implements Writable {
    * Construct an empty {@link JobProfile}.
    */
   public JobProfile() {
+    jobid = new JobID();
   }
 
   /**
@@ -166,8 +167,9 @@ public class JobProfile implements Writable {
     Text.writeString(out, name);
     Text.writeString(out, queueName);
   }
+
   public void readFields(DataInput in) throws IOException {
-    this.jobid = JobID.read(in);
+    jobid.readFields(in);
     this.jobFile = Text.readString(in);
     this.url = Text.readString(in);
     this.user = Text.readString(in);

src/mapred/org/apache/hadoop/mapred/JobStatus.java

@@ -48,7 +48,7 @@ public class JobStatus implements Writable, Cloneable {
   public static final int PREP = 4;
   public static final int KILLED = 5;
 
-  private JobID jobid;
+  private final JobID jobid;
   private float mapProgress;
   private float reduceProgress;
   private float cleanupProgress;
@@ -62,6 +62,7 @@ public class JobStatus implements Writable, Cloneable {
   /**
    */
   public JobStatus() {
+    jobid = new JobID();
   }
 
   /**
@@ -287,7 +288,7 @@ public class JobStatus implements Writable, Cloneable {
   }
 
   public synchronized void readFields(DataInput in) throws IOException {
-    this.jobid = JobID.read(in);
+    jobid.readFields(in);
     this.setupProgress = in.readFloat();
     this.mapProgress = in.readFloat();
     this.reduceProgress = in.readFloat();

src/mapred/org/apache/hadoop/mapred/KillJobAction.java

@@ -30,10 +30,11 @@ import java.io.IOException;
  * 
  */
 class KillJobAction extends TaskTrackerAction {
-  JobID jobId;
+  final JobID jobId;
 
   public KillJobAction() {
     super(ActionType.KILL_JOB);
+    jobId = new JobID();
   }
 
   public KillJobAction(JobID jobId) {
@@ -52,7 +53,7 @@ class KillJobAction extends TaskTrackerAction {
 
   @Override
   public void readFields(DataInput in) throws IOException {
-    jobId = JobID.read(in);
+    jobId.readFields(in);
   }
 
 }

src/mapred/org/apache/hadoop/mapred/KillTaskAction.java

@@ -29,10 +29,11 @@ import java.io.IOException;
  * 
  */
 class KillTaskAction extends TaskTrackerAction {
-  TaskAttemptID taskId;
+  final TaskAttemptID taskId;
   
   public KillTaskAction() {
     super(ActionType.KILL_TASK);
+    taskId = new TaskAttemptID();
   }
   
   public KillTaskAction(TaskAttemptID taskId) {
@@ -51,6 +52,6 @@ class KillTaskAction extends TaskTrackerAction {
 
   @Override
   public void readFields(DataInput in) throws IOException {
-    taskId = TaskAttemptID.read(in);
+    taskId.readFields(in);
   }
 }

src/mapred/org/apache/hadoop/mapred/ReduceTaskStatus.java

@@ -131,7 +131,9 @@ class ReduceTaskStatus extends TaskStatus {
     int noFailedFetchTasks = in.readInt();
     failedFetchTasks = new ArrayList<TaskAttemptID>(noFailedFetchTasks);
     for (int i=0; i < noFailedFetchTasks; ++i) {
-      failedFetchTasks.add(TaskAttemptID.read(in));
+      TaskAttemptID id = new TaskAttemptID();
+      id.readFields(in);
+      failedFetchTasks.add(id);
     }
   }
 

src/mapred/org/apache/hadoop/mapred/Task.java

@@ -104,7 +104,7 @@ abstract class Task implements Writable, Configurable {
   ////////////////////////////////////////////
 
   private String jobFile;                         // job configuration file
-  private TaskAttemptID taskId;                          // unique, includes job id
+  private final TaskAttemptID taskId;             // unique, includes job id
   private int partition;                          // id within job
   TaskStatus taskStatus;                          // current status of the task
   protected boolean cleanupJob = false;
@@ -135,6 +135,7 @@ abstract class Task implements Writable, Configurable {
 
   public Task() {
     taskStatus = TaskStatus.createTaskStatus(isMapTask());
+    taskId = new TaskAttemptID();
   }
 
   public Task(String jobFile, TaskAttemptID taskId, int partition) {
@@ -262,7 +263,7 @@ abstract class Task implements Writable, Configurable {
   }
   public void readFields(DataInput in) throws IOException {
     jobFile = Text.readString(in);
-    taskId = TaskAttemptID.read(in);
+    taskId.readFields(in);
     partition = in.readInt();
     taskStatus.readFields(in);
     this.mapOutputFile.setJobId(taskId.getJobID()); 

src/mapred/org/apache/hadoop/mapred/TaskAttemptID.java

@@ -44,8 +44,7 @@ import java.io.IOException;
  */
 public class TaskAttemptID extends ID {
   private static final String ATTEMPT = "attempt";
-  private TaskID taskId;
-  private static final char UNDERSCORE = '_';
+  private final TaskID taskId;
   
   /**
    * Constructs a TaskAttemptID object from given {@link TaskID}.  
@@ -73,7 +72,9 @@ public class TaskAttemptID extends ID {
     this(new TaskID(jtIdentifier, jobId, isMap, taskId), id);
   }
   
-  private TaskAttemptID() { }
+  public TaskAttemptID() { 
+    taskId = new TaskID();
+  }
   
   /** Returns the {@link JobID} object that this task attempt belongs to */
   public JobID getJobID() {
@@ -114,26 +115,27 @@ public class TaskAttemptID extends ID {
   }
   @Override
   public String toString() { 
-    StringBuilder builder = new StringBuilder();
-    return builder.append(ATTEMPT).append(UNDERSCORE)
-      .append(toStringWOPrefix()).toString();
+    return appendTo(new StringBuilder(ATTEMPT)).toString();
   }
 
-  StringBuilder toStringWOPrefix() {
-    StringBuilder builder = new StringBuilder();
-    return builder.append(taskId.toStringWOPrefix())
-                  .append(UNDERSCORE).append(id);
+  /**
+   * Add the unique string to the StringBuilder
+   * @param builder the builder to append ot
+   * @return the builder that was passed in.
+   */
+  protected StringBuilder appendTo(StringBuilder builder) {
+    return taskId.appendTo(builder).append(SEPARATOR).append(id);
   }
   
   @Override
   public int hashCode() {
-    return toStringWOPrefix().toString().hashCode();
+    return taskId.hashCode() * 5 + id;
   }
   
   @Override
   public void readFields(DataInput in) throws IOException {
     super.readFields(in);
-    this.taskId = TaskID.read(in);
+    taskId.readFields(in);
   }
 
   @Override
@@ -142,6 +144,7 @@ public class TaskAttemptID extends ID {
     taskId.write(out);
   }
   
+  @Deprecated
   public static TaskAttemptID read(DataInput in) throws IOException {
     TaskAttemptID taskId = new TaskAttemptID();
     taskId.readFields(in);
@@ -152,11 +155,12 @@ public class TaskAttemptID extends ID {
    * @return constructed TaskAttemptID object or null if the given String is null
    * @throws IllegalArgumentException if the given string is malformed
    */
-  public static TaskAttemptID forName(String str) throws IllegalArgumentException {
+  public static TaskAttemptID forName(String str
+                                      ) throws IllegalArgumentException {
     if(str == null)
       return null;
     try {
-      String[] parts = str.split("_");
+      String[] parts = str.split(Character.toString(SEPARATOR));
       if(parts.length == 6) {
         if(parts[0].equals(ATTEMPT)) {
           boolean isMap = false;
@@ -167,7 +171,8 @@ public class TaskAttemptID extends ID {
               isMap, Integer.parseInt(parts[4]), Integer.parseInt(parts[5]));
         }
       }
-    }catch (Exception ex) {//fall below
+    } catch (Exception ex) {
+      //fall below
     }
     throw new IllegalArgumentException("TaskAttemptId string : " + str 
         + " is not properly formed");
@@ -191,20 +196,22 @@ public class TaskAttemptID extends ID {
    * @param attemptId the task attempt number, or null
    * @return a regex pattern matching TaskAttemptIDs
    */
+  @Deprecated
   public static String getTaskAttemptIDsPattern(String jtIdentifier,
       Integer jobId, Boolean isMap, Integer taskId, Integer attemptId) {
-    StringBuilder builder = new StringBuilder(ATTEMPT).append(UNDERSCORE);
+    StringBuilder builder = new StringBuilder(ATTEMPT).append(SEPARATOR);
     builder.append(getTaskAttemptIDsPatternWOPrefix(jtIdentifier, jobId,
         isMap, taskId, attemptId));
     return builder.toString();
   }
   
+  @Deprecated
   static StringBuilder getTaskAttemptIDsPatternWOPrefix(String jtIdentifier
       , Integer jobId, Boolean isMap, Integer taskId, Integer attemptId) {
     StringBuilder builder = new StringBuilder();
     builder.append(TaskID.getTaskIDsPatternWOPrefix(jtIdentifier
         , jobId, isMap, taskId))
-        .append(UNDERSCORE)
+        .append(SEPARATOR)
         .append(attemptId != null ? attemptId : "[0-9]*");
     return builder;
   }

src/mapred/org/apache/hadoop/mapred/TaskCompletionEvent.java

@@ -45,7 +45,10 @@ public class TaskCompletionEvent implements Writable{
    * Default constructor for Writable.
    *
    */
-  public TaskCompletionEvent(){}
+  public TaskCompletionEvent(){
+    taskId = new TaskAttemptID();
+  }
+
   /**
    * Constructor. eventId should be created externally and incremented
    * per event for each job. 
@@ -220,12 +223,12 @@ public class TaskCompletionEvent implements Writable{
   }
   
   public void readFields(DataInput in) throws IOException {
-    this.taskId = TaskAttemptID.read(in); 
-    this.idWithinJob = WritableUtils.readVInt(in);
-    this.isMap = in.readBoolean();
-    this.status = WritableUtils.readEnum(in, Status.class);
-    this.taskTrackerHttp = WritableUtils.readString(in);
-    this.taskRunTime = WritableUtils.readVInt(in);
-    this.eventId = WritableUtils.readVInt(in);
+    taskId.readFields(in); 
+    idWithinJob = WritableUtils.readVInt(in);
+    isMap = in.readBoolean();
+    status = WritableUtils.readEnum(in, Status.class);
+    taskTrackerHttp = WritableUtils.readString(in);
+    taskRunTime = WritableUtils.readVInt(in);
+    eventId = WritableUtils.readVInt(in);
   }
 }

src/mapred/org/apache/hadoop/mapred/TaskID.java

@@ -47,8 +47,7 @@ import java.text.NumberFormat;
  */
 public class TaskID extends ID {
   private static final String TASK = "task";
-  private static char UNDERSCORE = '_';  
-  private static NumberFormat idFormat = NumberFormat.getInstance();
+  private static final NumberFormat idFormat = NumberFormat.getInstance();
   static {
     idFormat.setGroupingUsed(false);
     idFormat.setMinimumIntegerDigits(6);
@@ -83,7 +82,9 @@ public class TaskID extends ID {
     this(new JobID(jtIdentifier, jobId), isMap, id);
   }
   
-  private TaskID() { }
+  public TaskID() { 
+    jobId = new JobID();
+  }
   
   /** Returns the {@link JobID} object that this tip belongs to */
   public JobID getJobID() {
@@ -110,39 +111,45 @@ public class TaskID extends ID {
   public int compareTo(ID o) {
     TaskID that = (TaskID)o;
     int jobComp = this.jobId.compareTo(that.jobId);
-    if(jobComp == 0) {
-      if(this.isMap == that.isMap) {
+    if (jobComp == 0) {
+      if (this.isMap == that.isMap) {
         return this.id - that.id;
+      } else {
+        return this.isMap ? -1 : 1;
       }
-      else return this.isMap ? -1 : 1;
+    } else {
+      return jobComp;
     }
-    else return jobComp;
   }
   
   @Override
   public String toString() { 
-    StringBuilder builder = new StringBuilder();
-    return builder.append(TASK).append(UNDERSCORE)
-      .append(toStringWOPrefix()).toString();
+    return appendTo(new StringBuilder(TASK)).toString();
   }
 
-  StringBuilder toStringWOPrefix() {
-    StringBuilder builder = new StringBuilder();
-    builder.append(jobId.toStringWOPrefix())
-      .append(isMap ? "_m_" : "_r_");
-    return builder.append(idFormat.format(id));
+  /**
+   * Add the unique string to the given builder.
+   * @param builder the builder to append to
+   * @return the builder that was passed in
+   */
+  protected StringBuilder appendTo(StringBuilder builder) {
+    return jobId.appendTo(builder).
+                 append(SEPARATOR).
+                 append(isMap ? 'm' : 'r').
+                 append(SEPARATOR).
+                 append(idFormat.format(id));
   }
   
   @Override
   public int hashCode() {
-    return toStringWOPrefix().toString().hashCode();
+    return jobId.hashCode() * 524287 + id;
   }
   
   @Override
   public void readFields(DataInput in) throws IOException {
     super.readFields(in);
-    this.jobId = JobID.read(in);
-    this.isMap = in.readBoolean();
+    jobId.readFields(in);
+    isMap = in.readBoolean();
   }
 
   @Override
@@ -151,7 +158,8 @@ public class TaskID extends ID {
     jobId.write(out);
     out.writeBoolean(isMap);
   }
-  
+
+  @Deprecated
   public static TaskID read(DataInput in) throws IOException {
     TaskID tipId = new TaskID();
     tipId.readFields(in);
@@ -167,7 +175,7 @@ public class TaskID extends ID {
     if(str == null)
       return null;
     try {
-      String[] parts = str.split("_");
+      String[] parts = str.split(Character.toString(SEPARATOR));
       if(parts.length == 5) {
         if(parts[0].equals(TASK)) {
           boolean isMap = false;
@@ -200,19 +208,21 @@ public class TaskID extends ID {
    * @param taskId taskId number, or null
    * @return a regex pattern matching TaskIDs
    */
+  @Deprecated
   public static String getTaskIDsPattern(String jtIdentifier, Integer jobId
       , Boolean isMap, Integer taskId) {
-    StringBuilder builder = new StringBuilder(TASK).append(UNDERSCORE)
+    StringBuilder builder = new StringBuilder(TASK).append(SEPARATOR)
       .append(getTaskIDsPatternWOPrefix(jtIdentifier, jobId, isMap, taskId));
     return builder.toString();
   }
   
+  @Deprecated
   static StringBuilder getTaskIDsPatternWOPrefix(String jtIdentifier
       , Integer jobId, Boolean isMap, Integer taskId) {
     StringBuilder builder = new StringBuilder();
     builder.append(JobID.getJobIDsPatternWOPrefix(jtIdentifier, jobId))
-      .append(UNDERSCORE)
-      .append(isMap != null ? (isMap ? "m" : "r") : "(m|r)").append(UNDERSCORE)
+      .append(SEPARATOR)
+      .append(isMap != null ? (isMap ? "m" : "r") : "(m|r)").append(SEPARATOR)
       .append(taskId != null ? idFormat.format(taskId) : "[0-9]*");
     return builder;
   }

src/mapred/org/apache/hadoop/mapred/TaskReport.java

@@ -28,7 +28,7 @@ import org.apache.hadoop.io.WritableUtils;
 
 /** A report on the state of a task. */
 public class TaskReport implements Writable {
-  private TaskID taskid;
+  private final TaskID taskid;
   private float progress;
   private String state;
   private String[] diagnostics;
@@ -36,7 +36,9 @@ public class TaskReport implements Writable {
   private long finishTime; 
   private Counters counters;
 
-  public TaskReport() {}
+  public TaskReport() {
+    taskid = new TaskID();
+  }
 
   TaskReport(TaskID taskid, float progress, String state,
              String[] diagnostics, long startTime, long finishTime,
@@ -133,11 +135,11 @@ public class TaskReport implements Writable {
   }
 
   public void readFields(DataInput in) throws IOException {
-    this.taskid = TaskID.read(in);
-    this.progress = in.readFloat();
-    this.state = Text.readString(in);
-    this.startTime = in.readLong(); 
-    this.finishTime = in.readLong();
+    taskid.readFields(in);
+    progress = in.readFloat();
+    state = Text.readString(in);
+    startTime = in.readLong(); 
+    finishTime = in.readLong();
     
     diagnostics = WritableUtils.readStringArray(in);
     counters = new Counters();

src/mapred/org/apache/hadoop/mapred/TaskStatus.java

@@ -43,7 +43,7 @@ abstract class TaskStatus implements Writable, Cloneable {
   public static enum State {RUNNING, SUCCEEDED, FAILED, UNASSIGNED, KILLED, 
                             COMMIT_PENDING}
     
-  private TaskAttemptID taskid;
+  private final TaskAttemptID taskid;
   private float progress;
   private volatile State runState;
   private String diagnosticInfo;
@@ -59,7 +59,9 @@ abstract class TaskStatus implements Writable, Cloneable {
   private boolean includeCounters;
   private SortedRanges.Range nextRecordRange = new SortedRanges.Range();
 
-  public TaskStatus() {}
+  public TaskStatus() {
+    taskid = new TaskAttemptID();
+  }
 
   public TaskStatus(TaskAttemptID taskid, float progress,
                     State runState, String diagnosticInfo,
@@ -340,7 +342,7 @@ abstract class TaskStatus implements Writable, Cloneable {
   }
 
   public void readFields(DataInput in) throws IOException {
-    this.taskid = TaskAttemptID.read(in);
+    this.taskid.readFields(in);
     this.progress = in.readFloat();
     this.runState = WritableUtils.readEnum(in, State.class);
     this.diagnosticInfo = Text.readString(in);