ValuesIterator.next() doesn't return a new object, thus failing many equals()
tests.
------------------------------------------------------------------------------------
Key: HADOOP-3522
URL: https://issues.apache.org/jira/browse/HADOOP-3522
Project: Hadoop Core
Issue Type: Bug
Affects Versions: 0.17.0
Reporter: Spyros Blanas
h2.Problem
The ValuesIterator.next() doesn't return a new object representing the next
element.
It is expected that this is the case. Example from java.lang.String source code:
{noformat}
public boolean equals(Object anObject) {
if (this == anObject) {
return true;
}
if (anObject instanceof String) {
// custom code here
}
}
{noformat}
Changing the private fields of the object and returning it from next() will
make this object fail the equals() test, because this==anObject will always be
true.
h2. What is affected
The reduce() method presents (a subclass of) ValuesIterator to the user. So
every user-defined class can be affected.
Manifestations of this bug in 0.17.0:
* The Text class checks for equality similarly to String.equals(), which is
shown above.
* The contrib/data_join breaks because it stores tags in a Map. The behavior of
the next() method makes Object.equals() be true for all tags.
h2. JUnit test
Patch against hadoop-0.17.0:
{noformat}
Index: src/test/org/apache/hadoop/mapred/TestReduceTask.java
===================================================================
--- src/test/org/apache/hadoop/mapred/TestReduceTask.java (revision
665935)
+++ src/test/org/apache/hadoop/mapred/TestReduceTask.java (working copy)
@@ -122,4 +122,52 @@
runValueIterator(tmpDir, testCase, conf);
}
}
+
+ public void runValueIterator2(Path tmpDir, Pair[] vals,
+ Configuration conf) throws IOException {
+ FileSystem fs = tmpDir.getFileSystem(conf);
+ Path path = new Path(tmpDir, "data.in");
+ SequenceFile.Writer writer = new SequenceFile.Writer(fs, conf, path,
+ Text.class,
+ Text.class);
+ for(Pair p: vals) {
+ writer.append(new Text(p.key), new Text(p.value));
+ }
+ writer.close();
+ SequenceFile.Sorter sorter = new SequenceFile.Sorter(fs, Text.class,
+ Text.class, conf);
+ SequenceFile.Sorter.RawKeyValueIterator rawItr =
+ sorter.merge(new Path[]{path}, false, tmpDir);
+ @SuppressWarnings("unchecked") // WritableComparators are not generic
+ ReduceTask.ValuesIterator<Text,Text> valItr =
+ new ReduceTask.ValuesIterator<Text,Text>(rawItr,
+ WritableComparator.get(Text.class), Text.class, Text.class,
+ conf, new NullProgress());
+ while (valItr.more()) {
+ Object key = valItr.getKey();
+ String keyString = key.toString();
+ // make sure it matches!
+ assertEquals(vals[0].key, keyString);
+ // must have at least 1 value!
+ assertTrue(valItr.hasNext());
+ Text value1 = valItr.next();
+ // must have at least 2 values!
+ assertTrue(valItr.hasNext());
+ Text value2 = valItr.next();
+ // do test
+ assertNotSame(value1, value2);
+ assertTrue(!value1.equals(value2));
+ assertTrue(!value2.equals(value1));
+ // make sure the key hasn't changed under the hood
+ assertEquals(keyString, valItr.getKey().toString());
+ valItr.nextKey();
+ }
+ }
+
+ public void testValueIterator2() throws Exception {
+ Path tmpDir = new Path("build/test/test.reduce.task");
+ Configuration conf = new Configuration();
+ Pair[] test = new Pair[]{ new Pair("k1", "v1"), new Pair("k1", "v2") };
+ runValueIterator2(tmpDir, test, conf);
+ }
}
{noformat}
h2. A program with a bug
I would imagine that a programmer would be really confused when everything is
equal in the example below, for any text input:
{noformat}
package test;
import java.io.*;
import java.util.*;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.conf.*;
import org.apache.hadoop.io.*;
import org.apache.hadoop.util.*;
import org.apache.hadoop.mapred.*;
import org.apache.hadoop.mapred.lib.*;
public class TestNewReducer extends Configured implements Tool {
public int run(String[] args) throws Exception {
if (args.length < 2) {
System.out.println("TestNewReducer <inDir> <outDir>");
return -1;
}
JobConf conf = new JobConf(getConf(), TestNewReducer.class);
conf.setJobName("test-newreducer");
conf.setInputFormat(TextInputFormat.class);
conf.setMapperClass(Map.class);
conf.setReducerClass(Reduce.class);
conf.setOutputFormat(TextOutputFormat.class);
conf.setOutputKeyClass(LongWritable.class);
conf.setOutputValueClass(Text.class);
FileInputFormat.setInputPaths(conf, new Path(args[0]));
FileOutputFormat.setOutputPath(conf, new Path(args[1]));
JobClient.runJob(conf);
return 0;
}
public static void main(String[] args) throws Exception {
int res = ToolRunner.run(new Configuration(), new TestNewReducer(),
args);
System.exit(res);
}
public static class Map extends MapReduceBase implements Mapper
<LongWritable, Text, LongWritable, Text> {
private final static LongWritable nill = new LongWritable(1);
public void map (LongWritable key, Text value,
OutputCollector<LongWritable, Text> output,
Reporter reporter) throws IOException {
output.collect(nill, value);
}
}
public static class Reduce extends MapReduceBase implements
Reducer<LongWritable, Text, LongWritable, Text> {
private final static LongWritable nill = new LongWritable(1);
public void reduce(LongWritable key, Iterator<Text> values,
OutputCollector<LongWritable, Text> output,
Reporter reporter) throws IOException {
Text t1 = null;
if (values.hasNext())
t1 = values.next(); // t1 is the first element in values
while(values.hasNext()) {
// is this element equal to t1?
Text t = values.next();
output.collect(nill, t.equals(t1) ? new Text("Equal") : new
Text("Not equal"));
}
}
}
}
{noformat}
h2. Possible fixes
# Return a new object each time for next(). This might have significant
overhead.
# Return a new object for unknown object types and reuse the same object for
known types (like Text). Remove "if (this==anObject) return true;" check from
all equals() methods for known objects.
# Document clearly that all user-defined classes must implement an equals()
method, which doesn't do the "if (this==anObject) return true;" check (ie. push
the problem to the user).
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
