From 8360d98d6aafae7e97cdd634bc01bfc52bf6352b Mon Sep 17 00:00:00 2001 From: Jason Lowe Date: Fri, 25 Mar 2016 20:04:32 +0000 Subject: [PATCH] MAPREDUCE-6535. TaskID default constructor results in NPE on toString(). Contributed by Daniel Templeton (cherry picked from commit 3f622a143c5fb15fee7e5dded99e4a4136f19810) --- .../org/apache/hadoop/mapreduce/TaskID.java | 57 ++- .../apache/hadoop/mapreduce/TestTaskID.java | 461 ++++++++++++++++++ 2 files changed, 508 insertions(+), 10 deletions(-) create mode 100644 hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapreduce/TestTaskID.java diff --git a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/TaskID.java b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/TaskID.java index b9817dd16e3..3ddfbe97c84 100644 --- a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/TaskID.java +++ b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/TaskID.java @@ -63,6 +63,7 @@ public class TaskID extends org.apache.hadoop.mapred.ID { public static final String TASK_ID_REGEX = TASK + "_(\\d+)_(\\d+)_" + CharTaskTypeMaps.allTaskTypes + "_(\\d+)"; public static final Pattern taskIdPattern = Pattern.compile(TASK_ID_REGEX); + static { idFormat.setGroupingUsed(false); idFormat.setMinimumIntegerDigits(6); @@ -72,7 +73,8 @@ public class TaskID extends org.apache.hadoop.mapred.ID { private TaskType type; /** - * Constructs a TaskID object from given {@link JobID}. + * Constructs a TaskID object from given {@link JobID}. + * * @param jobId JobID that this tip belongs to * @param type the {@link TaskType} of the task * @param id the tip number @@ -88,6 +90,7 @@ public TaskID(JobID jobId, TaskType type, int id) { /** * Constructs a TaskInProgressId object from given parts. + * * @param jtIdentifier jobTracker identifier * @param jobId job number * @param type the TaskType @@ -99,6 +102,7 @@ public TaskID(String jtIdentifier, int jobId, TaskType type, int id) { /** * Constructs a TaskID object from given {@link JobID}. + * * @param jobId JobID that this tip belongs to * @param isMap whether the tip is a map * @param id the tip number @@ -110,6 +114,7 @@ public TaskID(JobID jobId, boolean isMap, int id) { /** * Constructs a TaskInProgressId object from given parts. + * * @param jtIdentifier jobTracker identifier * @param jobId job number * @param isMap whether the tip is a map @@ -120,23 +125,37 @@ public TaskID(String jtIdentifier, int jobId, boolean isMap, int id) { this(new JobID(jtIdentifier, jobId), isMap, id); } + /** + * Default constructor for Writable. Sets the task type to + * {@link TaskType#REDUCE}, the ID to 0, and the job ID to an empty job ID. + */ public TaskID() { - jobId = new JobID(); + this(new JobID(), TaskType.REDUCE, 0); } - /** Returns the {@link JobID} object that this tip belongs to */ + /** + * Returns the {@link JobID} object that this tip belongs to. + * + * @return the JobID object + */ public JobID getJobID() { return jobId; } - /**Returns whether this TaskID is a map ID */ + /** + * Returns whether this TaskID is a map ID. + * + * @return whether this TaskID is a map ID + */ @Deprecated public boolean isMap() { return type == TaskType.MAP; } /** - * Get the type of the task + * Get the type of the task. + * + * @return the type of the task */ public TaskType getTaskType() { return type; @@ -151,8 +170,14 @@ public boolean equals(Object o) { return this.type == that.type && this.jobId.equals(that.jobId); } - /**Compare TaskInProgressIds by first jobIds, then by tip numbers. Reduces are - * defined as greater then maps.*/ + /** + * Compare TaskInProgressIds by first jobIds, then by tip numbers. + * Reducers are defined as greater than mappers. + * + * @param o the TaskID against which to compare + * @return 0 if equal, positive if this TaskID is greater, and negative if + * this TaskID is less + */ @Override public int compareTo(ID o) { TaskID that = (TaskID)o; @@ -174,6 +199,7 @@ public String toString() { /** * Add the unique string to the given builder. + * * @param builder the builder to append to * @return the builder that was passed in */ @@ -204,7 +230,10 @@ public void write(DataOutput out) throws IOException { WritableUtils.writeEnum(out, type); } - /** Construct a TaskID object from given string + /** + * Construct a TaskID object from given string. + * + * @param str the target string * @return constructed TaskID object or null if the given String is null * @throws IllegalArgumentException if the given string is malformed */ @@ -224,7 +253,8 @@ public static TaskID forName(String str) throw new IllegalArgumentException(exceptionMsg); } /** - * Gets the character representing the {@link TaskType} + * Gets the character representing the {@link TaskType}. + * * @param type the TaskType * @return the character */ @@ -232,7 +262,8 @@ public static char getRepresentingCharacter(TaskType type) { return CharTaskTypeMaps.getRepresentingCharacter(type); } /** - * Gets the {@link TaskType} corresponding to the character + * Gets the {@link TaskType} corresponding to the character. + * * @param c the character * @return the TaskType */ @@ -240,6 +271,12 @@ public static TaskType getTaskType(char c) { return CharTaskTypeMaps.getTaskType(c); } + /** + * Returns a string of characters describing all possible {@link TaskType} + * values + * + * @return a string of all task type characters + */ public static String getAllTaskTypes() { return CharTaskTypeMaps.allTaskTypes; } diff --git a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapreduce/TestTaskID.java b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapreduce/TestTaskID.java new file mode 100644 index 00000000000..5531074dce8 --- /dev/null +++ b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapreduce/TestTaskID.java @@ -0,0 +1,461 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.mapreduce; + +import org.apache.hadoop.io.DataInputByteBuffer; +import org.apache.hadoop.io.DataOutputByteBuffer; +import org.apache.hadoop.io.WritableUtils; +import org.junit.Test; +import static org.junit.Assert.*; + +/** + * Test the {@link TaskID} class. + */ +public class TestTaskID { + /** + * Test of getJobID method, of class TaskID. + */ + @Test + public void testGetJobID() { + JobID jobId = new JobID("1234", 0); + TaskID taskId = new TaskID(jobId, TaskType.MAP, 0); + + assertSame("TaskID did not store the JobID correctly", + jobId, taskId.getJobID()); + + taskId = new TaskID(); + + assertEquals("Job ID was set unexpectedly in default contsructor", + "", taskId.getJobID().getJtIdentifier()); + } + + /** + * Test of isMap method, of class TaskID. + */ + @Test + public void testIsMap() { + JobID jobId = new JobID("1234", 0); + + for (TaskType type : TaskType.values()) { + TaskID taskId = new TaskID(jobId, type, 0); + + if (type == TaskType.MAP) { + assertTrue("TaskID for map task did not correctly identify itself " + + "as a map task", taskId.isMap()); + } else { + assertFalse("TaskID for " + type + " task incorrectly identified " + + "itself as a map task", taskId.isMap()); + } + } + + TaskID taskId = new TaskID(); + + assertFalse("TaskID of default type incorrectly identified itself as a " + + "map task", taskId.isMap()); + } + + /** + * Test of getTaskType method, of class TaskID. + */ + @Test + public void testGetTaskType_0args() { + JobID jobId = new JobID("1234", 0); + + for (TaskType type : TaskType.values()) { + TaskID taskId = new TaskID(jobId, type, 0); + + assertEquals("TaskID incorrectly reported its type", + type, taskId.getTaskType()); + } + + TaskID taskId = new TaskID(); + + assertEquals("TaskID of default type incorrectly reported its type", + TaskType.REDUCE, taskId.getTaskType()); + } + + /** + * Test of equals method, of class TaskID. + */ + @Test + public void testEquals() { + JobID jobId1 = new JobID("1234", 1); + JobID jobId2 = new JobID("2345", 2); + TaskID taskId1 = new TaskID(jobId1, TaskType.MAP, 0); + TaskID taskId2 = new TaskID(jobId1, TaskType.MAP, 0); + + assertTrue("The equals() method reported two equal task IDs were not equal", + taskId1.equals(taskId2)); + + taskId2 = new TaskID(jobId2, TaskType.MAP, 0); + + assertFalse("The equals() method reported two task IDs with different " + + "job IDs were equal", taskId1.equals(taskId2)); + + taskId2 = new TaskID(jobId1, TaskType.MAP, 1); + + assertFalse("The equals() method reported two task IDs with different IDs " + + "were equal", taskId1.equals(taskId2)); + + TaskType[] types = TaskType.values(); + + for (int i = 0; i < types.length; i++) { + for (int j = 0; j < types.length; j++) { + taskId1 = new TaskID(jobId1, types[i], 0); + taskId2 = new TaskID(jobId1, types[j], 0); + + if (i == j) { + assertTrue("The equals() method reported two equal task IDs were not " + + "equal", taskId1.equals(taskId2)); + } else { + assertFalse("The equals() method reported two task IDs with " + + "different types were equal", taskId1.equals(taskId2)); + } + } + } + + assertFalse("The equals() method matched against a JobID object", + taskId1.equals(jobId1)); + + assertFalse("The equals() method matched against a null object", + taskId1.equals(null)); + } + + /** + * Test of compareTo method, of class TaskID. + */ + @Test + public void testCompareTo() { + JobID jobId = new JobID("1234", 1); + TaskID taskId1 = new TaskID(jobId, TaskType.REDUCE, 0); + TaskID taskId2 = new TaskID(jobId, TaskType.REDUCE, 0); + + assertEquals("The compareTo() method returned non-zero for two equal " + + "task IDs", 0, taskId1.compareTo(taskId2)); + + taskId2 = new TaskID(jobId, TaskType.MAP, 1); + + assertTrue("The compareTo() method did not weigh task type more than task " + + "ID", taskId1.compareTo(taskId2) > 0); + + TaskType[] types = TaskType.values(); + + for (int i = 0; i < types.length; i++) { + for (int j = 0; j < types.length; j++) { + taskId1 = new TaskID(jobId, types[i], 0); + taskId2 = new TaskID(jobId, types[j], 0); + + if (i == j) { + assertEquals("The compareTo() method returned non-zero for two equal " + + "task IDs", 0, taskId1.compareTo(taskId2)); + } else if (i < j) { + assertTrue("The compareTo() method did not order " + types[i] + + " before " + types[j], taskId1.compareTo(taskId2) < 0); + } else { + assertTrue("The compareTo() method did not order " + types[i] + + " after " + types[j], taskId1.compareTo(taskId2) > 0); + } + } + } + + try { + taskId1.compareTo(jobId); + fail("The compareTo() method allowed comparison to a JobID object"); + } catch (ClassCastException ex) { + // Expected + } + + try { + taskId1.compareTo(null); + fail("The compareTo() method allowed comparison to a null object"); + } catch (NullPointerException ex) { + // Expected + } + } + + /** + * Test of toString method, of class TaskID. + */ + @Test + public void testToString() { + JobID jobId = new JobID("1234", 1); + + for (TaskType type : TaskType.values()) { + TaskID taskId = new TaskID(jobId, type, 0); + String str = String.format("task_1234_0001_%c_000000", + TaskID.getRepresentingCharacter(type)); + + assertEquals("The toString() method returned the wrong value", + str, taskId.toString()); + } + } + + /** + * Test of appendTo method, of class TaskID. + */ + @Test + public void testAppendTo() { + JobID jobId = new JobID("1234", 1); + StringBuilder builder = new StringBuilder(); + + for (TaskType type : TaskType.values()) { + builder.setLength(0); + TaskID taskId = new TaskID(jobId, type, 0); + String str = String.format("_1234_0001_%c_000000", + TaskID.getRepresentingCharacter(type)); + + assertEquals("The appendTo() method appended the wrong value", + str, taskId.appendTo(builder).toString()); + } + + try { + new TaskID().appendTo(null); + fail("The appendTo() method allowed a null builder"); + } catch (NullPointerException ex) { + // Expected + } + } + + /** + * Test of hashCode method, of class TaskID. + */ + @Test + public void testHashCode() { + TaskType[] types = TaskType.values(); + + for (int i = 0; i < types.length; i++) { + JobID jobId = new JobID("1234" + i, i); + TaskID taskId1 = new TaskID(jobId, types[i], i); + TaskID taskId2 = new TaskID(jobId, types[i], i); + + assertTrue("The hashcode() method gave unequal hash codes for two equal " + + "task IDs", taskId1.hashCode() == taskId2.hashCode()); + } + } + + /** + * Test of readFields method, of class TaskID. + */ + @Test + public void testReadFields() throws Exception { + DataOutputByteBuffer out = new DataOutputByteBuffer(); + + out.writeInt(0); + out.writeInt(1); + WritableUtils.writeVInt(out, 4); + out.write(new byte[] { 0x31, 0x32, 0x33, 0x34}); + WritableUtils.writeEnum(out, TaskType.REDUCE); + + DataInputByteBuffer in = new DataInputByteBuffer(); + + in.reset(out.getData()); + + TaskID instance = new TaskID(); + + instance.readFields(in); + + assertEquals("The readFields() method did not produce the expected task ID", + "task_1234_0001_r_000000", instance.toString()); + } + + /** + * Test of write method, of class TaskID. + */ + @Test + public void testWrite() throws Exception { + JobID jobId = new JobID("1234", 1); + TaskID taskId = new TaskID(jobId, TaskType.JOB_SETUP, 0); + DataOutputByteBuffer out = new DataOutputByteBuffer(); + + taskId.write(out); + + DataInputByteBuffer in = new DataInputByteBuffer(); + byte[] buffer = new byte[4]; + + in.reset(out.getData()); + + assertEquals("The write() method did not write the expected task ID", + 0, in.readInt()); + assertEquals("The write() method did not write the expected job ID", + 1, in.readInt()); + assertEquals("The write() method did not write the expected job " + + "identifier length", 4, WritableUtils.readVInt(in)); + in.readFully(buffer, 0, 4); + assertEquals("The write() method did not write the expected job " + + "identifier length", "1234", new String(buffer)); + assertEquals("The write() method did not write the expected task type", + TaskType.JOB_SETUP, WritableUtils.readEnum(in, TaskType.class)); + } + + /** + * Test of forName method, of class TaskID. + */ + @Test + public void testForName() { + assertEquals("The forName() method did not parse the task ID string " + + "correctly", "task_1_0001_m_000000", + TaskID.forName("task_1_0001_m_000").toString()); + assertEquals("The forName() method did not parse the task ID string " + + "correctly", "task_23_0002_r_000001", + TaskID.forName("task_23_0002_r_0001").toString()); + assertEquals("The forName() method did not parse the task ID string " + + "correctly", "task_345_0003_s_000002", + TaskID.forName("task_345_0003_s_00002").toString()); + assertEquals("The forName() method did not parse the task ID string " + + "correctly", "task_6789_0004_c_000003", + TaskID.forName("task_6789_0004_c_000003").toString()); + assertEquals("The forName() method did not parse the task ID string " + + "correctly", "task_12345_0005_t_4000000", + TaskID.forName("task_12345_0005_t_4000000").toString()); + + try { + TaskID.forName("tisk_12345_0005_t_4000000"); + fail("The forName() method parsed an invalid job ID: " + + "tisk_12345_0005_t_4000000"); + } catch (IllegalArgumentException ex) { + // Expected + } + + try { + TaskID.forName("tisk_12345_0005_t_4000000"); + fail("The forName() method parsed an invalid job ID: " + + "tisk_12345_0005_t_4000000"); + } catch (IllegalArgumentException ex) { + // Expected + } + + try { + TaskID.forName("task_abc_0005_t_4000000"); + fail("The forName() method parsed an invalid job ID: " + + "task_abc_0005_t_4000000"); + } catch (IllegalArgumentException ex) { + // Expected + } + + try { + TaskID.forName("task_12345_xyz_t_4000000"); + fail("The forName() method parsed an invalid job ID: " + + "task_12345_xyz_t_4000000"); + } catch (IllegalArgumentException ex) { + // Expected + } + + try { + TaskID.forName("task_12345_0005_x_4000000"); + fail("The forName() method parsed an invalid job ID: " + + "task_12345_0005_x_4000000"); + } catch (IllegalArgumentException ex) { + // Expected + } + + try { + TaskID.forName("task_12345_0005_t_jkl"); + fail("The forName() method parsed an invalid job ID: " + + "task_12345_0005_t_jkl"); + } catch (IllegalArgumentException ex) { + // Expected + } + + try { + TaskID.forName("task_12345_0005_t"); + fail("The forName() method parsed an invalid job ID: " + + "task_12345_0005_t"); + } catch (IllegalArgumentException ex) { + // Expected + } + + try { + TaskID.forName("task_12345_0005_4000000"); + fail("The forName() method parsed an invalid job ID: " + + "task_12345_0005_4000000"); + } catch (IllegalArgumentException ex) { + // Expected + } + + try { + TaskID.forName("task_12345_t_4000000"); + fail("The forName() method parsed an invalid job ID: " + + "task_12345_t_4000000"); + } catch (IllegalArgumentException ex) { + // Expected + } + + try { + TaskID.forName("12345_0005_t_4000000"); + fail("The forName() method parsed an invalid job ID: " + + "12345_0005_t_4000000"); + } catch (IllegalArgumentException ex) { + // Expected + } + } + + /** + * Test of getRepresentingCharacter method, of class TaskID. + */ + @Test + public void testGetRepresentingCharacter() { + assertEquals("The getRepresentingCharacter() method did not return the " + + "expected character", 'm', + TaskID.getRepresentingCharacter(TaskType.MAP)); + assertEquals("The getRepresentingCharacter() method did not return the " + + "expected character", 'r', + TaskID.getRepresentingCharacter(TaskType.REDUCE)); + assertEquals("The getRepresentingCharacter() method did not return the " + + "expected character", 's', + TaskID.getRepresentingCharacter(TaskType.JOB_SETUP)); + assertEquals("The getRepresentingCharacter() method did not return the " + + "expected character", 'c', + TaskID.getRepresentingCharacter(TaskType.JOB_CLEANUP)); + assertEquals("The getRepresentingCharacter() method did not return the " + + "expected character", 't', + TaskID.getRepresentingCharacter(TaskType.TASK_CLEANUP)); + } + + /** + * Test of getTaskType method, of class TaskID. + */ + @Test + public void testGetTaskType_char() { + assertEquals("The getTaskType() method did not return the expected type", + TaskType.MAP, + TaskID.getTaskType('m')); + assertEquals("The getTaskType() method did not return the expected type", + TaskType.REDUCE, + TaskID.getTaskType('r')); + assertEquals("The getTaskType() method did not return the expected type", + TaskType.JOB_SETUP, + TaskID.getTaskType('s')); + assertEquals("The getTaskType() method did not return the expected type", + TaskType.JOB_CLEANUP, + TaskID.getTaskType('c')); + assertEquals("The getTaskType() method did not return the expected type", + TaskType.TASK_CLEANUP, + TaskID.getTaskType('t')); + assertNull("The getTaskType() method did not return null for an unknown " + + "type", TaskID.getTaskType('x')); + } + + /** + * Test of getAllTaskTypes method, of class TaskID. + */ + @Test + public void testGetAllTaskTypes() { + assertEquals("The getAllTaskTypes method did not return the expected " + + "string", "(m|r|s|c|t)", TaskID.getAllTaskTypes()); + } +}