[GitHub] [maven-surefire] reinhapa opened a new pull request #343: SUREFIRE-1881 Adds additional debug log and fork connection timeout

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
50 messages Options
123
Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-surefire] reinhapa opened a new pull request #343: SUREFIRE-1881 Adds additional debug log and fork connection timeout

GitBox

reinhapa opened a new pull request #343:
URL: https://github.com/apache/maven-surefire/pull/343


   


--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-surefire] Tibor17 commented on pull request #343: SUREFIRE-1881 Adds additional debug log and fork connection timeout

GitBox

Tibor17 commented on pull request #343:
URL: https://github.com/apache/maven-surefire/pull/343#issuecomment-810609659


   The TCP client appears in the forked JVM and I think we should make the same trick in `SurefireMasterProcessChannelProcessorFactory`.
   I will use your changes and install artifacts to my local repo. Maybe this would unlock the connections.
   Thx for your work!


--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-surefire] Tibor17 commented on pull request #343: SUREFIRE-1881 Adds additional debug log and fork connection timeout

GitBox
In reply to this post by GitBox

Tibor17 commented on pull request #343:
URL: https://github.com/apache/maven-surefire/pull/343#issuecomment-810622484


   @reinhapa
   I have several experience with the influence of GC load to the locking and reorderinng due to delay caused by the GC, especially on MacOS.
   I think this iss similar with Windows because `-verbose:class` prints many logs and overloads the process much more than `-verbose:gc`. I guess the Win impl of JVM has some weaknesses in this case. Maybe a nice solution would be to implement `CompletionHandler` and use `CountDownLatch`. Maybe it would be better than having some sleeps and loops, etc. We are aming for getting faster and faster JVM startup and teardown since we made a big investment to speed up the JVM in M5.


--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-surefire] Tibor17 edited a comment on pull request #343: SUREFIRE-1881 Adds additional debug log and fork connection timeout

GitBox
In reply to this post by GitBox

Tibor17 edited a comment on pull request #343:
URL: https://github.com/apache/maven-surefire/pull/343#issuecomment-810622484


   @reinhapa
   I have several experiences with the influence of GC load to the locking and reorderinng due to delay caused by the GC, especially on MacOS. I think this is similar issue with JVm on Windows because `-verbose:class` prints many logs and overloads the process much more than `-verbose:gc`. I guess the Win impl of JVM has some weaknesses in this case. Maybe a nice solution would be to implement `CompletionHandler` and use `CountDownLatch`. Maybe it would be better than having some sleeps and loops, etc. We are aming for getting faster and faster JVM startup and teardown since we made a big investment to speed up the JVM in M5.


--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-surefire] kriegaex commented on pull request #343: SUREFIRE-1881 Adds additional debug log and fork connection timeout

GitBox
In reply to this post by GitBox

kriegaex commented on pull request #343:
URL: https://github.com/apache/maven-surefire/pull/343#issuecomment-810727496


   > I guess the Win impl of JVM has some weaknesses in this case.
   
   @Tibor17, please don't forget that @reinhapa said [here](https://issues.apache.org/jira/browse/SUREFIRE-1881?focusedCommentId=17311201&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17311201):
   
   > with your latest changes the build does hang also under Linux and the behaviour is reproducible
   
   My "latest changes" he refers to is the commit that removed the agent and just uses `-class:verbose` in order to provoke the hang. So this is not purely a Windows issue. Please keep that in mind.


--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-surefire] Tibor17 commented on pull request #343: SUREFIRE-1881 Adds additional debug log and fork connection timeout

GitBox
In reply to this post by GitBox

Tibor17 commented on pull request #343:
URL: https://github.com/apache/maven-surefire/pull/343#issuecomment-810834109


   @kriegaex
   This code is right!
   Why you think that we have so many working builds in GitHub and Jenkins in the Apache?
   I don;t know if you are only a scrum manager or a real developer but everybody can see that `.connect().get()` is right and legal code. We are not fixing my bug, we are here fixing JVM issues. You really should open a ticket in OpenJDK!


--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #343: SUREFIRE-1881 Adds additional debug log and fork connection timeout

GitBox
In reply to this post by GitBox

Tibor17 commented on a change in pull request #343:
URL: https://github.com/apache/maven-surefire/pull/343#discussion_r604652429



##########
File path: maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ForkStarter.java
##########
@@ -656,6 +652,10 @@ private RunResult fork( Object testSet, PropertiesWrapper providerProperties, Fo
                 errConsumer, countdownCloseable );
             err.start();
 
+            log.debug( "Waiting for fork [" + forkNumber + "] process to connect..." );

Review comment:
       You cannot move this part, you cannot read data unless you are connected to the client. If you expect any error message, then it has to come from the `accept()` method and the `get()` method.




--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-surefire] Tibor17 edited a comment on pull request #343: SUREFIRE-1881 Adds additional debug log and fork connection timeout

GitBox
In reply to this post by GitBox

Tibor17 edited a comment on pull request #343:
URL: https://github.com/apache/maven-surefire/pull/343#issuecomment-810834109


   @kriegaex
   This code is right!
   Why you think that we have so many working builds in GitHub and Jenkins in the Apache?
   I don;t know if you are only a scrum manager or a real developer but everybody can see that `.accept().get()` is right and legal code. We are not fixing my bug, we are here fixing JVM issues. You really should open a ticket in OpenJDK!


--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-surefire] Tibor17 edited a comment on pull request #343: SUREFIRE-1881 Adds additional debug log and fork connection timeout

GitBox
In reply to this post by GitBox

Tibor17 edited a comment on pull request #343:
URL: https://github.com/apache/maven-surefire/pull/343#issuecomment-810834109


   @kriegaex
   This code is right!
   Why you think that we have so many working builds in GitHub and Jenkins on Mac, Linux and Ubuntu in the Apache on Mac, Linux and Ubuntu?
   I don;t know if you are only a scrum manager or a real developer but everybody can see that `.accept().get()` is right and legal code. We are not fixing my bug, we are here fixing JVM issues. You really should open a ticket in OpenJDK!


--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-surefire] reinhapa commented on a change in pull request #343: SUREFIRE-1881 Adds additional debug log and fork connection timeout

GitBox
In reply to this post by GitBox

reinhapa commented on a change in pull request #343:
URL: https://github.com/apache/maven-surefire/pull/343#discussion_r604654736



##########
File path: maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ForkStarter.java
##########
@@ -656,6 +652,10 @@ private RunResult fork( Object testSet, PropertiesWrapper providerProperties, Fo
                 errConsumer, countdownCloseable );
             err.start();
 
+            log.debug( "Waiting for fork [" + forkNumber + "] process to connect..." );

Review comment:
       Well, I do not know why those builds pass... but in case my case it definitely fixes the locking forked process when having an output on the forked process when adding the `-verbose:classes` JVM argument...




--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-surefire] Tibor17 edited a comment on pull request #343: SUREFIRE-1881 Adds additional debug log and fork connection timeout

GitBox
In reply to this post by GitBox

Tibor17 edited a comment on pull request #343:
URL: https://github.com/apache/maven-surefire/pull/343#issuecomment-810834109


   @kriegaex
   This code is right!
   Why you think that we have so many working builds in GitHub and Jenkins on Mac, Linux and Ubuntu?
   I don;t know if you are only a scrum manager or a real developer but everybody can see that `.accept().get()` is right and legal code. We are not fixing my bug, we are here fixing JVM issues. You really should open a ticket in OpenJDK!
   Why you think this would happen only with `-verbose:class` and not with `-verbose:gc`? These are JVM options and users code has no notion about it, and must not have of course, but this it has a significant influence.


--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-surefire] Tibor17 edited a comment on pull request #343: SUREFIRE-1881 Adds additional debug log and fork connection timeout

GitBox
In reply to this post by GitBox

Tibor17 edited a comment on pull request #343:
URL: https://github.com/apache/maven-surefire/pull/343#issuecomment-810834109


   @kriegaex
   This code is right!
   Why you think that we have so many working builds in GitHub and Jenkins on Mac, Linux and Ubuntu?
   I don;t know if you are only a scrum manager or a real developer but everybody can see that `.accept().get()` is right and legal code. We are not fixing my bug, we are here fixing JVM issues. You really should open a ticket in OpenJDK!
   Why you think this would happen only with `-verbose:class` and not with `-verbose:gc`? These are JVM options and users code has no notion about it, and must not have of course, but still it has a significant influence.


--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-surefire] reinhapa commented on pull request #343: SUREFIRE-1881 Adds additional debug log and fork connection timeout

GitBox
In reply to this post by GitBox

reinhapa commented on pull request #343:
URL: https://github.com/apache/maven-surefire/pull/343#issuecomment-810843571


   > @kriegaex
   > This code is right!
   > Why you think that we have so many working builds in GitHub and Jenkins on Mac, Linux and Ubuntu?
   > I don;t know if you are only a scrum manager or a real developer but everybody can see that `.accept().get()` is right and legal code. We are not fixing my bug, we are here fixing JVM issues. You really should open a ticket in OpenJDK!
   > Why you think this would happen only with `-verbose:class` and not with `-verbose:gc`? These are JVM options and users code has no notion about it, and must not have of course, but still it has a significant influence.
   
   @Tibor17 I'm also an contributor to the OpenJDK and not "just" a scrum master (nothing said against this role), working for more then 20 years with Java now. I much appreciate the work for the project, but I do not really see a JDK problem here and this PR is still work in progress and can changes as we will get more details...


--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-surefire] Tibor17 commented on pull request #343: SUREFIRE-1881 Adds additional debug log and fork connection timeout

GitBox
In reply to this post by GitBox

Tibor17 commented on pull request #343:
URL: https://github.com/apache/maven-surefire/pull/343#issuecomment-810847701


   @rehevkor5
   You can see that the last change timed out the build because you reordered `accept` and the reads. Look, first you should understand the principal of the error and then make a treatment. From here it's obvious to me that we have no notion what's going on. We know that `-verbose:class` hangs the TCP connections, and `-verbose:gc` does not hang the connections. Therefore this has to tell me that yes it is a JVM related issue.


--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-surefire] Tibor17 edited a comment on pull request #343: SUREFIRE-1881 Adds additional debug log and fork connection timeout

GitBox
In reply to this post by GitBox

Tibor17 edited a comment on pull request #343:
URL: https://github.com/apache/maven-surefire/pull/343#issuecomment-810847701


   @rehevkor5
   You can see that the last change timed out the build because you reordered `accept` and the reads. Look, first you should understand the principal of the error and then make a treatment. From here it's obvious to me that we have no notion what's going on. All we know is the behavior, that `-verbose:class` hangs the TCP connections, and `-verbose:gc` does not hang the connections. Therefore this has to tell me that yes it is a JVM related issue.


--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-surefire] Tibor17 commented on pull request #343: SUREFIRE-1881 Adds additional debug log and fork connection timeout

GitBox
In reply to this post by GitBox

Tibor17 commented on pull request #343:
URL: https://github.com/apache/maven-surefire/pull/343#issuecomment-810851691


   It;s clear to me that the servers use `CompletionHandler` and we don't. Maybe the JDK is tested much more in the scenario with `CompletionHandler` as the third method parameter in `accept` and `connect` too. I don't know if we would workaround the JDK issue this way but we can try.


--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-surefire] kriegaex commented on pull request #343: SUREFIRE-1881 Adds additional debug log and fork connection timeout

GitBox
In reply to this post by GitBox

kriegaex commented on pull request #343:
URL: https://github.com/apache/maven-surefire/pull/343#issuecomment-810952849


   > @kriegaex
   > This code is right!
   
   What the heck, @Tibor17? Again, you did not read my comment carefully or misinterpret it. I was not commenting the code in this PR at all but **my own** last changes in my reproducer project.
   
   As for this PR, even I as a hobby programmer can see at first glance that the PR is work in progress and in its current state not meant to be merged but just to produce some insights into what might be going wrong. It is a debugging tool, nothing more! @reinhapa IMO only created a PR from his fork because it is easy for you to check out and comment on. So you mentioning that it makes build time out is not helpful, because it is not a surprise either. We are both just trying to help you debug this, because you seem to be unwilling to do it alone. Sorry for trying to support you.
   


--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-surefire] kriegaex edited a comment on pull request #343: SUREFIRE-1881 Adds additional debug log and fork connection timeout

GitBox
In reply to this post by GitBox

kriegaex edited a comment on pull request #343:
URL: https://github.com/apache/maven-surefire/pull/343#issuecomment-810952849


   > @kriegaex
   > This code is right!
   
   What the heck, @Tibor17? Again, you did not read my comment carefully or misinterpret it. I was not commenting the code in this PR at all but **my own** last changes in my reproducer project.
   
   As for this PR, even I as a hobby programmer can see at first glance that the PR is work in progress and in its current state not meant to be merged but just to produce some insights into what might be going wrong. It is a debugging tool, nothing more! @reinhapa IMO only created a PR from his fork because it is easy for you to check out and comment on. So you mentioning that it makes build time out is not helpful, because it is not a surprise either. We are both just trying to help you debug this, because you seem to be unwilling to do it alone. Sorry for trying to support you in this imperfect way, as you seem to know everything better (even if you don't) and seem to know how to solve the problem (even if you don't) or else put the blame on my sample project or this PR which both just try to reproduce the problem.


--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-surefire] kriegaex commented on pull request #343: SUREFIRE-1881 Adds additional debug log and fork connection timeout

GitBox
In reply to this post by GitBox

kriegaex commented on pull request #343:
URL: https://github.com/apache/maven-surefire/pull/343#issuecomment-810956819


   Oh, and one more thing. Just in case someone wants to bring up the issue of me not being friendly anymore: I was so patient over in the Surefire issue, explaining so much, delivering so much data. @Tibor17 has just been either ignorant or dismissive of all I did to make life easier for him as a maintainer, helping him to pinpoint the issue. This is why my patience was wearing thin and now I am just trying to push this forward. I am friendly and polite with persons who grant me the same courtesy. The condescending way you treated me has not helped me feel comfortable in collaborating with you. Despite all that, I am still collaborating. I am just not being courteous or nice anymore. Sorry for that, but I am a human being too.


--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-surefire] kriegaex edited a comment on pull request #343: SUREFIRE-1881 Adds additional debug log and fork connection timeout

GitBox
In reply to this post by GitBox

kriegaex edited a comment on pull request #343:
URL: https://github.com/apache/maven-surefire/pull/343#issuecomment-810956819


   Oh, and one more thing. Just in case someone wants to bring up the issue of me not being friendly anymore: I admit to it. But I was so patient over in the Surefire issue, explaining so much, delivering so much data. @Tibor17 has just been either ignorant or dismissive of all I did to make life easier for him as a maintainer, helping him to pinpoint the issue. This is why my patience was wearing thin and now I am just trying to push this forward. I am friendly and polite with persons who grant me the same courtesy. The condescending way you treated me has not helped me feel comfortable in collaborating with you. Despite all that, I am still collaborating. I am just not being courteous or nice anymore. Sorry for that, but I am a human being too.


--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


123