You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by "jbertram (via GitHub)" <gi...@apache.org> on 2023/11/21 20:38:23 UTC

[PR] ARTEMIS-4513 HTTP request logging not working [activemq-artemis]

jbertram opened a new pull request, #4686:
URL: https://github.com/apache/activemq-artemis/pull/4686

   (no comment)


-- 
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.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ARTEMIS-4513 HTTP request logging not working [activemq-artemis]

Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic merged PR #4686:
URL: https://github.com/apache/activemq-artemis/pull/4686


-- 
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.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ARTEMIS-4513 HTTP request logging not working [activemq-artemis]

Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4686:
URL: https://github.com/apache/activemq-artemis/pull/4686#discussion_r1406315823


##########
artemis-unit-test-support/src/main/java/org/apache/activemq/artemis/utils/ThreadLeakCheckRule.java:
##########
@@ -295,6 +295,8 @@ private boolean isExpectedThread(Thread thread) {
          return true;
       } else if (threadName.contains("GC Daemon")) {
          return true;
+      } else if (threadName.contains("org.eclipse.jetty.util.RolloverFileOutputStream")) {

Review Comment:
   I don't mean to move the whole test into integration-tests-isolated. but rather write just this test into the isolated package.. 
   
   
   This is definitely causing a thread leakage.. and this thread could be bothering other tests.
   
   I strongly recommend against it.



-- 
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.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ARTEMIS-4513 HTTP request logging not working [activemq-artemis]

Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4686:
URL: https://github.com/apache/activemq-artemis/pull/4686#discussion_r1406312534


##########
artemis-unit-test-support/src/main/java/org/apache/activemq/artemis/utils/ThreadLeakCheckRule.java:
##########
@@ -295,6 +295,8 @@ private boolean isExpectedThread(Thread thread) {
          return true;
       } else if (threadName.contains("GC Daemon")) {
          return true;
+      } else if (threadName.contains("org.eclipse.jetty.util.RolloverFileOutputStream")) {

Review Comment:
   Isn't this hiding a thread leakage? can't we just close the thread instead?



-- 
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.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ARTEMIS-4513 HTTP request logging not working [activemq-artemis]

Posted by "jbertram (via GitHub)" <gi...@apache.org>.
jbertram commented on code in PR #4686:
URL: https://github.com/apache/activemq-artemis/pull/4686#discussion_r1406373246


##########
artemis-unit-test-support/src/main/java/org/apache/activemq/artemis/utils/ThreadLeakCheckRule.java:
##########
@@ -295,6 +295,8 @@ private boolean isExpectedThread(Thread thread) {
          return true;
       } else if (threadName.contains("GC Daemon")) {
          return true;
+      } else if (threadName.contains("org.eclipse.jetty.util.RolloverFileOutputStream")) {

Review Comment:
   The thread is specific to the HTTP request logging from Jetty. Looking at the Jetty source code there appears to be no way to close it. I'll move the test into the isolated module.



-- 
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.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ARTEMIS-4513 HTTP request logging not working [activemq-artemis]

Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4686:
URL: https://github.com/apache/activemq-artemis/pull/4686#discussion_r1406313760


##########
artemis-unit-test-support/src/main/java/org/apache/activemq/artemis/utils/ThreadLeakCheckRule.java:
##########
@@ -295,6 +295,8 @@ private boolean isExpectedThread(Thread thread) {
          return true;
       } else if (threadName.contains("GC Daemon")) {
          return true;
+      } else if (threadName.contains("org.eclipse.jetty.util.RolloverFileOutputStream")) {

Review Comment:
   Or move this test to the isolated tests if you can't figure out how to close the thread.



-- 
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.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org