hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Aaron T. Myers (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HADOOP-8755) Print thread dump when tests fail due to timeout
Date Fri, 07 Sep 2012 00:45:07 GMT

    [ https://issues.apache.org/jira/browse/HADOOP-8755?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13450230#comment-13450230
] 

Aaron T. Myers commented on HADOOP-8755:
----------------------------------------

bq. You're right - changing this timeout wouldn't help. There are 2 timeouts for test execution
time: one in surefire and another is in junit. Surefire just kills a child process when timeout
is exceeded, and this patch doesn't handle this.

Got it. Makes sense. Thanks a lot for the explanation.

bq. What's handled is if a test method is annotated with @Test and the timeout parameter is
given, then junit will fail the test and thread dump will be printed. TestFileAppend4 is an
example of a test providing timeout parameter.

Gotcha. I also tested this similarly and was able to confirm that the patch works as expected.
Very cool.

bq. AFAIK we can't really do anything with the surefire timeout. Still we may have thread
dumps printed for all tests in case of timeouts if we introduce a default timeout for all
tests on the junit level. I guess it is doable with a custom surefire provider for junit,
but I'm not sure we really need this. What do you think?

I think that would be a great thing to add. Not very many of the Hadoop tests are annotated
with a JUnit timeout (79 / 3984, by my quick count), and in my experience tests which aren't
annotated with a JUnit timeout certainly do in fact timeout and end up hitting the Surefire
fork timeout. If that's not handled somehow, that would make this change much less useful.
If it's easy, mind adding a default JUnit timeout to the next rev of this patch? If not, we
could certainly do it as a separate JIRA.

As for comments on the contents of the patch, just a few little things:

# I think it'd be good to print an informative header before dumping the stacks, e.g. something
like "====> TEST TIMED OUT. PRINTING THREAD DUMP. <===="
# Several of the lines in the patch go over 80 characters.
# Our coding guidelines say that lines which continue onto the next line should be indented
4 spaces, not 2.

Otherwise the patch looks great.
                
> Print thread dump when tests fail due to timeout 
> -------------------------------------------------
>
>                 Key: HADOOP-8755
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8755
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: test
>    Affects Versions: 1.0.3, 0.23.1, 2.0.0-alpha
>            Reporter: Andrey Klochkov
>            Assignee: Andrey Klochkov
>         Attachments: HDFS-3762-branch-0.23.patch, HDFS-3762.patch, HDFS-3762.patch, HDFS-3762.patch,
HDFS-3762.patch, HDFS-3762.patch
>
>
> When a test fails due to timeout it's often not clear what is the root cause. See HDFS-3364
as an example.
> We can print dump of all threads in this case, this may help finding causes.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Mime
View raw message