Dependent tasks may be started more than once

[GoogleCodeExporter]

What steps will reproduce the problem?

1. Run the attached hamakefile (with -n): multiplestarts.xml

What is the expected output? What do you see instead?

I expect to see task c executed only once, instead it gets run twice!

hadoop jar dist/hamake*.jar -n -f file://$(pwd)/multiplestarts.xml
11/07/22 12:11:11 INFO hamake.Main: Using Hadoop 0.20.2-cdh3u0
11/07/22 12:11:11 INFO hamake.Main: Working dir:  
hdfs://hadoop-name01.den01.oneriot.com:54310/user/peter
11/07/22 12:11:11 INFO hamake.Main: Reading hamake-file 
file:///home/peter/src/hamake/multiplestarts.xml
11/07/22 12:11:11 INFO hamake.Main: Parsing hamake-file 
file:///home/peter/src/hamake/multiplestarts.xml
11/07/22 12:11:12 WARN syntax.SyntaxParser: Could not locate RELAX NG schema 
factory. Using XSD validator
11/07/22 12:11:12 INFO hamake.TaskRunner: Calculating dependencies
11/07/22 12:11:12 INFO hamake.TaskRunner: Starting a
11/07/22 12:11:12 INFO hamake.TaskRunner: Starting b
11/07/22 12:11:12 INFO hamake.TaskRunner: Execution of a is completed
11/07/22 12:11:12 INFO hamake.TaskRunner: Execution of b is completed
11/07/22 12:11:12 INFO hamake.TaskRunner: Starting d
11/07/22 12:11:12 INFO hamake.TaskRunner: Starting c
11/07/22 12:11:12 INFO hamake.TaskRunner: Starting c
11/07/22 12:11:12 INFO hamake.TaskRunner: Execution of d is completed
11/07/22 12:11:12 INFO hamake.TaskRunner: Execution of c is completed
11/07/22 12:11:12 INFO hamake.TaskRunner: Execution of c is completed

Please use labels and text to provide additional information.

It seems that NoDepsExecutionGraph.getReadyForRunTasks(*) were attempting to 
use Collections.binarySearch() to eliminate duplicates, but were not keeping 
the list sorted such that that would work.  The attached patch simply replaces 
the Lists with Sets, both in those functions and in the corresponding 
signatures defined by ExecutionGraph, eliminating the need to call 
Collections.binarySearch() at all.

-peter

Original issue reported on code.google.com by petenewc...@gmail.com on 22 Jul 2011 at 6:26

Attachments:

• multiplestarts.xml
• multiplestarts.patch

[GoogleCodeExporter]

Oops, forgot the necessary patch to TestNoDepsExecutionGraph...  Use this patch 
instead.

Original comment by petenewc...@gmail.com on 22 Jul 2011 at 6:35

[GoogleCodeExporter]

Hmm... don't know if that last patch actually got through (I can't see the 
attachment).  Here's the additional patch text just in case:

Index: test/java/com/codeminders/hamake/TestNoDepsExecutionGraph.java
===================================================================
--- test/java/com/codeminders/hamake/TestNoDepsExecutionGraph.java  (revision 
379)
+++ test/java/com/codeminders/hamake/TestNoDepsExecutionGraph.java  (working 
copy)
@@ -3,7 +3,7 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.List;
+import java.util.Set;

 import junit.framework.Assert;

@@ -117,7 +117,7 @@

    private void assertGraphHasLevels(ExecutionGraph graph, int expectedSteps){     
        int actualSteps = 0;
-       List<String> tasks = null;
+       Set<String> tasks = null;
        do{
            tasks = graph.getReadyForRunTasks();
            if(tasks.size() > 0){

Original comment by petenewc...@gmail.com on 22 Jul 2011 at 6:36

[GoogleCodeExporter]

Thank you for the Patch. 

Vladimir, please review and commit TODAY.

Original comment by kroko...@gmail.com on 22 Jul 2011 at 9:40

[GoogleCodeExporter]

Thank you Peter! This patch fixes an issue caused by a bad code. I've added a 
new unit test with name 'testGraphWithDublicatedDependencies' to the class 
'TestNoDepsExecutionGraph' that emulates use case, described by you.

Vladimir

Original comment by v...@codeminders.com on 24 Jul 2011 at 7:11

[GoogleCodeExporter]

Vladimit, please mark this issue as Fixed once patch is committed.

Original comment by kroko...@gmail.com on 25 Jul 2011 at 10:07

[GoogleCodeExporter]

Vadim, could you please add vorl@codeminders.com as a comitter?

Original comment by vorl.s...@gmail.com on 26 Jul 2011 at 7:30

[GoogleCodeExporter]

done

Original comment by kroko...@gmail.com on 26 Jul 2011 at 4:15

[GoogleCodeExporter]

Original comment by v...@codeminders.com on 1 Aug 2011 at 6:10

• Changed state: Fixed