I traced out the State Machine for TaskAttemptImpl and I verified that all states after ASSIGNED, can handle TA_CONTAINER_LAUNCHED and/or TA_CONTAINER_LAUNCH_FAILED depending on how they are returned. I also looked at the JobImpl state Machine ad did a similar thing for the Counter Updates.

Robert Joseph Evans
added a comment - 10/Apr/12 18:26 I traced out the State Machine for TaskAttemptImpl and I verified that all states after ASSIGNED, can handle TA_CONTAINER_LAUNCHED and/or TA_CONTAINER_LAUNCH_FAILED depending on how they are returned. I also looked at the JobImpl state Machine ad did a similar thing for the Counter Updates.

Hadoop QA
added a comment - 10/Apr/12 19:35 +1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12522148/MR-3932.txt
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 1 new or modified test files.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 eclipse:eclipse. The patch built with eclipse:eclipse.
+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 core tests. The patch passed unit tests in .
+1 contrib tests. The patch passed contrib unit tests.
Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2187//testReport/
Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2187//console
This message is automatically generated.

Bobby, the patch looks good. Couple of minor comments.
The additional comment in TestTaskAttempt about "testAttemptContainerRequest" needing to run first - nice catch, would be useful to have a little more info in the comment though (referring to ContainerLaunchContext creation ?).
Also, I believe the new test you added can be run without bothering with credentials, tokens etc.

Siddharth Seth
added a comment - 11/Apr/12 04:53 Bobby, the patch looks good. Couple of minor comments.
The additional comment in TestTaskAttempt about "testAttemptContainerRequest" needing to run first - nice catch, would be useful to have a little more info in the comment though (referring to ContainerLaunchContext creation ?).
Also, I believe the new test you added can be run without bothering with credentials, tokens etc.

Hadoop QA
added a comment - 11/Apr/12 18:04 +1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12522277/MR-3932.txt
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 1 new or modified test files.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 eclipse:eclipse. The patch built with eclipse:eclipse.
+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 core tests. The patch passed unit tests in .
+1 contrib tests. The patch passed contrib unit tests.
Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2202//testReport/
Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2202//console
This message is automatically generated.